1189602 - TrackBuffersManager not always notifying demuxer of new data

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

The TrackBuffersManager demux all data as they are available.

It does so by appending the incoming data to a SourceBufferResource and then calling the MediaDataDemuxer .

However, it doesn't always notify the demuxer whenever the SourceBufferResources has been modified.

It isn't a problem with the MP4Demuxer has it doesn't rely on this information to know which data is available in the MediaResource ; however the WebM demuxer does which could lead to only being notified of partial and incomplete data.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] Always notify demuxer when data is added (or removed) to the resource.

Comment 2

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

Comment on attachment 8641487 [details] [diff] [review]
[MSE] Always notify demuxer when data is added (or removed) to the resource.

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

r+ with nits.

::: dom/media/mediasource/TrackBuffersManager.cpp
@@ +780,5 @@
> +{
> +  MOZ_ASSERT(mCurrentInputBuffer);
> +  int64_t offset = mCurrentInputBuffer->GetLength();
> +  mCurrentInputBuffer->AppendData(aData);
> +  mInputDemuxer->NotifyDataArrived(aData->Length(), offset);

Length() returns a size_t but NotifyDataArrived() takes a uint32_t there. I'm guessing an overflow is practically impossible, but doesn't the compiler complain?

@@ +1032,4 @@
>    MediaByteRange mediaRange = mParser->MediaSegmentRange();
>    uint32_t length;
>    if (mediaRange.IsNull()) {
>      length = mInputBuffer->Length();

'length' is not needed in this block anymore.

@@ +1037,4 @@
>      mInputBuffer = nullptr;
>    } else {
>      // The mediaRange is offset by the init segment position previously added.
>      length = mediaRange.mEnd - (mProcessedInput - mInputBuffer->Length());

Declare |uint32_t length| (or whatever type is appropriate) here and remove the declaration at line 1033 above.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1e049536f283

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1e049536f283