Closed Bug 1183653 Opened 4 years ago Closed 4 years ago

MediaFormatReader should error if no usable tracks are found

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

Attachments

(1 file)

When attaching a video element to an audio-only HTML MediaElement, the video tracks should be ignored.
Assignee: nobody → jyavenard
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.
Attachment #8633480 - Flags: review?(cpearce) → review+
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?
Summary: MediaFormatReader doesn't ignore video track when using audio only HTMLElement → MediaFormatReader should error if no usable tracks are found
https://hg.mozilla.org/mozilla-central/rev/47193134162b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.