PlatformDecoderModule::SupportsMimeType should accurately determine if the underlying decoder is usable.

RESOLVED FIXED in Firefox 43

Status

()

defect
P2
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

Trunk
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed, firefox44 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
All of the current PlatformDecoderModule::SupportsMimeType implementations only check for support by performing a lookup on the mimetype as a strink and always assume that the underlying decoder is usable and is working.

This is an invalid assumption ; especially with the WMF and FFMpeg decoder that may not have the required library installed (like no media framework installed in Windows N) or FFmpeg not compiled with some particular codec support (like if ffmpeg was compiled with --disable-nonfree).

We've put in place some work around in the MP4Decoder and MP3Decoder by attempting to create a platform decoder module and then creating a decoder and checking if that resulted in an error.

Ultimately, SupportsMimeType should only report true if the PlatformDecoderModule may actually decode them, if it knows that it can't with certainty then it should return false.
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Blocks: 1206977
Depends on: 1207019
Comment on attachment 8669537 [details] [diff] [review]
Have FFMpegDecoderModule properly return if a codec is supported.

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

::: dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp
@@ +244,5 @@
> +    sFFmpegInitDone = true;
> +  }
> +  return avcodec_find_decoder(aCodec);
> +}
> +  

Trailing space.
Attachment #8669537 - Flags: review?(cpearce) → review+
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/fe514a4a22a3
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
(Assignee)

Updated

4 years ago
Blocks: 1214943
(Assignee)

Comment 7

4 years ago
Comment on attachment 8673996 [details] [diff] [review]
Have FFMpegDecoderModule properly return if a codec is supported.

Approval Request Comment
[Feature/regressing bug #]: Making FFmpeg more robust
[User impact if declined]: Should FFmpeg be compiled without some particular codec, we would error 
[Describe test coverage new/current, TreeHerder]: in central for a few weeks
[Risks and why]: Low, just adding extra checks
[String/UUID change made/needed]: None
Attachment #8673996 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 8

4 years ago
Comment on attachment 8673996 [details] [diff] [review]
Have FFMpegDecoderModule properly return if a codec is supported.

we actually don't need a rebase ; forgot a patch in between
Attachment #8673996 - Attachment is obsolete: true
Attachment #8673996 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 9

4 years ago
Comment on attachment 8669537 [details] [diff] [review]
Have FFMpegDecoderModule properly return if a codec is supported.

Approval Request Comment
[Feature/regressing bug #]: Making FFmpeg more robust
[User impact if declined]: Should FFmpeg be compiled without some particular codec, we would error 
[Describe test coverage new/current, TreeHerder]: in central for a few weeks
[Risks and why]: Low, just adding extra checks
[String/UUID change made/needed]: None
Attachment #8669537 - Flags: approval-mozilla-aurora?
Comment on attachment 8669537 [details] [diff] [review]
Have FFMpegDecoderModule properly return if a codec is supported.

Adds error checking for ffmpeg support. OK to uplift to aurora.
Attachment #8669537 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1234157
You need to log in before you can comment on or make changes to this bug.