-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix for devices that report incorrect supported types. #53
Conversation
Certain devices can report a codec in the supported list but don't really support it. getCapabilitiesForType will return IllegalArgument and we shouldn't add it to the list.
We observed this while testing on certain devices at Vine. (e.g. Samsung GT-I9100 running 4.1.2). The codec that's reported but not supported was " video/x-vnd.on2.vp8" getCapabilities will return IllegalArgument and if trying to use it later OMX will throw a ComponentNotFound error code and crash in native code. |
Is "video/x-vnd.on2.vp8" the mimeType that you're passing to the method to cause the crash? What's the name of the decoder (i.e. the value of codecName)? |
The name of the decoder that got resolved was something like "OMX.SEC.vp8" The memeType was resolved from a video file. |
Although we're seeing a crash when querying OMX.SEC.vp8, it's unclear whether or not it does actually support video/x-vnd.on2.vp8. Did you attempt to play the content using this decoder, and if so did it fail? Just trying to confirm the correct fix (which is either to ignore the decoder, or to hardcode that it does actually support the mime type). |
Is video/x-vnd.on2.vp8 supported? Yes. There are two codecs on the device that declares that it supports the mime type from MediaCodecList: OMX.SEC.vp8.dec and OMX.google.vpx.decoder The first one will throw the illegal argument, but 2nd one works just fine when get returned. |
When does OMX.SEC.vp8.dec throw the IllegalArgumentException? I understand it throws it when querying the capabilities (which is what this change guards against), but I was wondering if the decoder works if you just don't query what the capabilities are and try and instantiate the decoder anyway. I'm guessing the decoder is actually able to play VP8 content? |
Just tried it by "use it anyway" instead of looking further in that loop. It turns out, OMX.SEC.vp8.dec simply does not work, so it's probably better to skip it in this specific case, however, of course this is not to say that if failing in the getCapability call is 100% the decision signal to say that the codec is not a working codec.
|
Agreed. Blacklisting that decoder/device combination seems like the right thing to do. There's another decoder on that device that also fails. Are you aware of any other devices on which this decoder fails consistently? |
I don't think I had kept a list, probably should have by just logging it as a crash on Crashlytics, but we found this before we released that feature, so there's never really any crash for it. That device is bad all around on MediaCodec...but that's a separate issue and granted it is a Galaxy S2 and Galaxy S7 is coming out in a few months or so. The best solution might be just give up on these guys for now, but enforce better CTS tests on the upcoming devices. (I would totally vote to make ExoPlayer demo part of CTS tests..heh) |
@edisonw, we are adding a way to blacklist decoders to address issue #377. I'd be grateful if you could let me know the device fingerprint (output by 'adb shell getprop ro.build.fingerprint') on the phone where you saw this. I will need to compile a list of devices with the issue, so if you now know of any others please tell me about those too. Thanks! |
Closing this. We've added functionality to blacklist specific decoder/device combinations. We can add additional combinations to the blacklist as needed/reported. |
Certain devices can report a codec in the supported list but don't really support it. getCapabilitiesForType will return IllegalArgument and we shouldn't add it to the list.