allow B frames with video encoders

[totaam]

Issue migrated from trac ticket # 800

component: encodings | priority: minor | resolution: fixed

2015-02-04 11:39:04: antoine created the issue

---

• add client encoding capability flag (and make it a number specifying the max ref frames? will only support 1 for now?)
• video encoders take this into account (ie: NVENC4 YUV444 and lossless mode with NVENC4 #796)
• window video source needs to know about this and schedule a new frame refresh after a timeout to ensure we push the last frame out one way or another
• client decoding code needs to ignore empty frames (B frames need the next frame to arrive first), and be able to paint more than one frame from a single buffer push..

[totaam]

2015-07-21 12:38:52: totaam uploaded file delayed-frames.patch (11.9 KiB)

work in progress to support b frames

[totaam]

2015-07-22 13:28:09: totaam changed status from new to assigned

[totaam]

2015-07-22 13:28:09: totaam changed owner from antoine to totaam

[totaam]

2015-07-22 13:28:09: totaam commented

---

Code to flush the encoder:

while( x264_encoder_delayed_frames( h ) )
{
    i_frame_size = Encode_frame( h, opt->hout, NULL );
    if( i_frame_size < 0 )
        return -1;
    i_file += i_frame_size;
}

What we want is to keep at most N frames in the encoder at any point in time.
Where N is very low without av-sync (maybe just 1), or derived from the av-sync value.
We will need a timer to flush things if no new frames come in, and this belongs in the window source logic.

[totaam]

2015-11-13 06:52:29: antoine commented

---

re-scheduling

[totaam]

2016-06-17 09:05:07: antoine commented

---

For NVENC, we will have to ensure we don't exceed NV_ENC_CAPS_NUM_MAX_BFRAMES.

From the headers for NV_ENC_CONFIG.frameIntervalP: Specifies the GOP pattern as follows: frameIntervalP = 0: I, 1: IPP, 2: IBP, 3: IBBP If goplength is set to NVENC_INFINITE_GOPLENGTH frameIntervalP should be set to 1.

[totaam]

2016-06-18 16:46:13: antoine commented

---

Done for x264 in r12858.

Still todo:

• verify the bandwidth savings / performance
• take delayed frames into account for av-sync
• we don't record encoding statistics when flushing (minor)
• we call "fire_paint_callbacks" for the packet we just received, which may not be different from the frame that comes out of the decoder: the data we collect is no longer directly related to this frame, and when dealing with delayed frames with no data we don't take into account the time it takes to paint.. since there isn't anything to paint
• implement for other encoders: vpx, nvenc..

---

Somewhat related: Explanation of x264 tune, maybe we could tune to "fastdecode" when we find the client is struggling with the decode speed. At the moment we just increase the batch delay which may lower the speed and increase the complexity for the decoding.. making things worse.

[totaam]

2016-06-19 09:45:42: antoine changed status from assigned to new

[totaam]

2016-06-19 09:45:42: antoine changed owner from totaam to afarr

[totaam]

2016-06-19 09:45:42: antoine commented

---

• not going to bother with NVENC for now (too hard), moved to nvenc support for b-frames #1235
• vp8 does not have b-frames: Improving Prediction without B Frames: The lack of B frames in VP8 has sparked some discussion.. and we don't care about vp9 (too slow), and xvid is only useful for testing
• not going to fix the missing statistics either: we record when things get sent out... and in the case of delayed frames, that complicates things too much - so we just record when things do get sent, without caring if the actual frame being sent corresponds to the timestamp of the request, for all intents and purposes this is good enough
• not going to change the "fire_paint_callbacks" code for now either, for similar reasons: it's close enough and changing it might mess up the heuristics
• r12868 takes delayed frames into account when calculating the av-sync delay, which we now also re-calculate when the batch delay changes, looks like this with "-d av-sync":

update_av_sync_frame_delay() video encoder=x264_encoder(BGRX - 300x300), delayed frames=1, frame delay=49

• r12869 makes it possible to disable b-frames with XPRA_B_FRAMES=0 on the server, r12890 adds the env var to the client

---

@afarr: ready for testing.
b-frames should compress better, and make better use of the delay used for av-sync.
Verifying that we compress better is hard because of the usual problems: "define better". So many heuristics are intertwined that we could get higher fps, lower bandwidth, higher quality, etc...

What you can check though:

• xpra info now exposes what types of frames were encoded with x264:

$ xpra info  | grep frame-types
window.2.encoder.frame-types.B=73
window.2.encoder.frame-types.IDR=1
window.2.encoder.frame-types.P=74

• "-d x264" on the server shows what types or frames are being used:

x264 encode frame 2733 as    B slice with 4 nals, total    6148 bytes, keyframe=0, delayed=1
x264 encode frame 2734 as    P slice with 4 nals, total    9670 bytes, keyframe=0, delayed=1

• "-d paint" on the client shows (amongst many other things...):

draw_region(0, 0, 300, 300, h264, 7057 bytes, 0, \
   {'speed': 56, 'type': 'B', 'pts': 3579, 'frame': 350, 'quality': 98, 'delayed': 1, 'csc': 'YUV444P'}, \
    [<function record_decode_time at 0x7f4df4073e60>, <function after_draw_refresh at 0x7f4df4073ed8>])

[totaam]

2016-06-24 19:24:29: maxmylyn changed owner from afarr to antoine

[totaam]

2016-06-24 19:24:29: maxmylyn commented

---

With a trunk r12899 server and client (both Fedora 23, connected over TCP), I'm getting some really really weird painting when I enable B-Frames. I'll attach a quick screenrecord.

That being said, with B-Frames enabled video appears to be better than with B-Frames disabled. That's just my subjective opinion, we'll need some kind of performance tests edit: as mentioned in comment:5.

[totaam]

2016-06-24 19:24:59: maxmylyn uploaded file 2016-06-24 11-41-13.mp4 (3718.8 KiB)

Recorded with OBS - scrolling and switching tabs in Firefox

[totaam]

2016-06-28 15:00:01: antoine changed owner from antoine to maxmylyn

[totaam]

2016-06-28 15:00:01: antoine commented

---

I believe that what you are recording is caused by the delayed video frames used for the whole browser window: those arrive out of order, after the other updates.

• r12931 should fix this by only enabling b-frames for video regions.
• r12941 also sends a frame in place of the initial buffered video frame (using whatever non-video encoding is available) and r12942 makes it prefer jpeg

Does that improve things?

[totaam]

2016-06-28 17:24:49: maxmylyn changed owner from maxmylyn to antoine

[totaam]

2016-06-28 17:24:49: maxmylyn commented

---

Upped client and server to r12942:

Somewhat. I don't get the full repaints on switching tabs, but I do still see some painting issues that I only get with B-Frames enabled. The easiest to show is collapsing and uncollapsing images on Reddit with RES enabled in Firefox. I'll attach a screenrecord to demonstrate.

[totaam]

2016-06-28 17:25:57: maxmylyn uploaded file 2016-06-28 09-15-18.mp4 (3913.1 KiB)

Scrolling and collapsing images on Reddit - notice the flicker at the end.

[totaam]

2016-07-03 18:09:52: antoine commented

---

Support for b-frames was included in the enc_ffmpeg codec, see #1107#comment:10.

The issue shown on the video above is likely due to the fact that we mix video and non-video encodings on the page. The video gets delayed, but the non-video is not. This is a similar issue to #1218.
The best solution is probably to only use b-frames for video regions, which is not easy... will try.

[totaam]

2016-07-12 17:52:23: antoine commented

---

Milestone renamed

[totaam]

2016-07-15 16:14:39: antoine changed status from new to assigned

[totaam]

2016-07-15 16:14:39: antoine commented

---

See r13022 and #1257.

We also need to skip some frames if we've sent them as jpeg already.

[totaam]

2016-07-20 09:24:06: antoine commented

---

• r13045 fixes a bug in the x264 encoder, made apparent by the use of b-frames and the work in better x264 tuning for type of content #1257
• r13046 tells the client to skip the delayed video frames if we have sent a non-video frame for it already - this should fix some of the out-of-order display issues reported

[totaam]

2016-07-20 20:26:13: antoine changed status from assigned to new

[totaam]

2016-07-20 20:26:13: antoine changed owner from antoine to maxmylyn

[totaam]

2016-07-20 20:26:13: antoine commented

---

@maxmylyn: can you reproduce with the latest code?
(see also: #1257#comment:3)

[totaam]

2016-07-20 20:32:45: maxmylyn changed owner from maxmylyn to antoine

[totaam]

2016-07-20 20:32:45: maxmylyn commented

---

Yes, it's still problematic as of r13052.

Most notably, you can see it by scrolling text-sites in Firefox, and entering text in text-boxes (Trac comments, yelling at people through the internet, etc etc), and by changing tabs.

It's better, but still nowhere as good as before the B-Frames started.

[totaam]

2016-07-20 21:00:00: antoine changed owner from antoine to maxmylyn

[totaam]

2016-07-20 21:00:00: antoine commented

---

It's better, but still nowhere as good as before the B-Frames started.
[[BR]]
Does it go away if you run the with b-frames disabled? (XPRA_B_FRAMES=0)

If so, then we're using b-frames for the main window area, and as per #1257#comment:3, this should only occur if the whole window is detected as a video area.
Can you confirm? Is the video region detection getting it wrong? (#410)

Using XPRA_VIDEO_SUBREGION=0 should have a similar effect: disabling video regions should now also disable b-frames.

[totaam]

2016-08-26 10:58:20: antoine commented

---

• converted to attachments so I can read comment:19
• r13474 rounds up the fps value which would cause the bencoder to choke otherwise
• the fact that the region fps is so low makes me think that there is a bottleneck before we get the damage events: either the application submitting events to X11, X11 itself, or us processing X11 events.. will have to try to figure out which - now tracked in low fps when vigorously mousing over the video #1299.

[totaam]

2016-09-07 06:20:51: antoine changed status from reopened to new

[totaam]

2016-09-07 06:20:51: antoine changed owner from antoine to afarr

[totaam]

2016-09-07 06:20:51: antoine commented

---

It is possible that when the fps drops lower than 10, we switch back to a non-video encoding whilst a b-frame is still due. This would cause the non-video frame to arrive before both the delayed b-frame and its auto-refresh... r13598 should fix this by excluding the video region whenever a b-frame is still pending.
There may still be a corner case: if the video region is moving on screen (ie: the page is scrolling), then we may end up excluding the region where it was rather than where it actually is... r13599 tries to prevent that.

---

This may be easier to test before #1299 is fixed (assuming it is our problem to fix).
This issue could also be triggered by playing a youtube video (high fps) then stopping it (the last few updates will be low fps), but it is going to be hard to spot: b-frames only happen 50% of the time and those last few frames are likely identical... a better option may be to use opengl paint box, or "-d compress" (server side) or "-d paint" (client side).

[totaam]

2016-09-09 12:31:36: antoine commented

---

See also #1232#comment:16

[totaam]

2016-09-16 17:44:01: maxmylyn commented

---

With an r13767 Fedora 23 Server and Client:

(As mentioned in the scrum Tuesday - wasn't in office to make a screenrecord)

• Navigating to [https://en.wikipedia.org/wiki/World_War_I] and grabbing the scroll bar and very very aggresively scrolling will eventually cause the whole window to be recognized as video (expected)

• Continued scrolling well after the quality gets lowered causes an odd corner case:

• If you then scroll with the mouse and stop scrolling, the heuristics will continue painting, but the last frame or so gets skipped, so the window keeps scrolling, and then when the refresh triggers, the window appears to "jump". It's reliably triggerable in Firefox and Chrome. I'll attach a screenrecord.

[totaam]

2016-09-16 17:49:17: maxmylyn commented

---

Okay, so the video I made is too large to attach here, so I uploaded it to Google drive - I'll paste the link into the bottom of this ticket.

Also of note:

• This behavior is not triggerable (despite much trying) with the flag XPRA_B_FRAMES=0 set client-side before connecting. (Disabling b-frames)

EDIT:

The actual link:

[https://drive.google.com/file/d/0B54bNUvCEvvrNXdDS3J3U2ZzNEE/view?usp=sharing]

[totaam]

2016-09-17 08:44:55: antoine changed owner from afarr to maxmylyn

[totaam]

2016-09-17 08:44:55: antoine commented

---

This behaviour may well be unavoidable to some extent: we don't know if and when a new frame is going to come through... so we schedule a flush of the video encoder just in case nothing comes (the delay varies, it will be longer when we're quite busy). r13773 improves this by also flushing before doing any kind of screen refresh. This should make things appear smoother: you'll get another intermediate b-frame before the lossless refresh.

Another fix: prior to r13775, the video encoder would only use b-frames if we detected "true video" before (re)creating the video encoder. The new code can toggle this on or off as the results from the heuristics change. This should result in b-frames being used in more cases, which may also make some issues more apparent..

PS: #1014 may make it more difficult to test b-frames at certain screen sizes, you may want to force "encoding=h264" if that's the case.

[totaam]

2016-09-20 00:10:45: maxmylyn changed owner from maxmylyn to antoine

[totaam]

2016-09-20 00:10:45: maxmylyn commented

---

r13773 seems to have fixed my corner case and I'm having a hard time finding another one, despite trying very vigorously.

As farr (heh) as I can tell, it's been fixed; even after several hours testing. Even Chrome which is very liberal with its painting is behaving well.

Passing back to you to decide what to do with this ticket.

[totaam]

2016-09-20 04:55:23: antoine changed status from new to closed

[totaam]

2016-09-20 04:55:23: antoine set resolution to fixed

[totaam]

2016-09-20 04:55:23: antoine commented

---

Thanks!
Closing at last.