Closed Bug 930829 Opened 11 years ago Closed 11 years ago

Create MediaData base class for Audio/VideoData classes

Categories

(Core :: Audio/Video, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla27

People

(Reporter: cpearce, Assigned: cpearce)

References

Details

Attachments

(1 file)

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.
Attached patch Patch v1 (deleted) — Splinter Review
* 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 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+
(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!
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Depends on: 1008785
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: