google / ExoPlayer

This project is deprecated and stale. The latest ExoPlayer code is available in https://github.com/androidx/media
https://developer.android.com/media/media3/exoplayer
Apache License 2.0
21.73k stars 6.03k forks source link

Only be 'permissive' in decoder resolution check in certain circumstances #9142

Open icbaker opened 3 years ago

icbaker commented 3 years ago

https://github.com/google/ExoPlayer/issues/6551 reported an issue where ExoPlayer was failing to play H264 content because it was passing the 'display resolution' from an HLS manifest to android.media.MediaCodecInfo.VideoCodecCapabilities#areSizeAndRateSupported. In that case the 'display resolution' width was 1129 but the actual samples had a width of 1130. Odd widths are illegal in H264, so the codec reported it couldn't play the content.

The fix (https://github.com/google/ExoPlayer/commit/d874656e8a7668a83c6702413f094f6a1f509070) was to always round video dimensions based on [android.media.MediaCodecInfo.VideoCodecCapabilities#getHeightAlignment](https://developer.android.com/reference/android/media/MediaCodecInfo.VideoCapabilities#getHeightAlignment()) and [getWidthAlignment](https://developer.android.com/reference/android/media/MediaCodecInfo.VideoCapabilities#getWidthAlignment()).

Unfortunately this then caused some confusion in https://github.com/google/ExoPlayer/issues/9103 when a VP8 video with an odd height (405px) failed to decode but the log said format_supported=YES because the height had been rounded to 406px before querying the decoder. Removing the rounding didn't fix the playback error, but it changed the log to say format_supported=NO_EXCEEDS_CAPABILITIES, so it's at least a somewhat 'expected' failure.

Ideally we'd only do the rounding in cases the resolution represents something other than 'sample resolution'. i.e. if it comes from an HLS manifest, we should round. If it comes directly from a media container, we probably shouldn't.

This info probably needs to propagated through Format.

tidux commented 3 years ago

This is currently breaking WebM playback in browsers on the Galaxy S21 line.

icbaker commented 3 years ago

@tidux: I think there's possibly been a slight misunderstanding. This issue is marked low priority because it's tracking a change to ExoPlayer that only affects logging before a playback failure, it won't have an effect on whether playback succeeds or not (see #9103 for more details).

Pentaphon commented 3 years ago

it won't have an effect on whether playback succeeds or not (see #9103 for more details).

It absolutely keeps playback from succeeding. Webms affected by this issue DO NOT play at all and a toast error simply appears.

123141615-6d660d00-d40d-11eb-9121-cd2a8898ca7a

icbaker commented 3 years ago

It absolutely keeps playback from succeeding. Webms affected by this issue DO NOT play at all and a toast error simply appears.

I'm not disputing the videos with odd dimensions don't play with the c2.android.vp8.decoder codec - I agree with that. I'm saying that the changes covered by this issue won't change that. If we implement the enhancement suggested by this issue then playback of these videos will still fail with this codec. The only difference will be that the format will be logged as format_supported=NO_EXCEEDS_CAPABILITIES instead of the current format_supported=YES so the failure will be less surprising.

The fact that c2.android.vp8.decoder fails to play videos with odd dimensions is not related to ExoPlayer (we don't control the codec implementations). Given the code I found in https://github.com/google/ExoPlayer/issues/9103#issuecomment-867649077 it looks like a deliberate decision not to support odd-dimensioned videos.

Pentaphon commented 3 years ago

The fact that c2.android.vp8.decoder fails to play videos with odd dimensions is not related to ExoPlayer (we don't control the codec implementations). Given the code I found in #9103 (comment) it looks like a deliberate decision not to support odd-dimensioned videos.

if the issue is with an AOSP codec, why is this happening on some phones but not others? Isn't that basic part of AOSP just taken by OEMs and used as is? I doubt an OEM would mess with Google's codec implementation if they want their customers to have a consistent experience.

icbaker commented 3 years ago

if the issue is with an AOSP codec, why is this happening on some phones but not others? Isn't that basic part of AOSP just taken by OEMs and used as is? I doubt an OEM would mess with Google's codec implementation if they want their customers to have a consistent experience.

I'm not aware of any phones that are able to successfully play the video from #9103 with the c2.android.vp8.decoder. Some devices have alternative VP8 decoders, and so are able to successfully play the video using those, but then crash when they are manually ignored. This is what I saw with a Pixel 3a XL - with no modifications it played successfully using the c2.qti.vp8.decoder, but modifying the app to ignore that decoder resulted in it using c2.android.vp8.decoder and crashing: https://github.com/google/ExoPlayer/issues/9103#issuecomment-867649077

So I think the "happening on some phones but not others" is likely related to whether the phone has an alternative VP8 decoder that is used in preference to c2.android.vp8.decoder.

tidux commented 3 years ago

Would it be possible to mandate decoder fallback to handle broken hardware decoders? IIRC this is how mpv does it on desktop.

https://github.com/mpv-player/mpv/blob/master/video/decode/vd_lavc.c

baonq-2356 commented 3 years ago

@icbaker hi, whether this issue can fixed soon?

Pentaphon commented 3 years ago

So I think the "happening on some phones but not others" is likely related to whether the phone has an alternative VP8 decoder that is used in preference to c2.android.vp8.decoder.

Do you think there's any way for Exoplayer to "take over" in case an OEM has a buggy decoder (that they might never end up fixing due to short Android update commitments) in order to ensure proper playback across all devices? It seems to me that Google/AOSP/etc has an incentive to come up with a solution for these bad OEM practices since the end user is just going to "blame Android" instead of the OEM when a video doesn't play at all, or in our case, many videos don't play at all since webm is so widely used now.

icbaker commented 3 years ago

There are 3 interlinked issues here:

  1. c2.android.vp8.decoder doesn't support videos with odd resolutions. It declares this correctly (i.e. if you ask it whether it supports a video of 1280x719 then it will say "no"). This is tracked by:
    • 9103

  2. Codecs sometimes say "no we don't support that resolution" when in fact playing the video works fine. This is in fact the case for the c2.qti.vp8.decoder - it also claims to only support even-resolutioned videos, but as we've seen it can play odd-dimensioned videos fine. Therefore if ExoPlayer can't find any codecs that say "yes we support that resolution" we will try and play a video with one of the codecs that said it couldn't play it, because it might work. The alternative is to not play the video at all.
  3. ExoPlayer rounds the height & width before asking the codec if it supports a particular resolution. This means that for videos with an odd resolution, we may end up asking the codec if it supports an even resolution, and it will say "yes" but then when we try to play the video it will fail.
    • This leads to one real and one theoretical problem:
      1. The logging in the error incorrectly indicates that the codec said it supported the video.
      2. If a device has two codecs that both support the MIME type in question, but only one actually declares support for the odd-dimensioned video, we may end up choosing the one that should have said "no I don't support this video". This could result in playback failing on a device that could play the video if we selected a different codec. As I say, this is theoretical and since the only codec we're aware of that doesn't support odd-dimensioned VP8 is the c2.android one, and that is generally chosen last anyway if there are other decoders on the device.
    • This is tracked by this issue.

If you have questions relating specifically to (1), please comment on #9103. Please only comment on this issue if your question is specifically about (3). I will hide all off-topic comments on this issue to keep the discussion focussed.

icbaker commented 3 years ago

@baonq-2356 We have no plans to resolve this issue soon. As explained in various comments above it has no real-world effect on whether an odd-dimensioned VP8 video is playable on a device or not, it only changes the logging and is therefore not considered high priority.