Android 11 IncorrectContextUseViolation in VideoFrameReleaseTimeHelper

[adrientetar]

An application context is used to retrieve window manager in VideoFrameReleaseTimeHelper, which Android 11 StrictMode warns about.

Seems there's another way that should be used on recent android versions: https://github.com/microsoft/appcenter-sdk-android/pull/1429/files#diff-aeda387f31bf19cc06b12c402a17da91e0fc84cd47675e1d74164ca2fd164fdfR153-R166

IncorrectContextUseViolation: Visual services, such as WindowManager, WallpaperService or LayoutInflater should be accessed from Activity or other visual Context. Use an Activity or a Context created with Context#createWindowContext(int, Bundle), which are adjusted to the configuration and visual bounds of an area on screen.
        at android.os.StrictMode.onIncorrectContextUsed(StrictMode.java:2192)
        at android.app.ContextImpl.getSystemService(ContextImpl.java:1917)
        at android.content.ContextWrapper.getSystemService(ContextWrapper.java:803)
        at com.google.android.exoplayer2.video.VideoFrameReleaseTimeHelper.<init>(VideoFrameReleaseTimeHelper.java:76)
        at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.<init>(MediaCodecVideoRenderer.java:348)
        at com.google.android.exoplayer2.DefaultRenderersFactory.buildVideoRenderers(DefaultRenderersFactory.java:322)
        at com.google.android.exoplayer2.DefaultRenderersFactory.createRenderers(DefaultRenderersFactory.java:260)
        at com.google.android.exoplayer2.SimpleExoPlayer.<init>(SimpleExoPlayer.java:430)
        at com.google.android.exoplayer2.SimpleExoPlayer.<init>(SimpleExoPlayer.java:378)
        at com.google.android.exoplayer2.SimpleExoPlayer$Builder.build(SimpleExoPlayer.java:296)

version 2.11.7

[ojw28]

This is probably a result of changes in Android to better support multi-display devices. These changes mean that visual services should be retrieved in a way that identifies the display on which the app is running.

This is somewhat tricky to fix in ExoPlayer's case. For now, you should be fine to ignore/disable the warning. We'll need to figure out a way of addressing this at some point though, so thanks for filing the issue!

[jackmichalak]

Would love to see this addressed. Following through the stack trace it appears (at a quick glance) that replacing context.getApplicationContext() with just context in a half dozen or so places would allow the user-supplied Context to be utilized. What am I missing that makes this tricky?

[ojw28]

I agree it's quite straightforward for the common case of video playback in a single activity. However, there are some complications for other cases. For example, it's quite easy to accidentally leak an activity context with the suggested approach, particularly for use cases where playback may transition between activities, or be detached from an activity altogether (e.g., for transitioning into background playback). If the player is initially constructed for background playback, then the application may not have a suitable context to provide at construction time.

More generally, VideoFrameReleaseTimeHelper is making some assumptions about video being rendered on the default display. So far this has not caused any real issues that we're aware of, but the assumption itself is increasingly less likely to hold true as multi-display devices become more common. If such devices start to drive their displays in more flexible ways, the current implementation of VideoFrameReleaseTimeHelper could become more problematic in the future. The only use of WindowManager in VideoFrameReleaseTimeHelper is a line of code that makes such an assumption, so it might be that we should be making more substantial changes that will cause the need to retrieve it to disappear entirely.

[ojw28]

As a short term fix, it looks like we can just replace use of WindowManager.getDefaultDisplay with DisplayManager.getDisplay(DEFAULT_DISPLAY). This doesn't solve any of the potential problems around multiple displays, but it does remove the warning whilst avoiding both (a) the need for plumbing changes outside of VideoFrameReleaseTimeHelper, and (b) increased risk of accidental context leaks. We'll go ahead and make that change.

[jackmichalak]

Thank you very much for the additional context, that makes a lot of sense. I would be happy with the mentioned short-term fix.