-
Notifications
You must be signed in to change notification settings - Fork 6k
-
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
setOutputSurface workaround needed for some more devices #4468
Comments
Also, I'm not sure if this is feasible or already being done, but is it possible (or even useful) to avoid dispatching/handling In the lifecycle event Thanks :) |
Addressing your second comment first, since it's easy to tick off:
I'm pretty sure this already happens. Which as an aside, implies that |
Addressing your first comment: We recently split the workaround you mention in two: Does your app ever switch the surface that it outputs too? If so, and if that case isn't failing, that would indicate that If you could answer the question above, that would be great. In the meantime we'll try and get hold of some of the devices listed and test them manually.
I don't think we can blacklist solely based on decoder name. SOC vendors tend to use the same decoder name across all of their products. So for example, any device that uses a MediaTek chipset will have a decoder named OMX.MTK.VIDEO.DECODER.AVC. However most likely only a subset of MediaTek chipsets need the workaround, and there's no public API on Android for querying what the chipset is. Still, the data provided is really useful for helping us to dig for a pattern. What we'll do on our side is:
After all that is done, we'll try and close the loop with you (i.e. check that some version you release in the future is not still logging these failures). |
Note to self: It was suggested in #3835 that all devices that use MT6737 and MT6753 chipsets may be affected by this issue. |
@qvk - Is it possible for you to add a new column to this data that shows more specific Android build information? I'm not sure what data you have exactly. Build fingerprints (
If you don't have that just the build name ( Some of the devices in your list appear to have passed the tests that are supposed to prevent this issue from occurring, however it's unclear whether the reports you're seeing are from devices that are running earlier builds than those which passed the tests. It would be good to check that as part of our cross-referencing. |
@ojw28 I suggest tracking all the devices that need any type of ExoPlayer workaround in a wiki page, here on GitHub. Doesn't have to be a public-edit wiki. |
@ojw28 Thanks for the help!
I believe this is present in
No. I guess this means there isn't enough information to use the more targeted
Sorry, we currently don't have Is the build name available inside a Also, I was wondering that if an application never needs to switch surfaces, is it ok to have |
Assuming it's relatively straightforward for you to add this to the data that's being logged, it would be great if you could! Unsure about the build name (by itself), but providing the full fingerprints would be ideal.
Yes, I think doing that would be fine for that use case. Alternatively, it's probably significantly easier for you just to call |
Some initial findings:
|
Since this is an ongoing problem, it's reasonable that we allow developers to toggle these workarounds without too much hassle. Issue: #4468 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=203364488
Issue: #4468 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=203479944
Where a device has this issue, it most likely applies to all SOC vendor provided decoders, which are normally the only video decoders other than the OMX.google software decoders on the device. The fact we currently only blacklist for AVC decoders is likely just a side effect of AVC's popularity. I checked two devices already on the blacklist, and both failed with other SOC vendor decoders on the device as well (but passed if forced to use OMX.google decoders instead) Issue: #4468 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=203728285
As part of this change: - Don't apply the workaround on API level 27+. GTS coverage should prevent such devices from existing. - Use Util.DEVICE consistently. - Remove the "// Device name", which don't really add much but make extra work when updating the list. Issue: #4468 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=204889657
@ojw28 we had the same issue on a Meizu Pro 7 phone (the device name is PRO7S). I added the device name inside the Could you add this device name too, please? |
Issue: #4468 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=205821059
Issue: #4468 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=205821059
This should have been largely addressed in 2.8.3. Please let us know if you see further devices experiencing this issue using 2.8.3 or later (a list with occurrence counts, similar to the one above, would be great). Thanks! |
Thanks! It might take a while to gather data with the 2.8.3 version though. |
We're seeing this issue since updating to v2.8.4, so it seems this issue is still present. We don't have a list of occurrence counts per device yet as we didn't have this problem with an older version of ExoPlayer, but we have quite a significant count of these Exceptions. On Fabric alone we can see around 10,000 to be exact. Following information is what I was able to extract from Fabric's logged Exceptions. We're using an instance of Devices and their API levels from last several Exceptions being thrown:
Example log:
|
@gapl - I think we need better data (see e.g. the kind of data in the original post of this issue). In particular occurrence counts for each device ( |
We've since implemented the workaround by always returning |
We're still adding the odd device to this list, but I think the bulk of the work here is done. We also made it easy for applications to override the method that decides whether the workaround is applied. I think it's fine to close at this point. |
@ojw28 does Google plan to make ExoPlayer's OEM testing a mandatory part of Android certification? That should make sure this list doesn't get any longer... |
A subset of the tests you refer to are already a mandatory part of Android certification, including the one covering this issue. |
That's what I was hoping for, thanks. |
Hi, we got customer complaints which point to a similar problem when decoding DRM streams.
Device info: Since this ticket is closed, should we enable the workaround for this device ourselves or do you guys still add devices to the list? |
We can enable the workaround for that device (end EML-L09, which looks identical). |
Perfect, thanks! |
Issue: #4468 PiperOrigin-RevId: 231759438
Issue: #4468 PiperOrigin-RevId: 231759438
Issue: google#4468 PiperOrigin-RevId: 231759438
Issue description
We're getting multiple records of
ExoPlaybackException
caused byIllegalArgumentException
(sometimesMediaCodec#CodecException
) whenMediaCodec#setOutputSurface
is called as seen in collected stacktraces. These happen when exiting the video activity, so I'm assumingSimpleExoPlayer
sends aMSG_SET_SURFACE
whensurfaceDestroyed
is invoked by the system, which eventually leads to the playback exception.We started logging decoder name, error stacktraces and device info inside
onPlayerError
callback only in recent releases, so the actual occurrences are likely much more than we could record.It sure doesn't look good adding all these devices to the workaround list, but there are only 4 or 5 unique decoder names in the list, so maybe there is some pattern here?
Reproduction steps
We don't have the affected devices on hand but according to our logs, the issue occurs upon exiting any activity which is playing media using exoplayer.
Version of ExoPlayer being used
2.7.0
Device(s) and version(s) of Android being used
These are data collected from last couple of weeks for cases where
onPlayerError
callback is received with the above mentioned exception.Many devices near the end of the list with very few occurrences might not warrant adding them to the exception list. Although they could be new devices just starting to grow in use.
A full bug report captured from the device
Unfortunately, we only noticed these errors from logs collected from remote user devices, so it's not possible to collect bug report using
adb
.However, the stacktrace collected in all cases (collected inside
onPlayerError
callback) is either of the following two:The text was updated successfully, but these errors were encountered: