Closed Bug 1064160 Opened 5 years ago Closed 5 years ago

Remove WaitForData from MediaSourceDecoder's public API.

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: kinetik, Assigned: kinetik)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
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.
Attachment #8485615 - Flags: review?(cajbir.bugzilla)
Blocks: MSE
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?
Attachment #8485615 - Flags: review?(cajbir.bugzilla) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/d911ec123f2e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.