1533001 - Assertion failure in WebrtcGmpVideoCodec.cpp when using OpenH264 1.8.1 on Android

Status: RESOLVED WONTFIX

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

Description

[Dan Minor [:dminor]]

From the log:

03-06 09:47:00.825 17218 17322 F MOZ_Assert: Assertion failure: size != 0 && buffer + size <= end, at /home/dminor/src/fennec/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp:578

The assertion checks whether the plugin is returning extra bytes [1].

[1] https://searchfox.org/mozilla-central/rev/3e0f1d95fcf8832413457e3bec802113bdd1f8e8/media/webrtc/signaling/src/media-conduit/WebrtcGmpVideoCodec.cpp#578

Comment 1

[Dan Minor [:dminor]]

If I remove the assertion and the associated if statement video playback works as expected, so maybe this error checking is too strict.

Comment 2

[Dan Minor [:dminor]]

I added some logging, and we're getting a bad buffer size:

03-07 15:40:15.739 19928 20209 E >>>> : buffer: 0xe43ef004 size: 125 end: 0xe43ef081 bufferType: 4
03-07 15:40:15.757 19928 20209 E >>>> : buffer: 0xe4501004 size: 103 end: 0xe450106b bufferType: 4
03-07 15:40:15.784 19928 20209 E >>>> : buffer: 0xdcee1004 size: 443 end: 0xdcee11bf bufferType: 4
03-07 15:40:15.792 19928 20209 E >>>> : buffer: 0xe4501004 size: 16777216 end: 0xe4501041 bufferType: 4

Comment 3

[Dan Minor [:dminor]]

This does not reproduce with local builds of the openh264 plugin using gcc, the version of clang included with the NDK or the version of clang used to build Firefox. That points to a problem with the build configuration used on Taskcluster.

Comment 4

[Dan Minor [:dminor]]

It turns out that I was testing master on my local builds on Friday. It does reproduce if I build 1.8.1 locally. If I rebase the patches for clang support on top of the first commit labelled as 1.8.1 [1] it does not reproduce locally. It does reproduce if I base those patches on top of the last commit in 1.8.1 [2]. There are only a handful of commits in between, which are related to Bug 1397539. Of the two substantial commits, one has changes to gmp-openh264.cpp, and the other adds code to remove the requirement for C++11 to build on Android. I'll dig in further tomorrow.

[1] https://github.com/cisco/openh264/commit/a883d19f38a77dc5297e51072f17f2a0c6f66e60
[2] https://github.com/cisco/openh264/commit/2e1774ab6dc6c43debb0b5b628bdf122a391d521

Comment 5

[Dan Minor [:dminor]]

This reproduced on top of [1] if I rebase the clang patches on top of it.

To get it to build, I needed to change platform-android.mk to use the llvm_libc++ STL implementation, add -std=c++11 to the module Makefile, and I had to set NDKLEVEL=21. This avoids having to use the commits to remove the dependency on -std=c++11 that come after this commit in 1.8.1.

[1] https://github.com/cisco/openh264/commit/9cd2e7b5c4152cd1da2144235ec22c1543ebedf5

Comment 6

[Dan Minor [:dminor]]

This bug appears to have been introduced by the changes for Bug 1397539. Would it be possible for someone from Cisco to investigate this further. Please let me know if you need any help reproducing the bug or verifying a fix. Thank you!

Comment 7

[Byron Campen [:bwc]]

The bad buffer length in comment 2 happens to be equal to the value in the assertion here:

https://github.com/cisco/openh264/commit/9cd2e7b5c4152cd1da2144235ec22c1543ebedf5#diff-1a6d791e6763438cac728bb7d4b7900bR546

That's probably not a coincidence.

Comment 8

[Hank Peng]

I'll take a look at this issue.

Comment 9

[Dan Minor [:dminor]]

Just to be sure, I tested this again with the patch from Bug 1535766 applied and it still reproduces.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

OpenH264 isn't supported in Fenix.