1183653 - MediaFormatReader should error if no usable tracks are found

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

When attaching a video element to an audio-only HTML MediaElement, the video tracks should be ignored.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: MediaFormatReader: returns error if no tracks are usable.

Comment 2

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

Comment on attachment 8633480 [details] [diff] [review]
MediaFormatReader: returns error if no tracks are usable.

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

This patch does what the patch comment says it does, and it does something sensible, but it doesn't do what the bug comments/title say needs to be done. In fact, it looks like videoActive already checks whether there's an ImageContainer on the MediaDecoder, so we should already have the behaviour of ignoring video tracks when there's an audio track.

Maybe you meant to update the bug title before uploading this patch?

::: dom/media/MediaFormatReader.cpp
@@ +368,5 @@
>        mMainThreadDemuxer->GetTrackDemuxer(TrackInfo::kAudioTrack, 0);
>      MOZ_ASSERT(mAudioTrackDemuxer);
>    }
>  
> +  if (!videoActive && !audioActive) {

Why don't you put this back after the block that initializes audioActive (line 322 for me)? It seems like nothing can change audioActive after that, and we could bail out sooner.

Comment 3

[Jean-Yves Avenard [:jya]]

We could have an audio track still. 

Though I'm not sure I understand your earlier comment. 

Yes, the current code ignores the video track if its an audio element. 
But we don't error readnetadata if we have no active tracks whatsoever. 


Which commit comment do you think it should be?

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/47193134162b

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47193134162b