Currently, the 8x10 H.264 OMX encoder requires an IDR to change bitrates, which we do fairly continuously. We've worked around this with a complicated heuristic to send IDRs after a bitrate change with code that tries to control how often we send IDRs (since they normally cause 1-4 frames to be skipped, and hurt visual and perceived quality). This bug is for removing the restriction in the underlying 8x10 H.264 rate controller to allow us to adjust the bitrate without an IDR.

Hi, the rate controller is not implemented on chip? How much control, and from which files, do we have on the rate control algorithm? I thought that these SoC had the rate control algorithm implemented on its firmware. Also, this bitrate change is big and depends on network traffic I guess... And one more question. Why the IDRs are causing 4 frames dropped? Are they dropped at decoding or encoding? Is it a 8x10 performance limitation then? Can I see your bitrate control algorithm ? Thanks

These are largely questions for QC. We have no direct control of the actual bitrate algorithms, other than selecting variable or constant-with-skipped-frames, and setting the target bandwidth. The number of skipped frames on an IDR is also out of our control; I don't know if it varies depending on bandwidth, frame content (black frames shouldn't generate skips I hope), etc. The target bandwidth is selected by the rate-control mechanisms in the webrtc code, which monitors if delay appears to be increasing, decreasing or stable. The overall algorithm is complex; it can be found in the remote bitrate estimation code in meda/webrtc/trunk/webrtc.

[Blocking Requested - why for this release]: The latest DSP image removes the requirements on sending IDR to change bitrate. Can we remove the workaround from webRTC in v2.0 itself?

The meta build M8610AAAAANFYD1530.1 or later has this fix.

WIP patch, not tested yet

Hi Bhavana, could you please kindly help triage 2.0? on this ? Thank you very much.

Hi Jay, in order for the triage group to make a blocking decision we need to understand the user impact of the bug, what the risk is, and why we need to take that risk so late in the 2.0 cycle. Can you clarify further why this is required at this stage? Thanks!

The main impact is the user experience as birtate is set regularly every 1-2sec. This triggers to send IDR frame every 1-2sec. As 8x10 support constant-with-skipped-frames mode as mentioned in comment #2, it can translate to 3-4 frame drops every 1-2sec in order to maintain the target bitrate. Obviously, this can vary by multiple factor (target fps, target bitrate, resolution and scenes). Anyway, considering target FPS as 30 with 700kbps, 3-4frame drop can be 10% of the frame. In addition, in this case, we are taking out the code instead of adding new logic so I think the risk is relatively low.

Hi Bhavana, can you help to decide the blocking decision here? thanks.

Randell, I tested your patch with the latest meta here and it looks ok. Can we get the change landed?

Spoke offline with :jesup and blocking on this change given the codec fix.

Jesup, passing this onto you to back this hack across all branches, feel free to re-assign as needed..

Comment on attachment 8492373 [details] [diff] [review] Don't send IDRs to change bitrates or periodic ones to fix encoder errors Review of attachment 8492373 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/signaling/src/media-conduit/WebrtcOMXH264VideoCodec.h @@ +25,5 @@ > class OMXOutputDrain; > > // XXX see if we can reduce this > #define WEBRTC_OMX_H264_MIN_DECODE_BUFFERS 10 > +#define OMX_IDR_NEEDED_FOR_BITRATE 0 get rid of the 0 Remove/comment-out this line if we know we're running on the "right" version...

Comment on attachment 8492373 [details] [diff] [review] Don't send IDRs to change bitrates or periodic ones to fix encoder errors Per Maria Oteo, we have no restrictions on landing this. Once it lands, all H.264 teting MUST be done on flame 184 or later (or partner devices with DSP 1530.1 or later)

Comment on attachment 8492373 [details] [diff] [review] Don't send IDRs to change bitrates or periodic ones to fix encoder errors [Approval Request Comment] Bug caused by (feature/regressing bug #): N/A User impact if declined: Worse visual quality, frequent dropped frames (jerky) and more delay Testing completed: QC has tested this on 2.0 Risk to taking this patch (and alternatives if risky): Low so long as it runs on base builds with bug-fixed firmware (1530.1 and above). This includes Flame v184 builds (they have the right DSP firmware). String or UUID changes made by this patch: none.

https://hg.mozilla.org/releases/mozilla-aurora/rev/692d0f59a8fe https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/4540201b5da0