Assert when appending an init segment after a partial media segment

RESOLVED FIXED in Firefox 42

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(3 attachments)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Created attachment 8644734 [details] [diff] [review]
[MSE] P1. Error when we detect invalid/incoherent data.
Attachment #8644734 - Flags: review?(gsquelart)
(Assignee)

Comment 2

3 years ago
Created attachment 8644735 [details] [diff] [review]
[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.
Attachment #8644735 - Flags: review?(gsquelart)
(Assignee)

Comment 3

3 years ago
Created attachment 8644736 [details] [diff] [review]
[MSE] P3. Add logging for when insertion index is reset.
Attachment #8644736 - Flags: review?(gsquelart)
(Assignee)

Updated

3 years ago
Blocks: 1191142
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.)
Attachment #8644734 - Flags: review?(gsquelart) → review+
Attachment #8644735 - Flags: review?(gsquelart) → review+
Attachment #8644736 - Flags: review?(gsquelart) → review+
https://hg.mozilla.org/mozilla-central/rev/b44aae5769e7
https://hg.mozilla.org/mozilla-central/rev/cadd7469af41
https://hg.mozilla.org/mozilla-central/rev/aab68c0d752c
Assignee: nobody → jyavenard
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.