1064160 - Remove WaitForData from MediaSourceDecoder's public API.

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Matthew Gregan [:kinetik]]

Rename NotifyGotData to slightly clearer NotifyTimeRangesChanged.  Move
WaitForData from MediaSourceDecoder's public API to MediaSourceReader's
private API and hoist the wait loop logic inside it.

Comment 1

[Matthew Gregan [:kinetik]]

Attachment: p1: Remove WaitForData from MediaSourceDecoder's public API.

Move NotifyGotData calls to TrackBuffer so they are closer to the state
being waited/notified on.

Rename NotifyGotData to slightly clearer NotifyTimeRangesChanged.  Move
WaitForData from MediaSourceDecoder's public API to MediaSourceReader's
private API and hoist the wait loop logic inside it.

Comment 2

[cajbir (:cajbir)]

Comment on attachment 8485615 [details] [diff] [review]
p1: Remove WaitForData from MediaSourceDecoder's public API.

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

::: content/media/mediasource/MediaSourceReader.h
@@ +111,5 @@
>  
>    bool CanUseAudioReader(MediaDecoderReader* aNewReader);
>    bool CanUseVideoReader(MediaDecoderReader* aNewReader);
>  
> +  void WaitForTimeRange(double aTime);

Add a comment explaining what this does and that the current implementation blocks.

::: content/media/mediasource/TrackBuffer.cpp
@@ +108,5 @@
>    SourceBufferResource* resource = mCurrentDecoder->GetResource();
>    // XXX: For future reference: NDA call must run on the main thread.
>    mCurrentDecoder->NotifyDataArrived(reinterpret_cast<const char*>(aData),
>                                       aLength, resource->GetLength());
>    resource->AppendData(aData, aLength);

Should these be swapped around? Reading resource->GetLength() for the offset before appending of course. It seems odd to notify of data arriving before appending the data. Can this affect anything else?

Comment 3

[Matthew Gregan [:kinetik]]

(In reply to cajbir (:cajbir) from comment #2)
> Comment on attachment 8485615 [details] [diff] [review]
> Should these be swapped around? Reading resource->GetLength() for the offset
> before appending of course. It seems odd to notify of data arriving before
> appending the data. Can this affect anything else?

Not sure it has any effect, but it'd be more logical in the order you suggest, so I'll swap them.

Comment 4

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d911ec123f2e

Comment 5

[Carsten Book [:Tomcat]]

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