xpra icon
Bug tracker and wiki

Opened 11 months ago

Last modified 3 months ago

#2064 new defect

x264 produces too many delayed frames

Reported by: Antoine Martin Owned by: Smo
Priority: major Milestone: 2.5
Component: encodings Version: 2.4.x
Keywords: Cc: steved424@…

Description

As per r19322, we don't need to enable b-frames to see delayed frames.

Sometimes many:

compress:   0.3ms for  300x300  pixels at    0,0    for wid=2     using      h264 with ratio   0.9%  \
    (  351KB to     3KB), sequence   167, client_options=\
    {u'pts': 188, u'frame': 10, u'delayed': 11, 'csc': u'YUV420P', 'paint': False, u'type': u'B'}
(..)
compress:   0.7ms for  300x300  pixels at    0,0    for wid=2     using      h264 with ratio   2.1%  \
   (  351KB to     7KB), sequence   493, client_options=\
   {u'type': u'B', u'frame': 336, u'pts': 6929, u'delayed': 11, 'csc': u'YUV420P'}

We do send jpeg frames to prevent stuttering, but 11 frames is too many and will cause av-sync to fail.

Attachments (1)

h264frames.zip (166.7 KB) - added by J. Max Mena 10 months ago.
h264frames test output data, organized by XPRA_X264_MAX_DELAYED_FRAMES

Download all attachments as: .zip

Change History (11)

comment:1 Changed 11 months ago by Antoine Martin

Status: newassigned

Probably comes from this code setting i_delay:

    if( h->param.i_bframe_adaptive == X264_B_ADAPT_TRELLIS && !h->param.rc.b_stat_read )
        h->frames.i_delay = X264_MAX(h->param.i_bframe,3)*4;
    else
        h->frames.i_delay = h->param.i_bframe;
    if( h->param.rc.b_mb_tree || h->param.rc.i_vbv_buffer_size )
        h->frames.i_delay = X264_MAX( h->frames.i_delay, h->param.rc.i_lookahead );
    i_slicetype_length = h->frames.i_delay;
    h->frames.i_delay += h->i_thread_frames - 1;
    h->frames.i_delay += h->param.i_sync_lookahead;
    h->frames.i_delay += h->param.b_vfr_input;

Because later on:

        if( h->frames.i_input <= h->frames.i_delay + 1 - h->i_thread_frames )
        {
            /* Nothing yet to encode, waiting for filling of buffers */
            pic_out->i_type = X264_TYPE_AUTO;
            return 0;
        }
Last edited 11 months ago by Antoine Martin (previous) (diff)

comment:2 Changed 11 months ago by Antoine Martin

  • the first one is OK, we already replace X264_B_ADAPT_TRELLIS with X264_B_ADAPT_FAST - and i_bframe should be set to 1 if enabled
  • the value of i_lookahead is not greater than i_bframe
  • i_thread_frames is set to: h->i_thread_frames = h->param.b_sliced_threads ? 1 : h->param.i_threads (see also #1840): when using sliced threads this is not a problem, but when sliced threads are disabled we end up using i_threads which is set to X264_THREADS_AUTO by default and this evaluates to i_threads = x264_cpu_num_processors() * (h->param.b_sliced_threads?2:3)/2; - this can be a fairly big number.
  • i_sync_lookahead should probably be capped (set elsewhere by the profile or tune)
  • b_vfr_input is always 0

comment:3 Changed 11 months ago by Antoine Martin

Owner: changed from Antoine Martin to J. Max Mena
Status: assignednew

Preparatory work in r21113.
r21114:

  • XPRA_X264_MAX_DELAYED_FRAMES=4
  • we now always set the number of threads used to get_cpu_count()//2 (maximum 4)

on systems with 4 cpus or less, this probably doesn't make much of a difference.

Changed 10 months ago by J. Max Mena

Attachment: h264frames.zip added

h264frames test output data, organized by XPRA_X264_MAX_DELAYED_FRAMES

comment:4 Changed 10 months ago by J. Max Mena

I ran a series of tests on a VM with 8 cores, and quite a bit of RAM. I ran the same series of tests with XPRA_X264_MAX_DELAYED_FRAMES= set to [1, 2, 4, 6, 8]. I have attached the output data.

Charts will come eventually, but for now, here's the test data.

comment:5 Changed 9 months ago by Antoine Martin

Deciphering the filenames of your CSV files (ie: 8_2_3.csv), I guess the first number is the value of XPRA_X264_MAX_DELAYED_FRAMES, the second number is always 2 (whatever that means) and the third is the test run?

Some files have 2 records, some 3. Why is that?
Why not aggregate them into a single sheet and segregate the records using the Custom Params? (which should contain XPRA_X264_MAX_DELAYED_FRAMES)

Just like the other ticket, the most useful data is missing (batch delay, etc)

You are testing with version r21230M - why is the Modification flag set? What did you change and why?


I've just skimmed the logs and they're full of:

Error importing libyuv colorspace conversion (csc_libyuv)

You need csc_libyuv, it's a must.

There are lots of other errors in there: r21274 (already fixed), dbus-launch not installed, r21442 (found today in those logs) etc..

Last edited 9 months ago by Antoine Martin (previous) (diff)

comment:6 Changed 9 months ago by J. Max Mena

Why not aggregate them into a single sheet and segregate the records using the Custom Params?

I uhhh didn't know you could do that. I'll look in to how it's done and rewrite the Bash script to do it that way. From what I can tell you can just add extra arguments when calling test_measure_perf.py? In this case I'd run the commands as (?):

./test_measure_perf.py config_file /path/to/csv.csv # # XPRA_X264_MAX_DELAYED_FRAMES=# > /path/to/logs.log

I guess the first number is the value of XPRA_X264_MAX_DELAYED_FRAMES, the second number is always 2 (whatever that means) and the third is the test run?

From what I can tell by reading the comments in test_measure_perf.py and perf_config_default.py, the second number is the Xpra version number; I've been leaving it as 2. If it should be something else, I can change it pretty trivially. The last number is ordinal referring to which run in the set. I've been following the pattern of:

NAMEOFTESTRUN_XPRAVER#_RUN#

You are testing with version r21230M - why is the Modification flag set? What did you change and why?

Nothing - I just had the latest version from /beta installed. I just ran an update on the VM and now it's at r21402. None of my VMs or bare metal test boxes build from source, they all pull from /beta by default. I have a VM that's specially marked as building from trunk if I need it, but for the most part I don't use that VM or set up. I much prefer to pull from the repos as that's much easier to automate using Ansible.

You need csc_libyuv, it's a must.

As far as I can tell libyuv and libyuv-devel are both installed on all the VMs I've set up so I'm not sure why csc_libyuv isn't loading.

Just like the other ticket, the most useful data is missing (batch delay, etc)

I'll file a new ticket in a minute - they seem to be missing from most of the test runs, and as is mentioned in the other ticket the tests will have to be updated.

Some files have 2 records, some 3. Why is that?

That's really odd. As far as I can tell, some have to xterm lines and some don't. I'm really not sure what could have caused that - the Bash script I wrote uses the exact same command for each run. Same configs, same everything - the only thing that changes is the name of the output files.


I'll file that ticket for the tests not outputting all the correct data and then make some changes to the Bash scripts for this, #1840, and #619.

comment:7 Changed 9 months ago by Antoine Martin

Why not aggregate them into a single sheet and segregate the records using the Custom Params?

I uhhh didn't know you could do that.

CSV is plain text, copy + paste.

In this case I'd run the commands as (?):

Don't add extra arguments that don't add value.

the second number is the Xpra version number; I've been leaving it as 2.

Why record the version number there when it is already recorded directly in the test data itself? (from both client and server)
This will prevent you from using that column as a number.

I'm not sure why csc_libyuv isn't loading.

Some jpeg symbols are missing.

comment:8 Changed 8 months ago by Antoine Martin

Owner: changed from J. Max Mena to Jonathan Anthony

comment:9 Changed 7 months ago by tc424

Cc: steved424@… added

comment:10 Changed 3 months ago by Smo

Owner: changed from Jonathan Anthony to Smo
Note: See TracTickets for help on using tickets.