Closed Bug 1333879 Opened 3 years ago Closed 6 months ago

Receiving multiple codecs in a SDP answer does not work

Categories

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

51 Branch
x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed
Blocking Flags:

People

(Reporter: shyamsadhwani, Assigned: bwc, NeedInfo)

References

Details

(Whiteboard: [need info reporter 2017-1-27])

Attachments

(3 files)

Attached file sdp.txt
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36

Steps to reproduce:

I have been working on a app which tries to use VP9 for webrtc video calls. Using Firefox on OSX 10.12.1.
When making a call from my app to Firefox, things work fine and VP9 gets used both ways. 
But when we make call from Firefox to my mobile app, I see that Firefox offers video codecs : VP8, VP9.
My app answers with VP9, VP8. Answer contains both codecs.
I can see the video in my app, but Firefox shows black video. 
Firefox is encoding to VP9 and my app can decode is properly.
My app is encoding to VP8 and firefox fails to display the video. I think firefox is expecting VP9 instead of VP8.
The same app works fine with Chrome and the order of codecs in SDPs is same in Chrome (offer: VP8,VP9, answer:VP9,VP8).  Stream going from app -> chrome is vp8 and from chrome -> app is VP9.
To me this looks like a bug in Firefox's implementation of WebRTC stack.
Attached is the file with SDPs and logs from Firefox.


Actual results:

Video on Firefox side shows black.


Expected results:

Video stream should be decoded properly and displayed correctly as in Chrome.
Component: Untriaged → WebRTC: Audio/Video
OS: Unspecified → Mac OS X
Product: Firefox → Core
Hardware: Unspecified → x86
Few more clarifications: Does FireFox assume that the video codec has to be same for send and receive? 
Chrome seems to work fine with mixed codec. When making call from Firefox to VP9Client, Firefox will offer SDP with (VP8, VP9); VP9Client will answer with SDP (VP9, VP8) (note the order is switched). When the connection is setup, Firefox encodes to VP9 and VP9Client is able to decode and see the video properly.
However VP9Client is encoding to VP8 and sending to Firefox, Firefox shows black video, I suspect its failing to decode, which could be because Firefox might be expecting VP9. 
The exact same scenario works with Chrome.
Can you verify the if the described scenario does not work in firefox?
Flags: needinfo?(drno)
Whiteboard: [need info drno 2017-1-27]
Yes, I have been using Firefox beta which has VP9 support and the scenario does not work completely. The video is not decoded/displayed in Firefox beta, on the other side things are fine. Local preview is fine on the Firefox side, but the remote video is not decoded/displayed. I suspect Firefox is expecting VP9 instead of VP8 and hence the decode fails.
On the app side, both preview and remote videos show up correctly.
Shyam: Can you try in Developer Edition or Nightly (53 or 54)?  Major changes to how codec configuration landed in 53.

Also: a log generated with NSPR_LOG_MODULES=signaling:5 NSPR_LOG_FILE=some_temp_log_file would help a bunch.  (those are env vars; if in windows, use the 'set' command from a shell then start firefox from the shell.

about:webrtc will also have some useful information
Flags: needinfo?(shyamsadhwani)
Flags: needinfo?(docfaraday)
Whiteboard: [need info drno 2017-1-27] → [need info drno and reporter 2017-1-27]
I don't know how to repro this, beside hacking Firefox into behaving like the described endpoint in this scenario.
I think the log file is hopefully the way easier solution to get an idea of what is going on here.
Flags: needinfo?(drno)
Whiteboard: [need info drno and reporter 2017-1-27] → [need info reporter 2017-1-27]
Flags: needinfo?(docfaraday)
Status: UNCONFIRMED → NEW
backlog: --- → webrtc/webaudio+
Rank: 19
Ever confirmed: true
Priority: -- → P1
Summary: Firefox 51.0b14 (64-bit) does now work properly for VP9 video calls → Receiving multiple codecs in a SDP answer does not work
Shyam I would like to ask you to test a try build with my patch once build has finished (I'll post a link to the executable in here).
Assignee: nobody → drno
Based on your initial bug report I assume you would want to test the Mac binaries: https://archive.mozilla.org/pub/firefox/try-builds/drno@ohlmeier.org-74a92c4d07c10f81399ce29f598ad2c69bc52510/try-macosx64/firefox-54.0a1.en-US.mac.dmg

Shyam could you download the build above and let me know if it fixes your problem?
Attachment #8832359 - Flags: review?(rjesup)
Comment on attachment 8832359 [details]
Bug 1333879: permit receiving multiple codecs.

https://reviewboard.mozilla.org/r/108692/#review110260

::: media/webrtc/signaling/src/jsep/JsepTrack.cpp:414
(Diff revision 1)
> -  // answer.  For now, remove all but the first codec unless the red codec
> +  // answer where we build the answer. If we received an answer we 
> +  // For now, remove all but the first codec unless the red codec

Remove "For now"?
Attachment #8832359 - Flags: review+
Thanks for providing the fix, unfortunately I am not able to run this app, I get error : “Nightly” is damaged and can’t be opened. You should eject the disk image.
I am on 10.12.1 macOS Sierra MacBook Pro (Retina, 13-inch, Early 2015)
nils - can we land this?  and how far can we uplift it?  It looks to be a pretty simple patch.

we could probably add a test by munging the offer to be vp8/vp9, then mung the response from vp8 to vp9 vp8, so one side will send in vp9, and the other side will send in vp8.  then verify the side receiving vp8 gets video.
Flags: needinfo?(drno)
Unfortunately no. Unit tests are screaming left and right with my last patch. And I remember that I did not see an easy solution to that back then.
Flags: needinfo?(drno)
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
My team has run into this recently. We have an SFU that bridges Firefox and Safari. The leg between Firefox and the SFU negotiates an m=video line that will transmit VP8 or H.264. Firefox offers first with support for VP8 and H.264; then, the SFU answers with support for VP8 and H.264; finally, in a subsequent round of negotiation, the SFU offers (again with VP8 and H.264); but, Firefox answers only with support for VP8.

I have two JSFiddles demonstrating this, one for Firefox and one for Chrome:

* Firefox: https://jsfiddle.net/yf7buzf0/
* Chrome: https://jsfiddle.net/o5qcd5Lz/

In the JSFiddle, I see Firefox 58 and 59 first offer VP8 and H.264, then only VP8. Chrome continues offering H.264 and VP8.
Hello everybody!!
It is also easily reproducible just running https://webrtc.github.io/samples/src/content/peerconnection/pc1/
You can see that the PeerConnection answerer only adds the preferred cocec in the SDP answer.
I hope this can help to test the issue in an easy way ;).
ping? Anyway to help getting this fixed?
I don't have the capacity any more to work on this any time soon.
Assignee: drno → nobody
Another jsfidle for this:

https://jsfiddle.net/7g2t5ryz/

It creates an offer, removes the first codec and sets it back to another PC, then restores the codec when setting the answer.

This way pcB sends vp9 to pcA and it fails. It works on chrome perfectly.
Assignee: nobody → docfaraday

The fiddle in comment 18 seems to be working for me, although there needs to be a fair bit of work to get red/ulpfec working properly (I don't think the intent is for us to support red by doing a single red encoding with all available codecs, and ulpfec, but that's what the pre-existing code tries to do).

Attachment #9064120 - Attachment description: Bug 1333879: (WIP) Handle multiple codecs in answer. → Bug 1333879: Handle multiple codecs in answer. r?mjf

Try looks good.

Pushed by bcampen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e420eb1ad7a
Handle multiple codecs in answer. r=mjf
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Duplicate of this bug: 814227
Regressions: 1581898
You need to log in before you can comment on or make changes to this bug.