1187076 - Assertion failure: mBitReader.numBitsLeft() [@mp4_demuxer::BitReader::ReadBits]

Status: RESOLVED FIXED

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

Description

[Tyson Smith [:tsmith]]

Attachment: log.txt

Assertion failure: mBitReader.numBitsLeft(), at /builds/slave/m-cen-l64-asan-d-0000000000000/build/src/media/libstagefright/binding/H264.cpp:29

Comment 1

[Tyson Smith [:tsmith]]

Attachment: test_case.mp4

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Need to make this a non-fatal assert.

Comment 3

[Jean-Yves Avenard [:jya]]

that line can be removed as we test for the size just after.

Comment 4

[Ralph Giles (:rillian)]

I don't think that's right. If we skip the assert, ReadBit() will return zero, the zero-count loop in ReadUE() can exhaust and we'd still MOZ_ASSERT(false) on line 74.

We either need some kind of error flag DecodeSPS can check at the end and report failure, or relax these to NS_WARNING so we still get debug information without failing the test. Depends how bad having zeroes in the SPS table is, I guess.

Comment 5

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Probably the best solution would be to change ByteReader to print warnings and set an error flag. We can check the error flag outside of ByteReader before the destructor gets called. It is an error if nobody has checked the error flag before calling the destructor and we could assert in that case.

Comment 6

[Ralph Giles (:rillian)]

Attachment: Convert assert to warning

Here's a minimal patch to convert the assert to a warning. Jean-Yves, is this what you had in mind?

Note I can't reproduce with the testcase, so this is based on looking at the code and the stack trace.

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8658521 [details] [diff] [review]
Convert assert to warning

Review of attachment 8658521 [details] [diff] [review]:
-----------------------------------------------------------------

yes..

ideally we should be able to handle error in SPS, the entire code assume it's always valid.

Comment 8

[Ralph Giles (:rillian)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c75f9ca74a29c05a3457d33531548c63f9b6d784
Bug 1187076 - Warn at end of SPS buffers. r=jya

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/c75f9ca74a29