Closed Bug 1153049 Opened 5 years ago Closed 5 years ago

Move TrackConfig objects away from libstagefright

Categories

(Core :: Audio/Video, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

Attachments

(5 files, 3 obsolete files)

The various Audio or Video configuration object are held in the MP4Demuxer.

They need to be moved away and made to be content agnostic
Blocks: 1153686
Refactor TrackInfo/VideoInfo/AudioInfo so it can replace mp4_demuxer::TrackConfig/VideoDecoderConfig/AudioDecoderConfig. Rename mOutputId member into mTrackId as that's how it's used everywhere
Attachment #8591498 - Flags: review?(cpearce)
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Fix VideoData const-ness
Attachment #8591499 - Flags: review?(cpearce)
Replace mp4_demuxer's config objects with TrackInfo. This remove the last references to mp4_demuxer data object in fmp4 related code.
Attachment #8591501 - Flags: review?(cpearce)
Remove redundant mHasAudio/mHasVideo from AudioInfo and VideoInfo, this also fixes bug 1153686 which could have resulted to rubbish audio info being presented to JS
Attachment #8591502 - Flags: review?(cpearce)
Simplify retrieval of element dimensions so it doesn't rely on initialization values set elsewhere.
Attachment #8591629 - Flags: review?(cpearce)
Attachment #8591629 - Attachment is obsolete: true
Attachment #8591629 - Flags: review?(cpearce)
rebase
Attachment #8592025 - Flags: review?(cpearce)
Attachment #8591502 - Attachment is obsolete: true
Attachment #8591502 - Flags: review?(cpearce)
spotted an error in WMFReader
Attachment #8592026 - Flags: review?(cpearce)
Attachment #8592025 - Attachment is obsolete: true
Attachment #8592025 - Flags: review?(cpearce)
Comment on attachment 8591498 [details] [diff] [review]
Part1. Merge MediaInfo and mp4_demuxer::TrackConfig

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

::: dom/media/MediaInfo.h
@@ +23,5 @@
> +    kAudioTrack,
> +    kVideoTrack,
> +    kTextTrack
> +  };
> +  TrackInfo(TrackType aType,

Make this explicit? Seems to be in fashion these days...
Attachment #8591498 - Flags: review?(cpearce) → review+
Attachment #8591499 - Flags: review?(cpearce) → review+
Comment on attachment 8591501 [details] [diff] [review]
Part3. Using AudioInfo/VideoInfo object

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

::: dom/media/fmp4/gonk/GonkVideoDecoderManager.h
@@ +39,5 @@
>  
>  public:
>    GonkVideoDecoderManager(MediaTaskQueue* aTaskQueue,
>                            mozilla::layers::ImageContainer* aImageContainer,
> +		                      const VideoInfo& aConfig);

Maybe you could fix the indentation while you're here?
Attachment #8591501 - Flags: review?(cpearce) → review+
Attachment #8592018 - Flags: review?(cpearce) → review+
Attachment #8592026 - Flags: review?(cpearce) → review+
(In reply to Chris Pearce (:cpearce) from comment #10)
> Comment on attachment 8591498 [details] [diff] [review]
> Part1. Merge MediaInfo and mp4_demuxer::TrackConfig
> 
> Review of attachment 8591498 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/MediaInfo.h
> @@ +23,5 @@
> > +    kAudioTrack,
> > +    kVideoTrack,
> > +    kTextTrack
> > +  };
> > +  TrackInfo(TrackType aType,
> 
> Make this explicit? Seems to be in fashion these days...

The static analyser is only choking on implicit one argument constructor. Though, I have in my queue something that remove all of that initialization and implement the TODO " TODO: TrackInfo should be initialized by its specific codec decoder."
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/bf8582b32c79

"I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE...I WILL NEVER MAKE LAST MINUTE CHANGE..."
Depends on: 1202012
You need to log in before you can comment on or make changes to this bug.