Change two players between two views problem on device MIBOX4 aka MIBOX S

[fgl27]

Issue description

I have a Picture Picture mode on my player, one full screen player one small over the full.

I allow to change with is the full or small on one click...

On the MIBOX4 when using PlayerView.setPlayer(SimpleExoPlayer); to change the players between the two views there is a Exo error. I notice that the error will almost always occur to the player that is in the view that is changing from full screen to small, but the other way around it occurs less.

This issue is only on this device as I have tested on others devices and is all OK, maybe some other device that I don't have may have the same issue, so far no user report yet from another device, this is a new feature...

I have open a similar issue here #7998, on that there is a video that shows how the process of change the views looks.

Reproduction steps

The function I'm using to make the change is this:

    private int pos = 1;
    private void SwitchPlayer() {
        Log.d(TAG, "SwitchPlayer");

        for (int i = 0; i < 2; i++) {

            PlayerView[i].setPlayer(player[i ^ pos]);

        }

        pos = pos ^ 1;
    }

By default I'm using SurfaceView for the view, but I also tested TextureView is all fine on that view, the issue only triggers when using SurfaceView.

Also using SW decoder (OMX.google) there is no problem, here are the device codecs used by this content , this is a low end device, in order to test two streams playback at the same time using SW decoder was used very low resolution/bitirate streams.

Link to test content

I'm playing HLS live streams, I assume the link is not applicable, as my link do expire I share one on request.

A full bug report captured from the device

bugreport-oneday-PI-2020-09-27-05-34-25.zip

On the above I have added a log before the change

STTV_PlayerActivity: SwitchPlayer

When the problem happens we have on the log some Exo errors as the bellow.

09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal: Playback error
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:   com.google.android.exoplayer2.ExoPlaybackException: Unexpected runtime error
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:582)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at android.os.Handler.dispatchMessage(Handler.java:102)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at android.os.Looper.loop(Looper.java:193)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at android.os.HandlerThread.run(HandlerThread.java:65)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:   Caused by: java.lang.IllegalArgumentException
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at android.media.MediaCodec.native_setSurface(Native Method)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at android.media.MediaCodec.setOutputSurface(MediaCodec.java:1979)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.setOutputSurfaceV23(MediaCodecVideoRenderer.java:1242)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.setSurface(MediaCodecVideoRenderer.java:516)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.handleMessage(MediaCodecVideoRenderer.java:478)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.ExoPlayerImplInternal.deliverMessage(ExoPlayerImplInternal.java:1441)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.ExoPlayerImplInternal.sendMessageToTarget(ExoPlayerImplInternal.java:1407)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.ExoPlayerImplInternal.sendMessageInternal(ExoPlayerImplInternal.java:1382)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:501)
09-27 05:29:25.394 10075 16599 16837 E ExoPlayerImplInternal:       ... 3 more

Version of ExoPlayer being used

dev-v2

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

MIBOX4 aka MIBOX S
Android 9

[fgl27]

After reading this #5169, I tryed to enable deviceNeedsSetOutputSurfaceWorkaround, I decide to do like this fgl27@573ae9a just for a qiuck test as the app content is only avc.

I don't see any setback and the issue has stopped, no more Playback error when changing the view, but there is a small freeze that happen only to the player that goes from full screen to small, is around 0.5 to 2 seconds is long enough that is noticeable, but is better than a error that cause the player to stop.

On this issue #7411 (comment) one of yours developers informs that has access to this device, maybe other Xiaomi device, you guys can check this better than me, so please let me know if any else is needed.

[fgl27]

After using deviceNeedsSetOutputSurfaceWorkaround for some time I say it doesn't worth it.

As the behavior of the app is worst because now instead of the player throw a error and restart (I have a internal function that will just re-initialize the player after error) it will freeze and the freeze is a random time value some times up to 5 seconds, sometimes there is audio but the video is freezed, sometimes the freeze is on the audio and video (maybe only on the video, maybe the video I was testing didn't produce audio on that freeze), so for the user experience feels better just restart the player after a error and keep deviceNeedsSetOutputSurfaceWorkaround disabled...

I hope you guys can give a feedback and improve this.

[krocard]

By default I'm using SurfaceView for the view, but I also tested TextureView is all fine on that view, the issue only triggers when using SurfaceView.

I'm afraid if the deviceNeedsSetOutputSurfaceWorkaround does not work well, it might be your only solution.
Though the error is similar to #6346, but I'm not aware if a codec is recreated on surface change.
@andrewlewis might know more.

[krocard]

A workaround would be to swap both surfaces size and positions instead of swapping the player surface. That would also allow you to apply an animation when swapping if wanted.
See also #6958 (comment)

Would that solve your issue?

[fgl27]

No, changing the surface size also causes the issue, the issue will happen on the view that goes from full to small, more often than the other way around.

I'm using playerView.setLayoutParams(layout). I don't think there is any other way, there is?

If this can be addressed in a way that is satisfied, can we at least ask Xiaomi to take a look as was done here #7411 (comment) ?

Until today there is no update from they side regarding that issue, but there says that they will, they can also revise this, as if this gets fixed it will probably prevent some other related of codec issues.

BTW on most devices that I have tested, when you animate the size change while the video is playing, it looks terrible, moving the view without change the size looks OKish with animation, but resizes looks bad, and I tested on devices that have good performance, this device in particularly is not good on performance.

The animation that I have tested was using beginDelayedTransition(ViewGroup sceneRoot, Transition transition) before changing the layout.

[krocard]

No, changing the surface size also causes the issue, the issue will happen on the view that goes from full to small, more often than the other way around.
I'm using playerView.setLayoutParams(layout).
So View.setLayoutParams causes the surface to change. I don't know if there is a better way to change the layout of the surface.

@andrewlewis could you look at this similar issue as #7996? Do you think there is a way to change the surface size without causing a MediaCodec.setOutputSurface ?

[andrewlewis]

I'm surprised changing layout params is causing the surface to change, as I think there are apps where the position/size of a SurfaceView showing decoder output is smoothly animated and I don't think that could happen if setOutputSurface was being called repeatedly on the codec (the codec doesn't re-render its last frame when its output surface is changed). Could you double-check the surface really is changing (possibly by setting breakpoints in the surface view callbacks)?

Aside: before Android N SurfaceView can't be used if you need to animate the view (see this documentation).

[krocard]

As the behavior of the app is worst because now instead of the player throw a error and restart (I have a internal function that will just re-initialize the player after error) it will freeze and the freeze is a random time value some times up to 5 seconds, sometimes there is audio but the video is freezed, sometimes the freeze is on the audio and video (maybe only on the video, maybe the video I was testing didn't produce audio on that freeze), so for the user experience feels better just restart the player after a error and keep deviceNeedsSetOutputSurfaceWorkaround disabled...

Unrelated, but I'm surprise that deviceNeedsSetOutputSurfaceWorkaround is slower than restarting the player after the error. In both case the MediaCodec is destroyed and recreated. Maybe the video codec can not catch up with the audio :/

[fgl27]

Could you double-check the surface really is changing (possibly by setting breakpoints in the surface view callbacks)?

Yes I can, thanks for the help, there was some confusion on my side... Sorry.

I re-tested everything I hope this is not too confusing, if it's let me know I'll try explaining better.

If I use playerView.setPlayer(player); to a player that is already working, I get the error from my first post.

If I just change the layout and fix the z order, the error will not happens...

Why I was confusing things???

I wasn't using SurfaceView.setZOrderMediaOverlay(boolean) to fix the order...
I was doing the bellow to the small view the one that stay in top:

playerView_parent.bringChildToFront(playerView);
playerView.setVisibility(View.GONE);
playerView.setVisibility(View.VISIBLE);

If I don't change the visibility like that, the view didn't showed on top.

So I discovered that what was causing the error was just:

playerView.setVisibility(View.GONE);
playerView.setVisibility(View.VISIBLE);

Yes I tried just that part alone to a playerView that is playing a video, and the error will happen on almost every time you do that.

Now I have change that part to:

SurfaceView_Small.setZOrderMediaOverlay(true);
SurfaceView_Full.setZOrderMediaOverlay(false);

SurfaceView_Small is the SurfaceView of the small player that must stay on top, the SurfaceView_Full is the background full screen player, as I allow to dynamically change with will be the full/small I have to do that all the time not just once...

And now there is no error.

I also did one more test and discovered that the error from first post will also happens when you remove and add the view when the playback is running like this:

playerView_parent.removeView(playerView);
playerView_parent.addView(playerView);

The reason why I bring that, is because of #7999
To have the SurfaceView visible after the change of the z order on android 7 and below I have to do like this:

playerView_parent.removeView(playerView_Small);
playerView_parent.removeView(playerView_Full);

SurfaceView_Small.setZOrderMediaOverlay(true);
SurfaceView_Full.setZOrderMediaOverlay(false);

playerView_parent.addView(playerView_Small);
playerView_parent.addView(playerView_Full);

If I don't the top view will not be visible... Technically I can just do the view remove/add on android 7 and bellow. To avoid the error on this device (this device original Android version is 8).

Unrelated, but I'm surprise that deviceNeedsSetOutputSurfaceWorkaround is slower than restarting the player after the error. In both case the MediaCodec is destroyed and recreated. Maybe the video codec can not catch up with the audio :/

Regarding using deviceNeedsSetOutputSurfaceWorkaround the error happens when change-ing the visibility or remove/add the view just like the cases that I inform before.

Sometimes the error will not happen but the screen will freeze for a few seconds only when using deviceNeedsSetOutputSurfaceWorkaround.

But the freezes or errors are resolved after not changing the view visibility or removing/adding it.

.........

So technically you guys have help me to solve the issue regarding this Picture in Picture feature of my...

But not fully, there is one case where I wanna to reuse the player of one view and I can't move the view, I can only use playerView.setPlayer(player); or playerView_parent.removeView / addView is a very particular case that only works well if do like that, because I wanna move the player that is on a playerView above the UI to a playerView that is below the UI. And both of those cases will cause the issue so is better not to change the player just start a new one using the same media.

So can you guys with this extra info work on that issue?

I don't have any priority on this I just would like if that can work.

Thanks for the help let me know if there is something else needed to better explain.

.....

BTW on most devices that I have tested, when you animate the size change while the video is playing, it looks terrible, moving the view without change the size looks OKish with animation, but resizes looks bad, and I tested on devices that have good performance, this device in particularly is not good on performance.

The animation that I have tested was using beginDelayedTransition(ViewGroup sceneRoot, Transition transition) before changing the layout.

BTW 2.

When I wrote that I was referring to the video playing, the playerView resize animation looks OK, but the video inside of it, when resizing the view from for example 1/5 full screen size to full screen doesn't animate very well and looks disturbing, maybe only to me but is visually un-pleasing, that is why animation on a video that is be playing I will not use.

Maybe because I'm doing this on Android TV on a big screens that bothers me, maybe the device I have tested aren't the best animating videos, maybe on a phone looks better. Not something demanding to use, not something be addressed on this issue, just some extra info that was discussed alongside.

[krocard]

Unrelated, but I'm surprise that deviceNeedsSetOutputSurfaceWorkaround is slower than restarting the player after the error. In both case the MediaCodec is destroyed and recreated. Maybe the video codec can not catch up with the audio :/

Sometimes the error will not happen but the screen will freeze for a few seconds only when using deviceNeedsSetOutputSurfaceWorkaround.

Side note: I think I have figure out why this happens. Because of deviceNeedsSetOutputSurfaceWorkaround, the video codec is destroy and recreated on setOutputSurface, but the audio keeps playing. Thus the new codec has to decode faster than real time from the last keyframe to catch up the audio until playback reaches the next keyframe.

[krocard]

But not fully, there is one case where I wanna to reuse the player of one view and I can't move the view, I can only use playerView.setPlayer(player); or playerView_parent.removeView / addView is a very particular case that only works well if do like that, because I wanna move the player that is on a playerView above the UI to a playerView that is below the UI. And both of those cases will cause the issue so is better not to change the player just start a new one using the same media.

Unfortunately, I don't know much about moving views. Why can't you move the z order of the player view to put it under the UI view? Similar to what you did for the two player view.

[fgl27]

But not fully, there is one case where I wanna to reuse the player of one view and I can't move the view, I can only use playerView.setPlayer(player); or playerView_parent.removeView / addView is a very particular case that only works well if do like that, because I wanna move the player that is on a playerView above the UI to a playerView that is below the UI. And both of those cases will cause the issue so is better not to change the player just start a new one using the same media.

Unfortunately, I don't know much about moving views. Why can't you move the z order of the player view to put it under the UI view? Similar to what you did for the two player view.

I need to further check to see if it's possible.

In order to have this mess I made working was easier to contain background players, UI, top player, on separated frame layouts...

I need to see if is possible to have it all in the same container and keep the expected z order on all supported android versions in all situations.

Probably is possible will work on it.

Thanks.

[fgl27]

I did some testing, on this device isn't possible to reuse the player by moving the view...

I have this

PlayerView_background --- UI --- PlayerView_onTop

The PlayerView_onTop is a small preview player that allows the user to choose something to play next without closing PlayerView_background

If after all the player have load a video I move the PlayerView_onTop to the place of PlayerView_background and vice versa, release the one that is now on top and fix all the orders, after all of that has finished all looks fine but if I try to load a new player on the Top PlayerView that at this point is PlayerView_background the player will not be visible, the view shows at the TOP during the buffer as soon as the Player.STATE_READY is called the view disappears or becomes transparent...

I tried the same on another device and all works so the logic is fine but this device isn't.

The only way I can make all players be visible after that visual problem is by releasing and restarting the player that is now on the background, the Top one is not playing during this moment, after the Background player finishes loading, I can load and see a player on the Top one again, and the issue repeats it self if I try another change.

The odd thing is that after this issue has happened, if I change the size of the player that is on the background so I can see if the other one is behind it I can't see the Top one, is just a very odd visual bug that causes the SurfaceView to disappear.

With make this feature useless on this device, is just better not change the view order and just reuse the media, as any other thing that I have tried will cause visual issues or player errors.

I also test this visual issue with deviceNeedsSetOutputSurfaceWorkaround enabled and is the same.

[andrewlewis]

If I understand correctly, moving the preview player (that was previously on top) to fill the background is working fine, but the new preview player (which used to be in the background) is not behaving correctly after the transition, and this problem only affects certain devices.

I think if I were you I'd add a mode where you release and create a new preview player and surface each time a preview is made full-screen, and enable that mode automatically on devices that are known to be affected. This is obviously a workaround but maybe the user experience isn't too clunky (you could show a thumbnail preview while the new preview is loading).

At this point this seems like a device-specific issue that would need to be addressed by the maker of the device, and we can't really provide much more guidance so I will close this bug.

[fgl27]

Are you going to inform Xiaomi?

As was done here #7411 (comment)

This is a device problem that must be addressed by the maker yes, but if they aren't informed how they will fix it? I don't have any contact.

[andrewlewis]

They will almost certainly want minimal steps/code to reproduce the problem, in addition to a description of what is going on. If you could make a test project (possibly based on the ExoPlayer demo app) and share a link to the code we can file a bug with them.

[fgl27]

OK I'll share it tomorrow or later this week.

Thank you.

[fgl27]

I made an app based on the demo app for testing as requested

Here is the apk (is zipped so extract so you can install)

mibox_demo-noDecoderExtensions-debug.zip

Here is the source

ExoPlayer-dev-v2_mi_test.zip

Here is the change

fgl27@c5a0273

This was the head of ExoPlayer dev-v2 branch on the day of the above change

https://github.com/google/ExoPlayer/commits/9753c3fcfb2e2e7fd878666c3f8aa11cbee9728f

Here is the result after 10 sec of playback

Here is a demonstration on how to use the app and that it works, at least on the emulator on the MIBOX4 will cause the above error

https://www.youtube.com/watch?v=2RUnSk1nlNo

Basically what that app does is change the playerView visibility to GONE and back to VISIBLE after 10 second of Player.STATE_READY.

As the error on all situations that I have found are the same if this case that is very simple to replicated and test gets fixed I expect all others will, I can properly retest it all the day the device gets an OS update.