xpeng reported on dev-media that G.722 is broken in 56. I can verify that it works in 55 but not in 56 or 57. Use the sample at [1] to test. I'm assuming it's fallout from the webrtc.org 57 update. [1] https://webrtc.github.io/samples/src/content/peerconnection/audio/

I'm also filing a bug for adding rudimentary mochitests of our supported codecs.

It looks like WEBRTC_CODEC_G722 is not set here [1] although a quick look at audio_coding.gypi and gyp.mozbuild makes it seem as though it should be. [1] http://searchfox.org/mozilla-central/rev/51b3d67a5ec1758bd2fe7d7b6e75ad6b6b5da223/media/webrtc/trunk/webrtc/modules/audio_coding/acm2/rent_a_codec.cc#253

Just FYI we do have signaling unit tests to ensure using/signaling G722 works. But we don't have any tests ensuring audio works. When reproduce this last night I noticed on about:webrtc no indication that RTP actually gets send or received (which also means a mochitest should/would have caught this easily :( ).

It appears that we may also not be properly enabling the RED codec based upon the lines just above this patch. If that is not expected, please let me know and I'll expand this patch to include RED as well.

Comment on attachment 8904573 [details] [diff] [review] Add G.722 defines to gyp file Review of attachment 8904573 [details] [diff] [review]: ----------------------------------------------------------------- We don't normally use RED yet, so that's probably ok for now.

Pushed by dminor@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b876f2c2f663 Fix G.722 audio codec; r=jesup

" Target: mozilla57" - Will the modification be merged into the coming firefox 56 ?

(In reply to xpeng from comment #9) > " Target: mozilla57" - Will the modification be merged into the coming > firefox 56 ? that just means it landed in 57.

Comment on attachment 8904573 [details] [diff] [review] Add G.722 defines to gyp file Approval Request Comment [Feature/Bug causing the regression]: bug 1341285 (webrtc.org 57 update) [User impact if declined]: g.722 isn't available. Various services webrtc interoperates with (like FreeSwitch and Asterisk) and conference mixers like Cisco Spark may use G.722 [Is this code covered by automated tests?]: A separate bug is adding mochitests (bug 1397872); probably that should be simultaneously uplifted (test-only patch). [Has the fix been verified in Nightly?]: Not yet but will be before landing [Needs manual test from QE? If yes, steps to reproduce]: Got to https://mozilla.github.io/webrtc-landing/pc_test.html. Start a test with G722 required. Unmute one of the large <video> elements to verify audio is flowing. [List of other uplifts needed for the feature/fix]: bug 1397872 [Is the change risky?]: No. This was simply left out in the 57 update [Why is the change risky/not risky?]: see above. There were no significant changes to g.722, we just weren't enabling it. [String changes made/needed]: none.

I suggest that it shoule be merged into firefox 56 , or many service will do not work. because they negotiate for G722 but it does not work in fact if use firefox 56

Hi Andreas, Can you help check if this issue was fixed in the latest nightly? Thanks.

(In reply to Gerry Chang [:gchang] from comment #13) > Hi Andreas, > Can you help check if this issue was fixed in the latest nightly? Thanks. Gerry I verified that the issue is fixed in Nightly 57.0a1 (2017-09-08).

Comment on attachment 8904573 [details] [diff] [review] Add G.722 defines to gyp file Fix a regression and was verified. Beta56+.