1165772 - MediaSource incorrectly reporting that mp4 is supported, even when it isn't.

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Jean-Yves Avenard [:jya]]

On Linux, we can only use MediaSource with mp4 if FFmpeg support has been compiled in, and FFmpeg external libraries are present.

However, MediaSource IsTypeSupported only checks if the type is mp4 or webm and if that the HTMLMediaElement can play such file.

With gstreamer installed, we would report that mp4 was supported, even though gstreamer isn't usable with MSE.

MediaSource should only report MP4 as supported if the MP4Reader is usable.

Comment 1

[Chris Peterson [:cpeterson]]

Jean-Yves, does this bug affect Windows XP or only Linux builds with FFmpeg compiled in?

Comment 2

[Jean-Yves Avenard [:jya]]

AFAICT, Only Linux.

It affects plain linux build, you don't need ffmpeg built-in.

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: P3. Update web-platform-tests expected data.

We now have identical results between Windows XP and Linux.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: P1. Refactor DecoderTraits::CanHandleMediaType.

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: P2. Properly reports if mimetype is support in mediasource.

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: P3. Update web-platform-tests expected data.

We now have identical results between Windows XP and Linux.

Sorry for that, I had missed two tests.

Comment 7

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8636438 [details] [diff] [review]
P1. Refactor DecoderTraits::CanHandleMediaType.

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

Suggestion: We could make this more OO by having a class that contains the codec list that we run queries on. Not sure if that would be better overall.

I'm fine with landing this patch as-is.

::: dom/media/DecoderTraits.cpp
@@ +191,5 @@
>  
>  /* static */ bool
>  DecoderTraits::IsWebMType(const nsACString& aType)
>  {
> +#ifdef MOZ_WEBM

Suggestion: We can probably get rid of the MOZ_WEBM configure flag.

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8636439 [details] [diff] [review]
P2. Properly reports if mimetype is support in mediasource.

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

I suspect creating a class that represents the parsed codec information would make the code cleaner. However it is still an improvement over what we've got. I'm OK with landing this patch as is.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75bedbf6633b
https://hg.mozilla.org/integration/mozilla-inbound/rev/03b87d091470
https://hg.mozilla.org/integration/mozilla-inbound/rev/340872ac89ce

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/75bedbf6633b
https://hg.mozilla.org/mozilla-central/rev/03b87d091470
https://hg.mozilla.org/mozilla-central/rev/340872ac89ce

Comment 11

[KTucker [:KTucker][Inactive 3/4/2016]]

This landing might be causing bug 1186965 which is a smoke test blocker so we might need this backed out.

Comment 12

[Jean-Yves Avenard [:jya]]

Backing it out would be difficult, and I doubt this particular commit is what causes that assert. all that commit does is preventing MSE to report that MP4 is support on Linux.

More likely the new MSE.