1064247 - FF32 generates invalid a=fmtp:0 profile-level-id fmtp

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Philipp Hancke]

reproduction (interop issue)
1) go to a room on talky.io (or apprtc) in Firefox
2) join the same room in chrome.
The call doesn't get established. While chrome://webrtc-internals doesn't show any errors, the setRemoteDescription error callback is called with "Failed to parse SessionDescription.  Failed to parse video codecs correctly."

Apparently chrome groks on
a=fmtp:0 profile-level-id=0x42e00c;packetization-mode=1
removing that from the offer helps.

http://googlechrome.github.io/webrtc/samples/web/content/munge-sdp/ should let you reproduce the fmtp:0 line easily. This doesn't happen in nightly.

I think chrome should ignore a=fmtp lines it can not parse, will file an issue in their bugtracker and link it here.

https://bugzilla.mozilla.org/show_bug.cgi?id=1042791 looks similar, but the id seems to be set there.

Comment 1

[Philipp Hancke]

Chrome issue report at https://code.google.com/p/webrtc/issues/detail?id=3790

Comment 2

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

The problem is caused by offering H.264 when 32 does not have H.264 support (except in FxOS 2.0 on platforms where HW OMX h.264 is enabled).

If the profiles has media.peerconnection.video.h264_enabled set to true the problem occurs.  (By default in 32 other than FxOS the pref doesn't appear even in about:config; you have to add it via New.)

Typically this happens if the user has manually set it to true.  It's possible (in theory) that running some version of 33/34 might have at some point set it without user interaction, but I doubt it.

We should guard against it, though, since 32 doesn't actually have h.264 support.

Comment 3

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

Attachment: only look at h264_enabled pref if HW OMX H.264 is enabled

Comment 4

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

Comment on attachment 8485746 [details] [diff] [review]
only look at h264_enabled pref if HW OMX H.264 is enabled

The other use of h264_enabled in 32 is in VcmSIPCCBinding.cpp, and is guarded by an #ifdef MOZ_WEBRTC_OMX.  Doing test build to verify.

Comment 5

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

Tracking as Maire requested that this bug be considered for a 32.0.1 ride along.

Comment 6

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

Removed tracking from 34/35, downgraded to ? for 33 until we verify that I'm right:

35 and I believe 34 do not show the bug when OpenH264 is explicitly disabled in AddOns, regardless of the state of the enable, though the code the patch modifies is still wrong.  The changes in VcmSIPCCBinding.cpp to support OpenH264 override the hard-coded value from PeerConnectionCtx.cpp.  33 likely is the same as 34/35 since it has pretty much all the OpenH264 fixes, but we should check.

If that flag-setting is now ignored, we should strongly consider removing it from PeerConnectionCtx.cpp in 35.

2.0 (based on 32) has OMX support, and has the pref but defaults to disabled unless the HW manufacturer's repo sets it, so this bug should not affect 2.0.

Comment 7

[Eric Rescorla (:ekr)]

I just tried with a brand new FF 32 and was able to replicate Jesup's
report from c2.

Philipp, can you try with a new profile and see if you can reproduce.

Comment 8

[Philipp Hancke]

ekr: yes, manually setting the flag to true on a fresh profile is the way to reproduce it.

Should have close to zero user impact fortunately

Comment 9

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

fix verified on a release build plus this patch, so it's available.  Since the code exists (and is wrong, though overridden) on 35, I'll land it there, but we should move to get rid of the hardcoded codec lists entirely.

Comment 10

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/0d9d104f8ba7

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0d9d104f8ba7

Comment 12

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

I spoke with Randell on irc. This bug has more limited impact than first suspected. As such, marking this as won't fix in 32.

Comment 13

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

verified 33 is unaffected