Closed
Bug 1341995
Opened 7 years ago
Closed 7 years ago
WebRTC: payload type of RED in stream is fixed as 122, which is not match the one in SDP.
Categories
(Core :: WebRTC: Networking, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: rzhang, Assigned: dminor)
Details
Attachments
(3 files)
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 Steps to reproduce: 1. Chrome canary 58 as caller, Firefox nightly 54 as callee. 2. RED+ulpFEC enabled by default. 3. The negotiated payload type of RED was 102. 4. Firefox sent RED stream with 122 payload type. 5. Chrome failed to decode the incoming stream. Actual results: Firefox always use the default payload type 122 as RED's payload type. It works fine if I hardcode the payload type of RED as 122 in caller side ( Chrome ). I tested Firefox 51 / Firefox DEV 53 / Firefox nightly 54. All have this issue. Expected results: The payload type of RED sent from Firefox should follow the negotiated one.
Assignee | ||
Comment 2•7 years ago
|
||
Yes, we set it here [1] to kRedPayloadType, which is defined as 122. It looks like we would have to add a red payload field to VideoCodecConfig (and I guess ulpfec while we are at it) and then populate those fields properly in the JsepCodecDescToCodecConfig function. We could then avoid using the constants. [1] http://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/media/webrtc/signaling/src/media-conduit/VideoConduit.cpp#667
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dminor)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dminor
Rank: 25
Priority: -- → P2
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
I checked this fix by manually testing with Chrome Canary.
Status: NEW → ASSIGNED
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8841621 [details] Bug 1341995 - Use negotiated values for RED and ULPFEC payload types; https://reviewboard.mozilla.org/r/115768/#review117154 ::: media/webrtc/signaling/src/jsep/JsepCodecDescription.h:276 (Diff revision 1) > + int redPt = (uint8_t)strtoul(redPayloadType.c_str(), nullptr, 10); > + if (redPt == 0 && redPayloadType != "0") { > + return; > + } > + > + int ulpfecPt = (uint8_t)strtoul(ulpfecPayloadType.c_str(), nullptr, 10); Let's use SdpHelper::GetPtAsInt here. ::: media/webrtc/signaling/src/jsep/JsepCodecDescription.h:748 (Diff revision 1) > + int mREDPayloadType; > + int mULPFECPayloadType; Let's use uint16_t or uint8_t here.
Attachment #8841621 -
Flags: review?(docfaraday)
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8841620 [details] Bug 1341995 - Make ULPFEC and RED payload types configurable in VideoCodecConfig; https://reviewboard.mozilla.org/r/115766/#review117274
Attachment #8841620 -
Flags: review?(rjesup) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8841621 [details] Bug 1341995 - Use negotiated values for RED and ULPFEC payload types; https://reviewboard.mozilla.org/r/115768/#review117522
Attachment #8841621 -
Flags: review?(docfaraday) → review+
Comment 11•7 years ago
|
||
Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/706398e37dbb Make ULPFEC and RED payload types configurable in VideoCodecConfig; r=jesup https://hg.mozilla.org/integration/autoland/rev/47c6c4caad99 Use negotiated values for RED and ULPFEC payload types; r=bwc
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/706398e37dbb https://hg.mozilla.org/mozilla-central/rev/47c6c4caad99
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•