Add ability for MediaFormatReader to detect change of stream content

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks 1 bug)

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
Currently, with the existing MSE architecture multiple MediaFormatReader will be created, one for each init segment received.

This allows to easily handle change of content as a MediaFormatReader handles a single content type.

Through the use of the SharedDecoderManager however, it is required that a MediaDataDecoder be able to handle change of stream transparently. This is currently done H264 by using the AnnexB format, and for decoders not supporting AnnexB by using the H264Wrapper that monitors the SPS content and destroy/create a new decoder on the fly.

With the new MediaSource architecture and the new MediaSourceDemuxer, we have a single instance of MediaFormatReader. It can pass samples from different stream
We need the MediaFormatReader to be able to easily detect if the sample if from a different stream than the preceding one and recreate a decoder on the fly.

By doing so, we remove the requirement on the MediaDataDecoder to have to handle content change, making the entire design much simpler.
(Assignee)

Comment 1

4 years ago
Add SharedTrackInfo object. A ref-counted TrackInfo. This will be added to all MediaRawData. Remove the size of the extradata array from the MediaRawData size calculation. As it's shared, there's no point having it counted multiple times.
Attachment #8617815 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Fill SharedTrackInfo data in samples returned by MSE.
Attachment #8617819 - Flags: review?(cpearce)
(Assignee)

Comment 3

4 years ago
Add SharedTrackInfo object. A ref-counted TrackInfo. This will be added to all MediaRawData. Remove the size of the extradata array from the MediaRawData size calculation. As it's shared, there's no point having it counted multiple times.
Attachment #8617829 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8617815 - Attachment is obsolete: true
Attachment #8617815 - Flags: review?(cpearce)
Comment on attachment 8617829 [details] [diff] [review]
P1. Add SharedTrackInfo object

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

::: dom/media/MediaInfo.h
@@ +360,5 @@
>    EncryptionInfo mCrypto;
>  };
>  
> +class SharedTrackInfo {
> +  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SharedTrackInfo)

As written, it looks like the visibility of all the methods here will be private, except that NS_INLINE_DECL_THREADSAFE_REFCOUNTING contains a "public" visibility declaration so they're not.

Please explicitly define the visibility of members here, i.e. please insert "public:" here, so it's obvious that these are public.
Attachment #8617829 - Flags: review?(cpearce) → review+
Comment on attachment 8617829 [details] [diff] [review]
P1. Add SharedTrackInfo object

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

::: dom/media/MediaInfo.h
@@ +374,5 @@
> +  {
> +    return mStreamSourceID;
> +  }
> +
> +  const TrackInfo* operator*() const

It kind of defeats the semantics of having a UniquePtr and then passing the raw pointer value out of the UniquePtr's scope; it means the pointer is no longer a unique reference to the object. Maybe you can make this return a reference to the UniquePtr instead?
Attachment #8617819 - Flags: review?(cpearce) → review+
(Assignee)

Comment 6

4 years ago
(In reply to Chris Pearce (:cpearce) from comment #5)
> Comment on attachment 8617829 [details] [diff] [review]
> P1. Add SharedTrackInfo object
> 
> Review of attachment 8617829 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/MediaInfo.h
> @@ +374,5 @@
> > +  {
> > +    return mStreamSourceID;
> > +  }
> > +
> > +  const TrackInfo* operator*() const
> 
> It kind of defeats the semantics of having a UniquePtr and then passing the
> raw pointer value out of the UniquePtr's scope; it means the pointer is no
> longer a unique reference to the object. Maybe you can make this return a
> reference to the UniquePtr instead?

We return a const pointer. You can't store that pointer to a non-const object. Could make it return a const TrackInfo* const to make it even more explicit, that not only the pointer is const, but so is the data pointed to.

This was mostly designed so you could use SharedTrackInfo in place of TrackInfo directly.
e.g. nsRefPtr<SharedTrackInfo> p; mInfo.mVideo = *p;
like we currently have:
nsAutoPtr<TrackInfo> p; mInfo.mVideo = *p;
(Assignee)

Updated

4 years ago
Blocks: 1171379
(Assignee)

Comment 7

4 years ago
Have MediaFormatReader detects change of stream content, recycle the decoder and seek to the previous keyframes, then drop all frames until we reach the time we want.
(Assignee)

Updated

4 years ago
Attachment #8622179 - Flags: review?(cpearce)
Attachment #8622179 - Flags: review?(cpearce) → review+
You need to log in before you can comment on or make changes to this bug.