1153049 - Move TrackConfig objects away from libstagefright

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

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

They need to be moved away and made to be content agnostic

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Part1. Merge MediaInfo and mp4_demuxer::TrackConfig

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

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Part2. Fix const-ness

Fix VideoData const-ness

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: Part3. Using AudioInfo/VideoInfo object

Replace mp4_demuxer's config objects with TrackInfo. This remove the last references to mp4_demuxer data object in fmp4 related code.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: Part4. Remove redundant mHasAudio/mHasVideo member

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

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Part5. Simplify retrieval of HTMLVideoElement dimensions

Simplify retrieval of element dimensions so it doesn't rely on initialization values set elsewhere.

Comment 6

[Jean-Yves Avenard [:jya]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=885df106effd

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Part5. Simplify retrieval of HTMLVideoElement dimensions

Rebase

Comment 8

[Jean-Yves Avenard [:jya]]

Attachment: Part4. Remove redundant mHasAudio/mHasVideo member

rebase

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Part4. Remove redundant mHasAudio/mHasVideo member

spotted an error in WMFReader

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

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...

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

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?

Comment 12

[Jean-Yves Avenard [:jya]]

(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."

Comment 13

[Jean-Yves Avenard [:jya]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/0a4a18ee6c82
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/3e617cb207c5
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/d6e188e8872e
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/8cde37caacef
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/65492fe83362

Comment 14

[Jean-Yves Avenard [:jya]]

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..."

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0a4a18ee6c82
https://hg.mozilla.org/mozilla-central/rev/3e617cb207c5
https://hg.mozilla.org/mozilla-central/rev/d6e188e8872e
https://hg.mozilla.org/mozilla-central/rev/8cde37caacef
https://hg.mozilla.org/mozilla-central/rev/65492fe83362
https://hg.mozilla.org/mozilla-central/rev/bf8582b32c79