Closed
Bug 930829
Opened 11 years ago
Closed 11 years ago
Create MediaData base class for Audio/VideoData classes
Categories
(Core :: Audio/Video, defect)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla27
People
(Reporter: cpearce, Assigned: cpearce)
References
Details
Attachments
(1 file)
(deleted),
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
I'd like to create a MediaData class that is a base class of AudioData and VideoData. This will mean we can move some of the duplicate fields of Audio/VideoData up into a common parent class, and will make it much easier to create a generic stream decoder abstraction for use in the new MP4 demuxer backend.
Assignee | ||
Comment 1•11 years ago
|
||
* Create MediaData, base class for Audio/VideoData.
* Moved offset, duration, timestamp, GetEndTime() into MediaData class.
* VideoData objects now have an inherited duration field, rather than an end time.
* All POD fields in *Data are now const.
* Created VideoData::ShallowCopyChangeDuration(VideoData*, duration), which creates a shallow copy of a VideoData with a different duration (since the MediaData::mDuration is now const, it can't be changed). The Fennec backend needs this. This does not copy the underlying image, so it's cheapish.
* I also changed the VideoData* in MediaPluginReader into nsAutoPtr<VideoData>.
* Green: https://tbpl.mozilla.org/?tree=Try&rev=fab79a9e1186
Attachment #822068 -
Flags: review?(kinetik)
Comment 2•11 years ago
|
||
Comment on attachment 822068 [details] [diff] [review]
Patch v1
Review of attachment 822068 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/media/MediaDecoderReader.h
@@ +100,5 @@
> +
> + enum Type {
> + AUDIO_SAMPLES = 0,
> + VIDEO_FRAME = 1
> + };
mType doesn't seem to be used. Can we get rid of this?
@@ +260,5 @@
> + // aOther's mImage, i.e. the Image is not copied. This function is useful
> + // in reader backends that can't determine the duration of a VideoData
> + // until the next frame is decoded, i.e. it's a way to change the const
> + // duration field on a VideoData.
> + static VideoData* ShallowCopyChangeDuration(VideoData* aOther,
ShallowCopyUpdateDuration maybe?
::: content/media/plugins/MediaPluginReader.cpp
@@ +256,5 @@
> mLastVideoFrame = v;
> continue;
> }
>
> + // Calculate the duration as the timestamp of the current frame minus the
trailing space
Attachment #822068 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Matthew Gregan [:kinetik] from comment #2)
> Comment on attachment 822068 [details] [diff] [review]
> Patch v1
>
> Review of attachment 822068 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: content/media/MediaDecoderReader.h
> @@ +100,5 @@
> > +
> > + enum Type {
> > + AUDIO_SAMPLES = 0,
> > + VIDEO_FRAME = 1
> > + };
>
> mType doesn't seem to be used. Can we get rid of this?
I'm using it in a later patch in my queue. I'm just trying to land stuff to reduce the chance of bitrot.
>
> @@ +260,5 @@
> > + // aOther's mImage, i.e. the Image is not copied. This function is useful
> > + // in reader backends that can't determine the duration of a VideoData
> > + // until the next frame is decoded, i.e. it's a way to change the const
> > + // duration field on a VideoData.
> > + static VideoData* ShallowCopyChangeDuration(VideoData* aOther,
>
> ShallowCopyUpdateDuration maybe?
OK.
>
> ::: content/media/plugins/MediaPluginReader.cpp
> @@ +256,5 @@
> > mLastVideoFrame = v;
> > continue;
> > }
> >
> > + // Calculate the duration as the timestamp of the current frame minus the
>
> trailing space
Thanks!
Assignee | ||
Comment 4•11 years ago
|
||
Landed for Firefox 27:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fd66c1fac23
Comment 5•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in
before you can comment on or make changes to this bug.
Description
•