Skip to content
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

MediaCodec initialisation fails. #3835

Closed
AntonAFA opened this issue Feb 12, 2018 · 58 comments
Closed

MediaCodec initialisation fails. #3835

AntonAFA opened this issue Feb 12, 2018 · 58 comments
Assignees

Comments

@AntonAFA
Copy link

Issue description

On certain devices that use OMX.MTK.VIDEO.DECODER.AVC decoder, on DRM content ONLY we receive the below error.

Reproduction steps

  • Initialise SimpleExoPlayer
  • Prepare MediaSource with drm.
  • Note, that in prepare step, we are NOT attaching SimpleExoplayerView to view hierarchy, and
    only doing so, after player state READY received.
  • Get the crash, immediately after prepare() called.

Link to test content

"uri": "https://storage.googleapis.com/wvmedia/cenc/h264/tears/tears.mpd"
"drm_license_url": "https://proxy.uat.widevine.com/proxy?provider=widevine_test"

Version of ExoPlayer being used

We can see this issue reproduced on Exoplayer 2.5.0 and above (including 2.6.1)
Previous Exoplayer versions work as expected.

Device(s) and version(s) of Android being used

Device - Meizu M5C, Lenovo K4 and Moto C+ (all of them with OMX.MTK.VIDEO.DECODER.AVC)
Android version - 6.0

### A full bug report captured from the device

com.google.android.exoplayer2.ExoPlaybackException
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.throwDecoderInitError(MediaCodecRenderer.java:418)
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.maybeInitCodec(MediaCodecRenderer.java:405)
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.onInputFormatChanged(MediaCodecRenderer.java:839)
    at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.onInputFormatChanged(MediaCodecVideoRenderer.java:455)
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:536)
    at com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:560)
    at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:306)
    at android.os.Handler.dispatchMessage(Handler.java:107)
    at android.os.Looper.loop(Looper.java:207)
    at android.os.HandlerThread.run(HandlerThread.java:61)
 Caused by: com.google.android.exoplayer2.mediacodec.MediaCodecRenderer$DecoderInitializationException: Decoder init failed: OMX.MTK.VIDEO.DECODER.AVC, Format(1, null, video/avc, 720
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.maybeInitCodec(MediaCodecRenderer.java:405) 
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.onInputFormatChanged(MediaCodecRenderer.java:839) 
    at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.onInputFormatChanged(MediaCodecVideoRenderer.java:455) 
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:536) 
    at com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:560) 
    at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:306) 
    at android.os.Handler.dispatchMessage(Handler.java:107) 
    at android.os.Looper.loop(Looper.java:207) 
    at android.os.HandlerThread.run(HandlerThread.java:61) 
 Caused by: android.media.MediaCodec$CodecException: start failed
    at android.media.MediaCodec.native_start(Native Method)
    at android.media.MediaCodec.start(MediaCodec.java:1898)
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.maybeInitCodec(MediaCodecRenderer.java:397)
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.onInputFormatChanged(MediaCodecRenderer.java:839) 
    at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.onInputFormatChanged(MediaCodecVideoRenderer.java:455) 
    at com.google.android.exoplayer2.mediacodec.MediaCodecRenderer.render(MediaCodecRenderer.java:536) 
    at com.google.android.exoplayer2.ExoPlayerImplInternal.doSomeWork(ExoPlayerImplInternal.java:560) 
    at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:306) 
    at android.os.Handler.dispatchMessage(Handler.java:107) 
    at android.os.Looper.loop(Looper.java:207) 
    at android.os.HandlerThread.run(HandlerThread.java:61) 
@tonihei
Copy link
Collaborator

tonihei commented Feb 12, 2018

Was able to reproduce your problem on a Moto C+. It seems the device has problems switching output surfaces and needs a workaround. This is similar to issues with other devices we had in the past (e.g. #3355, #3439).

Can you provide us with the Build.DEVICE names of Meizu M5C and Lenovo K4 to add them to our workaround?

@tonihei tonihei self-assigned this Feb 12, 2018
@AntonAFA
Copy link
Author

As i see you already support something related to OMX.MTK codec in your workaround. Or you talking about some other workaround?
Will provide you with device info tomorrow.

@tonihei
Copy link
Collaborator

tonihei commented Feb 13, 2018

Yes, the existing workaround checks decoder name and device name to be as restrictive as possible (see here). That's why I need the device names of the devices where it doesn't work to add to this list.

@AntonAFA
Copy link
Author

AntonAFA commented Feb 13, 2018

Here is the device names(same as appear in your Util.DEVICE)
Meizu M5C - M5c
Lenovo K4 Note - A7010a48

Anyway, I tried to change the codecNeedsSetOutputSurfaceWorkaround(String name), so that Meizu M5c will be included in the list of workaround needed devices. The result was the same as in reported bug. Even if I force codecNeedsSetOutputSurfaceWorkaround always return true, it is still crashing. Do you manage to reslove issue on Moto C++ by adding it to the workaround list?

Is there something more that I need to do in order to apply this workaround on Meizu?

@tonihei
Copy link
Collaborator

tonihei commented Feb 13, 2018

Yes, I added "panell_s".equals(Util.DEVICE) to the workaround list and this solved the issue for the Moto C+.

Were you able to test if it works for the Lenovo K4?

@AntonAFA
Copy link
Author

First of all, that is good news, because one of the devices our customer reported on, was Moto C+. So I will check it with them.
I am testing it on Meizu. Unfortunately I have no Lenovo K4 around me. As I mentioned above, even i force hardcoded codecNeedsSetOutputSurfaceWorkaround always return true and compile on Meizu it does not have any effect. Is there anything that I probably missing?

@tonihei
Copy link
Collaborator

tonihei commented Feb 13, 2018

Not if it's exactly the same issue.

Unfortunately, I don't have access to neither Meizu M5c nor Lenovo K4, so it's difficult to check what may cause it. Are the reproduction steps and the error message above the same for all three devices? Or is there something else?

@AntonAFA
Copy link
Author

AntonAFA commented Feb 13, 2018

Yes, the flow is the same.
For the matter of fact, I even dont see that exoplayer enter this code block.
getCodecInfo always return null, so dummySurface never initialised.

private void setSurface(Surface surface) throws ExoPlaybackException {
if (surface == null) {
      // Use a dummy surface if possible.
      if (dummySurface != null) {
        surface = dummySurface;
      } else {
        MediaCodecInfo codecInfo = getCodecInfo(); // getCodecInfo always return null, so dummySurface never initialised.
        if (codecInfo != null && shouldUseDummySurface(codecInfo.secure)) {
          dummySurface = DummySurface.newInstanceV17(context, codecInfo.secure);
          surface = dummySurface;
        }
      }
    }
}

@AntonAFA
Copy link
Author

AntonAFA commented Feb 13, 2018

maybeInitCodec() always happened only after setSurface(Surface surface) was called two times
and the incoming Surface is always null.
Actually, onCodecInitialized(String name, long initializedTimestampMs,
long initializationDurationMs) does not happen also.

@tonihei
Copy link
Collaborator

tonihei commented Feb 13, 2018

Can you provide more details on why setSurface is called two times? It's usually only called once when you attach the UI.

@AntonAFA
Copy link
Author

setSurface was called twice because we used old exoplayer api where you was supposed to call player.setSurfaceView(null) before you apply new surface (in order to clean the previous one. Now you are doing it with player.clearVideoSurfaceView() so the surface can be removed by SurfaceHolder) I updated our api to be aligned with yours. Now setSurface() called only once. But the issue still there.

@AntonAFA
Copy link
Author

Another thing that I see is that setOutputSurfaceV23(codec, surface); never happened.
surface is null, so if (this.surface != surface) will never enter the block where you actually apply the workaround.
From what I see the first time DummySurface created is in configureCodec(MediaCodecInfo codecInfo, MediaCodec codec, Format format, MediaCrypto crypto){ }

with if (dummySurface == null) { dummySurface = DummySurface.newInstanceV17(context, codecInfo.secure); }

But I think it is too late, because almost immediately called codec.start(); that actually crash the player

@tonihei
Copy link
Collaborator

tonihei commented Feb 13, 2018

With the workaround in place, dummySurface should never be created. The creation in configureCodec is guarded by a call to shouldInitCodec which also checks whether this workaround is needed.

When exactly do you attach the UI? I added a listener to onPlayerStateChanged and when the playback state changes to Player.STATE_READY for the first time, the UI is attached. When doing this, the codec.start method is called as a result of setSurface and not because onInputFormatChanged as in your case. There seems to be a difference in the way I try to reproduce the problem.

@AntonAFA
Copy link
Author

So in order to be maximum aligned with you I have moved to work on your demo application.
I want to be sure that we dont miss anything.
I did some small changes in player_activity.xml and PlayerActivity.java in order to reproduce issue on my device.
Changes I made: player_activity.xml ---> I comment out this block

<com.google.android.exoplayer2.ui.SimpleExoPlayerView android:id="@+id/player_view"
      android:layout_width="match_parent"
      android:layout_height="match_parent"/>

and added this view instead:

<FrameLayout android:id="@+id/test_view"
      android:layout_width="match_parent"
      android:layout_height="match_parent"/>

This should simulate my parent view to which I attach player view.

In PlayerActivity In onCreate()

  • I get the reference to the test_view,
    testRootView = findViewById(R.id.test_view);

  • create SimpleExoplayerView programmatically
    simpleExoPlayerView = new SimpleExoPlayerView(this);

  • subscribe to STATE_READY and when it received add simpleExoPlayerView to testRootView

if(playbackState == Player.STATE_READY) {
        testRootView.addView(simpleExoPlayerView);
      }

Also I added Meizu device name in MediaCodecVideoRenederer. Workaround method now looks like that:

   private static boolean codecNeedsSetOutputSurfaceWorkaround(String name) {
        // Work around https://github.com/google/ExoPlayer/issues/3236,
        // https://github.com/google/ExoPlayer/issues/3355 and
        // https://github.com/google/ExoPlayer/issues/3439.
        return (("deb".equals(Util.DEVICE) || "flo".equals(Util.DEVICE))
                && "OMX.qcom.video.decoder.avc".equals(name))
                || (("tcl_eu".equals(Util.DEVICE) || "SVP-DTV15".equals(Util.DEVICE)
                || "BRAVIA_ATV2".equals(Util.DEVICE))
                && "OMX.MTK.VIDEO.DECODER.AVC".equals(name)
                || "M5c".equals(Util.DEVICE) && "OMX.MTK.VIDEO.DECODER.AVC".equals(name));
    }

From what I see from debugging the code flow looks like that:

  • setSurface(Surface surface) called with surface null, so we exit this function without doing anything.
  • We get in onInputFormatChanged(Format newFormat) with audio codec first. so we continue to maybeInitCodec() and exiting this function ok.
  • We get onInputFormatChanged(Format newFormat) once again. This time with video codec. And going into maybeInitCodec()
  • shouldInitCodec() returns true. surface != null is false but shouldUseDummySurface(codecInfo.secure); is true. So the statement is true.
  • shouldUseDummySurface() returns true. Here all the statements return true except DummySurface.isSecureSupported(context) that returns false. but because of || sign it is enough that !codecIsSecure return true for this statement to be also true.
  • So then we going into configureCodec() (all the workaround flags in maybeInitCodec() are set to false). in configureCodec() the surface is null. Assertion not throwing exception. dummySurface instantiated.
dummySurface = DummySurface.newInstanceV17(context, codecInfo.secure);
  • And from there we continue to codec.start(); and Boom! exception thrown.

You say that dummySurface should never be created in a proper workaround mode. But in my case it happens to work in exact that way. Also the codec.start is called not from setSurface. Any idea for reason why it can happen? Note, that I in purpose reproduce this issue in your demo app with your drm content in order to be maximum aligned to your workspace. exoplayer branch is release-v2

@AntonAFA
Copy link
Author

AntonAFA commented Feb 14, 2018

It will be grate, if you add Meizu and Lenovo K4 devices to the list of excluded devices and push it on some side branch, that I can check if this fix apply for them. I am quite sure, that there is something that I am missing.

Here is the device names(same as appear in your Util.DEVICE)
Meizu M5C - M5c
Lenovo K4 Note - A7010a48

@tonihei
Copy link
Collaborator

tonihei commented Feb 14, 2018

Thanks a lot for this detailed analysis! Not many people make this effort to help us debug a problem :)

It turns out that there is a difference I didn't know about (thanks to @ojw28 for pointing this out). The original fix for other devices (#3439) included some further changes to prevent the DummySurface from being instantiated. Please have a look at this commit to see the details. I guess it will work when you add this change to your code (especially that shoulduseDummySurface also checks for the workaround).

I'll add the Meizu M5C and the Lenovo K4 to our list.

@AntonAFA
Copy link
Author

Thanks for your solution. I made the changes you suggest and it seems to work both on Meizu M5c and Lenovo K4 Note.
For some reason it did not work on Moto C+ that we tested on. When I print it Util.DEVICE, I could see that the name is slightly differ from the one you provide.
You told that it is "panell_s" , but in my case the name was "panell_p". Probably there is some minor difference in devices, and therefore they have different names. Anyway, adding "panell_p" to the device list solved the issue.
What I am wondering now, is how many devices out there that we does not know about them having this problem. Is there more general solution in plan, then just excluding device-device? Also, I observed that all of the our problematic devices have same chipset and codec (just to take it in your consideration). Maybe there we can detect some patterns and exclude devices by them and not by name.
Another thing I want to ask you about, is your release plans. I am not asking for exact date, just want to know if for now I should think about some temporary solution, or just wait for your release.

Thank you very much for your support.

@tonihei
Copy link
Collaborator

tonihei commented Feb 14, 2018

Yes, there is also panell_d, panell_dl and panell_dp. Guess I better change it to include all devices starting with "panell" then.

We also discussed the same question - whether we should enable the workaround for certain API levels and decoders for all devices or something similar. Because we also don't know for sure how many devices out there have this problem. Using the chipset may be a good idea, need to think about this a little bit further.

For our release plans, there is probably a new release in the next couple of days.

@AntonAFA
Copy link
Author

Thanks again.
I was thinking about possible solutions, and came to a little bit different possible approach. If you provide some sort of Settings API that can give application (me) ability to decide if I want this workaround or not for example: player.useDummySurfaceWorkaround(true) , I can catch this specific exception once and enable workaround for all previous sessions for this device. This will release you from responsibility of tracking all the problematic devices and adding them to the endless workaround list. On my side I can also relax and forget about this bug :)
The only thing I am worried about is this exception unique for this sort of behaviour, or it can be thrown in some other circumstances?

@tonihei
Copy link
Collaborator

tonihei commented Feb 14, 2018

This problem often results in the application not responding on these devices instead of an exception. Especially when switching between two real surfaces. That means we can't just always try first and enable the workaround as soon as it fails.

If you want to catch the exception and then set a flag in your app, you can still do so by overriding shouldInitCodec in MediaCodecVideoRenderer to disable initializing the dummy surface if surface == null. This extended renderer can then be used in ExoPlayer by overriding the buildVideoRenderers in DefaultRenderersFactory to return your renderer. And then pass in this renderers factory to ExoPlayerFactory.newSimpleInstance.

ojw28 pushed a commit that referenced this issue Feb 16, 2018
Issue:#3835

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185501181
ojw28 pushed a commit that referenced this issue Feb 16, 2018
Also added comments for all existing devices for easier reference.

Issue:#3835
Issue:#3236

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185661900
ojw28 pushed a commit that referenced this issue Feb 20, 2018
Issue:#3835

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185501181
ojw28 pushed a commit that referenced this issue Feb 20, 2018
Also added comments for all existing devices for easier reference.

Issue:#3835
Issue:#3236

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=185661900
@noamtamim
Copy link
Contributor

Hi @tonihei and @ojw28, thank you for fixing this issue.
I want to express my concerns about the "device-specific workaround" approach.
What we did (I work with @AntonAFA) is the following:

  • A customer has reported this issue on Moto C+ and Lenovo K4 Note
  • We wanted to reproduce the issue locally, so we looked online for devices we can buy that have the same chipset as those devices (MT6737 and MT6753, respectively). The reported devices don't sell here, and it was sort-of a shot in the dark.
  • We found that Meizu M5c has the same chipset as the Moto C+, so we bought it and easily reproduced the issue.

But there are many other phones with the same chipset -- we just chose one that was easy to get. So either it was just a lucky guess, or all those other phones (which are not explicitly named by ExoPlayer) will also fail.
In addition - as mentioned above, the Lenovo K4 Note has a different MediaTek chipset (albeit in the same family, MT67xx), and it fails in the same way.

Just my 2 cents.

@ojw28
Copy link
Contributor

ojw28 commented Feb 25, 2018

We agree that the current approach isn't doing a good job of targeting all affected devices. Although on the plus side, it's likely that we're targeting the popular ones; there are probably quite a lot of devices that barely anyone is using.

An approach that targets the chipset would be preferable, but I'm not sure how to do that. I don't think there's anything suitable in Build. Do you have any idea how we'd be able to target the workaround to a specific chipset?

@noamtamim
Copy link
Contributor

noamtamim commented Feb 26, 2018

Hi @ojw28,

From what I found, the best (read: least bad) source of the chipset id is android.os.SystemProperties.get("ro.board.platform") (a private API, which is why it's bad). For the Meizu M5c it gives me mt6737m.

I don't know why the extra "m"; probably a variant.

I'd read it into a static final field and query when required (maybe even create a "workaround-map").

At the end of the day, it's a matter of choosing between two hacks, so pick your poison. Using the chipset is more robust IMHO because I'm pretty sure that all devices with the same chipset have the same MediaCodec issues (also think about the adaptation workaround (#3257), which is required on some Exynos devices but not on MediaTek).

Being a Googler, can you reach out to the Android people and ask them to add the chipset id to android.os.Build? Moreover, maybe Google should maintain a public device properties/capabilities db.

As for device popularity - we have a large customer in India; even unpopular, cheap, devices can have a few tens of thousands of users.

@AntonAFA
Copy link
Author

Hello team. As was previously discussed on this thread, we made some sort of workaround test, that checks and gather potentially codec problematic devices. Those in order to report you and make exoplayer better.
The workaround is pretty straight forward (as referenced here by @noamtamim https://github.com/kaltura/playkit-android/blob/2e58d94c0c9e2b962e9f31699f1f1c2b07d80790/playkit/src/main/java/com/kaltura/playkit/player/MediaCodecWorkaroundTest.java).
Before actually playing real content we first instatiate "test" player with local asset and dummy drm license. We listen for the Player events which should indicate us, if media with drm was managed to play on the device. Receiving of
MediaCodecRenderer.DecoderInitializationException will indicate us that combination of the device specification and codec might be problematic. So we gather this device info and raise some flag that will make your codecNeedsSetOutputSurfaceWorkaround() return true.

The problem we face now, is that amount of "problematic" devices that was collected by this test is huge. So I believe, that our test covers to much use-cases and probably we should somehow narrow/filter MediaCodecRenderer.DecoderInitializationException by its cause. Can you help us with this filter by explaining which reasons for MediaCodecRenderer.DecoderInitializationException might be relevant for us?
As I see there are only 3 reasons where this exception might be thrown

  1. When DecoderQueryException is thrown
  2. When codecInfo is null in maybeInitCodec block
  3. And when trying to configure codec in the same block by calling configureCodec(codecInfo, codec, format, wrappedMediaCrypto);

Will appriciate your response on the subject

@tonihei
Copy link
Collaborator

tonihei commented May 21, 2018

I tried using your test code and the assets (mp4+mp4+init data) and also get workaroundRequired = true for a device which should not need the workaround.
Could it be that all devices are labelled as needing the workaround with this approach? Or am I missing something?

@AntonAFA
Copy link
Author

No, we label device as needed workaround only when we get MediaCodecRenderer.DecoderInitializationException. Probably there might be additional reasons for getting thes exception. And thats what I am trying to figure out. On which device did you run the test?

@tonihei
Copy link
Collaborator

tonihei commented May 22, 2018

I used a Nexus 6P and got a DecoderInitializationException as well. I fear that the workaround test doesn't cover the real thing because it does never switches (or sets) surfaces and probably fails for other reasons.

@AntonAFA
Copy link
Author

AntonAFA commented May 22, 2018

What was the cause fot this DecoderInitializationException? Can you suggest some more convinient way of doing that test? Maybe, ad some "setSurface" block?

@tonihei
Copy link
Collaborator

tonihei commented May 22, 2018

After looking into this in more detail, I think setting the surface shouldn't be necessary because the DummySurface is still created (as you don't set a surface at all). It just wouldn't catch devices which only have problems with switching surfaces but that's probably fine for the first try.

However, it seems the test content is too small. The reason is fails for me is "Unsupported WxH = (32)x(32) supported range is min(64)x(64) - max(4096)x(4096)". When you look at the failing stack traces, you'll notice that it fails in MediaCodec.configure and not in MediaCodec.start as in the stack trace above in your first post.

@AntonAFA
Copy link
Author

Unfortunately we have no device that is run in to this size issue. Is it possible to ask you test this content, and tell if it`s good enough?
https://github.com/kaltura/playkit-android/tree/surfacetest-fix/playkit/src/main/assets/DRMTest

@tonihei
Copy link
Collaborator

tonihei commented May 22, 2018

Your new sample works for me (i.e. it reports as not needing the workaround), but only if I replace the fakeDrmCallback with a real one (e.g. HttpMediaDrmCallback with "https://proxy.uat.widevine.com/proxy?provider=widevine_test").

@noamtamim
Copy link
Contributor

@tonihei the problem is that at the time the test runs, we don't have a license URL that will respond correctly. Are you saying that when you set this proxy URL the response it provides (which is obviously not a valid license response) is good enough? What if I'll be offline at the time of testing?
More importantly, the idea for the fakeDrmCallback with a delay of 10 seconds is making sure the license will not fail before the test reaches a conclusion.

In other words -- the test works for you on a device that really shouldn't be problematic (shouldn't be listed in the workaround list). But I suspect it will also skip the real culprits (such as Meizu 5c).

@tonihei
Copy link
Collaborator

tonihei commented May 23, 2018

The fakeDrmCallback doesn't work because the player just waits for 10 seconds and then issues an IllegalArgumentException because the key response is null.

To get a valid Widevine license (which allows to setup the secure surface), you either need the online license request or you manage to obtain an offline license which never expires.
Alternatively, [thanks to @AquilesCanta for pointing this out], you can use ClearKey to provide the key directly in the media. This should allow you to run the test offline.

Unfortunately, I don't have one of the failing devices available at the moment. So I can't confirm whether the test with the real drm callback actually fails.

Final remark: You should probably also release the test player for all other errors which are not a DecoderInitializationException to prevent resource leaking.

@noamtamim
Copy link
Contributor

  • I considered a license that never expires (I can create such a thing in our DRM server), but this license is still bound to the device that created the request -- so it's not something I can hold as an asset
  • ClearKey would be nice, but I'm pretty sure it's only supported from Android 7.1 and up. We need to support 4.3.
  • I don't need the license request to succeed, because I don't really intend to play the file. That's the point of the fakeDrmCallback: I want to give the renderer a chance to fail because of the DummySurface issue before it fails for an invalid response.

@tonihei
Copy link
Collaborator

tonihei commented May 23, 2018

I want to give the renderer a chance to fail because of the DummySurface issue before it fails for an invalid response.

The problem is that the player never reaches the READY state in such a setup. Instead, after 10 seconds waiting, it gets the wrong key request answers and fails with a DrmSessionException. If I understand you correctly, that's what you wanted. Waiting for the READY state is then probably not the right way to "pass" the test.

When you say the number of problematic devices is huge - how do you determine that a device is problematic? Do you count the number of positive and negative examples of this workaround test? If so, you should be able to filter by flaky decoder initialization problems for other reasons. This problem should be reproducible 100% of the time, so there shouldn't be a single passing device of the problematic ones.

@noamtamim
Copy link
Contributor

Hi @tonihei,
We don't ever expect the player to be READY. However, we do expect that if it failed with DecoderInitializationException, it will be because of the DummySurface issue:

if (error.getCause() instanceof MediaCodecRenderer.DecoderInitializationException) {
    workaroundRequired = true;
}

Putting it all together, I suspect that the only reason for the huge amount of false positives is the video size. Am I right?

BTW, in the released version we did move the player.release() call outside of the if.

@tonihei
Copy link
Collaborator

tonihei commented May 29, 2018

Yes, that seems fine. I think I got quite confused that your example code set workaroundRequired = false when READY was reached which clearly wouldn't work.

And yes I agree, that to some extent the failures were due to the video size. Please note that there will be other DecoderInitialization failures for various reasons and you'll still need to check that the test fails for all devices of the same model.

@noamtamim
Copy link
Contributor

We have another report of a device that should be added to the DummySurface workaround -- the new Huawei Y3 (2018), an Android Go device: https://www.gsmarena.com/huawei_y3_(2018)-9196.php.

@tonihei
Copy link
Collaborator

tonihei commented Jun 6, 2018

Thanks for reporting! That really is a never-ending-story ;(
Do you have the Build.MODEL for that? It might be "CAG-L02" or "CAG-L22".

@noamtamim
Copy link
Contributor

The Build.MODEL is listed as "gobo", but I don't think it's a real value. The tested device was not yet released at the time (TAGS=dev-keys), so all the values look strange:

        "BRAND": "google",
        "MODEL": "gobo",
        "MANUFACTURER": "alps",
        "TAGS": "dev-keys",
        "FINGERPRINT": "google/gobo/gobo:8.1.0/OMB1.171031.003/4428322:user/dev-keys",

The device was released since, but I don't have anyone that can check on a production device.

The chipset is MT6737M -- the same as some other devices listed above.

@noamtamim
Copy link
Contributor

noamtamim commented Jun 10, 2018

@tonihei / others -- does Google have a database of all the certified Android devices, that contain everything from android.os.Build?

@tonihei
Copy link
Collaborator

tonihei commented Jun 11, 2018

Yes, that's officially available here: https://support.google.com/googleplay/answer/1727131?hl=en-GB That's where my suggestion above came from.

@noamtamim
Copy link
Contributor

Thanks @tonihei, I didn't know about this list. Regarding the device, they both use the same chipset, and the issue is there.

@tonihei
Copy link
Collaborator

tonihei commented Jun 13, 2018

Do you also have the culprit decoder name? Forgot to ask about it.

@noamtamim
Copy link
Contributor

No, sorry. I don't have the device itself with me.

@ojw28
Copy link
Contributor

ojw28 commented Jul 4, 2018

Let's dupe this onto #4468, which is newer, but provides a more comprehensive list of (possibly) affected devices. We can use that for tracking.

I think we'll also go ahead and make codecNeedsSetOutputSurfaceWorkaround and codecNeedsDummySurfaceSurfaceWorkaround protected and non-static, as was suggested here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants