Closed Bug 1533001 Opened 6 years ago Closed 1 year ago

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

Categories

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

66 Branch
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: dminor, Unassigned)

References

Details

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

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

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

Blocks: 1513000
Rank: 15
Priority: -- → P2

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.

Assignee: nobody → dminor
Status: NEW → ASSIGNED

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

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

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!

Assignee: dminor → nobody
Blocks: 1397539
Status: ASSIGNED → NEW
Flags: needinfo?(hankpeng)

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.

I'll take a look at this issue.

Flags: needinfo?(hankpeng)

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

Blocks: 1540251
Severity: normal → S3

OpenH264 isn't supported in Fenix.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.