989944 - [B2G][WebRTC] Serious frame dropping when enabling HW H.264 video in real-time mode if the decoder has long delays

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[John Lin [:jhlin][:jolin]]

Apply patches in bug 911046 and start a H.264 video call. The frame rate is low.
This symptom won't occur if sender frame dropper is disabled.

Comment 1

[Maire Reavy [:mreavy]]

John -- Are we still seeing this problem?

Comment 2

[John Lin [:jhlin][:jolin]]

Yes, especially in flame to flame video call.
It seems that long latency in OMX H.264 decoding causes WebRTC to drop decoder output frames.
Test with larger kDecoderFrameMemoryLength value (http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/video_coding/main/source/generic_decoder.h#25) shows much better result.
Now we need to investigate why OMX decoder latency is long (usually > 400ms in my tests).

Comment 3

[Andreas Gal :gal]

This is the ideal kind of bug for the vendor to investigate. Please make sure they can reproduce and then optimize for us.

Comment 4

[:shell escalante]

Setting up call and emailed question on vendor plans to support low latency decode, which impacts the  2 usability bugs
Bug 989944 - [B2G][WebRTC] Serious frame dropping when enabling HW H.264 video in real-time mode.
Bug 989945 - [B2G][WebRTC] long video lag when using H.264 codec.
May be fixed with new firmware designed for KitKat.  Requested access.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Increase decode timestamp map size to handle delayed decode

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Note: the patch here wallpapers over a problem caused by excessive decoder delay in bug 989945 (an array of frames being decoded needs to be larger).  There's no reason we should modify the code permanently to handle second-plus decoder delays, so we don't want to land this patch.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

This is simply due to the long decode delay in bug 989945, so dupping to that.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Reopening since after review it's safe to land the increase in buffersize for the external encoder's timestamps (effectively it's a circular-buffersize for timestamp/frame correlation).  No perf hit and virtually no mem hit to increase it.

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8421325 [details] [diff] [review]
Increase decode timestamp map size to handle delayed decode

r+ (patch was by jhlin)

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3e6f92dca28
We want this on Aurora as well, though once we get kk-based 8x10 builds it should become moot

Comment 11

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8421325 [details] [diff] [review]
Increase decode timestamp map size to handle delayed decode

Approval Request Comment
[Feature/regressing bug #]: N/A

[User impact if declined]: Makes B2G 8x10 H.264 decode stall on builds without new DSP code that will only be available in KK-based builds.

[Describe test coverage new/current, TBPL]: Just landed on inbound; used extensively for months on Flame H.264 testing.

[Risks and why]: very low risk - simply increases the default size of a circular buffer used to match decoded frames with the inputs.  No perf downsize

[String/UUID change made/needed]: none

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8421325 [details] [diff] [review]
Increase decode timestamp map size to handle delayed decode

Looks very safe. Aurora+ even though this isn't on m-c yet.

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad5d65666cc5

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b3e6f92dca28