WebRTC FF38 attempts to set the maxFramerate to 0 on a H264 call where max_mbps is set.

RESOLVED FIXED in Firefox 38

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ehugg, Assigned: ehugg)

Tracking

unspecified
mozilla39
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

FF38 attempts to set the sending frame rate to 0 on an H264 call because of a calculation with max_mbps.

It has to do with WebrtcVideoConduit::SelectSendFrameRate here:
http://mxr.mozilla.org/mozilla-aurora/source/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#972

The framerate in parameter is 30, the current frame rate is 30, the mMaxMBPS is 27600, but it sets the mSendingFrameRate to 0 and attempts to SetSendCodec with that which fails.  If I comment out the line at 1006 that calls SetSendCodec then video is encoded and sent and the call works fine.

This can be reproduced by setting up a call in squared between FF36 and FF38.  Video will be seen on the FF38 side, but not encoded or sent.
Likely caused by bug 1127642
Depends on: 1127642
Assignee: nobody → ethanhugg
Status: NEW → ASSIGNED
Comment on attachment 8574223 [details] [diff] [review]
WebRTC check codec config max frame rate is set before using

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

The simplest fix seems to be to just check to make sure that the mCurSendCodecConfig->mMaxFramerate has been set.  This may be revisited in bug 1132318.
Attachment #8574223 - Flags: review?(rjesup)
Comment on attachment 8574223 [details] [diff] [review]
WebRTC check codec config max frame rate is set before using

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

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ +988,2 @@
>  
> +    if (mCurSendCodecConfig->mMaxFrameRate && mCurSendCodecConfig->mMaxFrameRate < mSendingFramerate) {

Add a "!= 0" to make it clearer, since it's not a ptr, and wrap at &&
Attachment #8574223 - Flags: review?(rjesup) → review+
Attachment #8574223 - Attachment is obsolete: true
Comment on attachment 8574243 [details] [diff] [review]
WebRTC check codec config max frame rate is set before using

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

bringing forward r+ from Randell.
Attachment #8574243 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/df5ecdd940d4
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8574243 [details] [diff] [review]
WebRTC check codec config max frame rate is set before using

Approval Request Comment

[Feature/regressing bug #]:
Caused by bug 1127642 which landed in FF38 shortly before uplift.

[User impact if declined]:
H264 WebRTC calls will fail if max_mbps is set.

[Describe test coverage new/current, TreeHerder]:
There are no tests in TreeHerder that use OpenH264.

[Risks and why]: 
This code is only exercised when H264 has been negotiated, so risks are limited to WebRTC H264 calls.

[String/UUID change made/needed]:
None
Attachment #8574243 - Flags: approval-mozilla-aurora?
Attachment #8574243 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.