1192097 - Assert when appending an init segment after a partial media segment

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Assertion failure: mInputBuffer->Length() >= length, at /Users/jyavenard/Work/Mozilla/mozilla-central/dom/media/mediasource/TrackBuffersManager.cpp:1069
#01: mozilla::TrackBuffersManager::CodedFrameProcessing()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x33beb1c]
#02: mozilla::TrackBuffersManager::SegmentParserLoop()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x33be237]
#03: mozilla::TrackBuffersManager::InitSegmentParserLoop()[/Users/jyavenard/Work/Mozilla/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x33ba44b]


This is typically corrupted data and we assert upon detecting it ...

However we should handle it more elegantly.

The MSE spec states that we should ignore any content that is invalid and remove it from the stream ; however this is not something we can do.

We should either return an error or stop parsing the media segment and jump into PARSING_INIT_SEGMENT state.

I don't think there's a proper way to do it.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] P1. Error when we detect invalid/incoherent data.

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] P2. Abort current operation when mediasource is detached.

Should a playback error occurs, the MediaSource element will be closed and will detach its sourcebuffers. Cancel any pending operations going in the sourcebuffers.

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] P3. Add logging for when insertion index is reset.

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8644734 [details] [diff] [review]
[MSE] P1. Error when we detect invalid/incoherent data.

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

::: dom/media/mediasource/TrackBuffersManager.cpp
@@ +1061,5 @@
>    if (mediaRange.IsNull()) {
>      AppendDataToCurrentInputBuffer(mInputBuffer);
>      mInputBuffer = nullptr;
>    } else {
> +    MOZ_ASSERT(mProcessedInput >= mInputBuffer->Length(), "Can't ever happen.");

Asserting should already mean "can't even happen", no need to display that!
(whereas the assert you had before was wrong because user-provided data could make it fail.)

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b44aae5769e7
https://hg.mozilla.org/integration/mozilla-inbound/rev/cadd7469af41
https://hg.mozilla.org/integration/mozilla-inbound/rev/aab68c0d752c

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b44aae5769e7
https://hg.mozilla.org/mozilla-central/rev/cadd7469af41
https://hg.mozilla.org/mozilla-central/rev/aab68c0d752c