WMFDecoderModule::SupportsMimeType() should check that it can create decoders before reporting that it supports a codec mimeType.

The MP4Decoder and WMF specific bits.

Comment on attachment 8669528 [details] [diff] [review] Patch: Ensure WMFDecoderModule::SupportsMimeType checks it can create decoders. Review of attachment 8669528 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comments: I think this patch should be split in two. The changes to the PDM itself (which is what causes the dependency to bug 1211335) and the changes to MP4Decoders and VideoUtils / DecoderTraits in the other. ::: dom/media/fmp4/MP4Decoder.cpp @@ +98,5 @@ > } > > /* static */ > bool > MP4Decoder::CanHandleMediaType(const nsACString& aType, should change the prototype here to be consistent with the header. @@ +145,5 @@ > + continue; > + } > + #ifndef MOZ_GONK_MEDIACODEC > + // Everything except B2G supports MP3 in MP4. > + if (codec.EqualsLiteral("mp3")) { this is more than likely no longer true (and has it ever been true?). MP4 or not is irrelevant as far the the PDM is concerned; it only deals with codecs ::: dom/media/platforms/wmf/WMFDecoderModule.cpp @@ +138,5 @@ > +template<const GUID& aGuid> > +static bool > +CanCreateWMFDecoder() > +{ > + static bool cachedResult = false; would be more elegant with a Maybe<bool>

Comment on attachment 8669528 [details] [diff] [review] Patch: Ensure WMFDecoderModule::SupportsMimeType checks it can create decoders. Review of attachment 8669528 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/fmp4/MP4Decoder.cpp @@ +53,5 @@ > return new MediaDecoderStateMachine(this, reader); > } > > static bool > +IsWhitelistedAudioCodec(const nsAString& aCodec) this is unused and gives me compilation warning (unused functions cause static analysis failure on try)

(In reply to Jean-Yves Avenard [:jya] from comment #2) > Comment on attachment 8669528 [details] [diff] [review] > @@ +145,5 @@ > > + continue; > > + } > > + #ifndef MOZ_GONK_MEDIACODEC > > + // Everything except B2G supports MP3 in MP4. > > + if (codec.EqualsLiteral("mp3")) { > > this is more than likely no longer true (and has it ever been true?). MP4 or > not is irrelevant as far the the PDM is concerned; it only deals with codecs GonkDecoderModule::SupportsMimeType() returns true for audio/mpeg. So we can probably not have the #ifdef guard here yes.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d34f694b13cf https://hg.mozilla.org/integration/mozilla-inbound/rev/91bff7bc67d7

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=15177504&repo=mozilla-inbound

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/e3f18ef6b5ce

We're failing web-platform tests because we're not returning negative for 'audio/mp4;codecs="avc1.4d001e"', i.e. a video codec in an audio container. This patch makes us return failure in that case. https://treeherder.mozilla.org/#/jobs?repo=try&revision=920b4557c62b

https://hg.mozilla.org/integration/mozilla-inbound/rev/b374a409750b https://hg.mozilla.org/integration/mozilla-inbound/rev/cf49f4d6c83c https://hg.mozilla.org/integration/mozilla-inbound/rev/dbe8d3254ccd

https://hg.mozilla.org/mozilla-central/rev/b374a409750b https://hg.mozilla.org/mozilla-central/rev/cf49f4d6c83c https://hg.mozilla.org/mozilla-central/rev/dbe8d3254ccd