1095106 - WebRTC video rendering FPS goes up to 20FPS only with V2.1

Status: RESOLVED WONTFIX

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

Description

[Jay]

[Blocking Requested - why for this release]:

Steps to reproduce:
1. Establish webRTC video call from device A to device B
2. Point device A's camera sensor to the brighter scene so the video is encoded @ 30FPS. This can be checked after enabling webRTC CSFlogging
3. When counting the number of painted frame with mozPaintedFrames on device B, it shows about 20 FPS.

Expected behavior:
It should be able to render @ 30FPS. The decoder statistics show that it's decoding @ 30FPS and CPU utilization is around 60% only so there is no reason that it can't be rendered @ 30FPS. In addition, V2.0 does support up to (25-28)FPS

Comment 1

[Jay]

Randell,

I tried the suggestions that you gave me over the IRC
This one http://pastebin.mozilla.org/7136834 with different duration helped to improve the rendering FPS. After I changed the duration to track rate/frame rate = 1000000/30 = 33333, the FPS goes up to 30FPS.

However, I am seeing the rendering freeze again in some condition. When I move the camera sensor between low light scene and high light scene few times during the call, the rendering stops after a while. 

Can you check if the patch have any side-effect?

Comment 2

[Jay]

Randell,

Any update on this?

Comment 3

[Maire Reavy [:mreavy]]

As the eng manager for WebRTC, I'm providing an update on blocking status:

This is a regression (compared to v2.0) that we're looking at.  It was introduced when we refactored MediaStreamGraph in Fx34 to remove delay, buffers, and the need to re-sample.

We are actively looking at this bug.  Jesup has a fix, but the fix has a problem.  He'll be providing a detailed update on where that fix stands later today.

I would love to fix this bug for v2.1 (and we're working to do so), but I'm not convinced we should hard-block on it.  20fps video on a mobile device is very usable, and when you consider real-world scenarios, the frame rate can get reduced to 20 fps or lower in several situations.  (As a simple example, the front camera on a Flame can not go higher than 16 fps;  it's a hardware limitation.)

Comment 4

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

Attachment: Fix max 20FPS remote WebRTC video on B2G in 2.1+

This seems to work well locally and has no lockups I've seen; Jay - please try on your 2.1 testbed

Comment 5

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

Jay: and verify if you see near 30fps instead of 20.  Thanks!

Comment 6

[Jay]

(In reply to Randell Jesup [:jesup] from comment #5)
> Jay: and verify if you see near 30fps instead of 20.  Thanks!

Tried the patch. It is little bit better. FPS goes around 22-26 but still lower than v2.0. In addition, I still see the screen freeze after 16mins.  It appears that the decoder still decoding the frame @ 28-30 FPS when freezing happens so it looks like the issue is on rendering.

Comment 7

[Paul Adenot (:padenot)]

Comment on attachment 8521621 [details] [diff] [review]
Fix max 20FPS remote WebRTC video on B2G in 2.1+

Review of attachment 8521621 [details] [diff] [review]:
-----------------------------------------------------------------

::: media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp
@@ +1562,5 @@
> +  AppendToTrack(image_, delta);
> +  if (delta2 > 0) {
> +    AppendToTrack(image2_, std::min(delta-1, USECS_PER_S/MAX_FPS));
> +    image_ = image2_.forget();
> +  }

Explain your logic in a comment block on top of that, please

Comment 8

[bhavana bajaj [:bajaj]]

Given this is a regression making a blocking call here.

Comment 9

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

Attachment: Fix max 20FPS remote WebRTC video on B2G in 2.1+

fix missed replacement of std::min() with delta2, and also do the image/image2 thing if we're passed a buffer and need to wrap it in an image

Comment 10

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

Attachment: Fix max 20FPS remote WebRTC video on B2G in 2.1+ (simpler, longer buffer)

jay - please give a try.  If FPS isn't good, please try 60 (maybe even 40) instead of 30 for MAX_FPS.

Comment 11

[Jay]

Ra(In reply to Randell Jesup [:jesup] from comment #10)
> Created attachment 8525558 [details] [diff] [review]
> Fix max 20FPS remote WebRTC video on B2G in 2.1+ (simpler, longer buffer)
> 
> jay - please give a try.  If FPS isn't good, please try 60 (maybe even 40)
> instead of 30 for MAX_FPS.

FPS looks good even with 30 for MAX_FPS. The rendering rate can reach 30fps. However, the screen freeze issue seems to be still there. But it takes much longer time to get to complete freeze.

Comment 12

[Sotaro Ikeda [:sotaro]]

(In reply to Jay from comment #11)
> 
> FPS looks good even with 30 for MAX_FPS. The rendering rate can reach 30fps.
> However, the screen freeze issue seems to be still there. But it takes much
> longer time to get to complete freeze.

I suspect that screen freeze issue could be caused by long running very high thread task like Bug 1087605.

Comment 13

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

(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> (In reply to Jay from comment #11)
> > 
> > FPS looks good even with 30 for MAX_FPS. The rendering rate can reach 30fps.
> > However, the screen freeze issue seems to be still there. But it takes much
> > longer time to get to complete freeze.
> 
> I suspect that screen freeze issue could be caused by long running very high
> thread task like Bug 1087605.

Perhaps, though that one has been fixed, and the queue it caused to overflow (by causing a 30-frame delay in decoding) has now been upped by the patch here to 300 frames.

Comment 14

[Josh Cheng [:josh]]

Dear Randell,
Could you help to raise approval and land this on 2.1? Thanks!

Comment 15

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

jay - can you retest with the current patch, especially given Sotaro's comment 12?  If we don't get the freeze, we're good to land I think.  I'll double-check for bitrot as well.

Comment 16

[Josh Cheng [:josh]]

Hi Vance,
Do we still need this for 2.1/2.1S?
Thanks!

Comment 17

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

No, don't need this one for 2.1/2.1s