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

Secure decoder is re-initialized if renderer is disabled during a transition (e.g. due to seeking or a discontinuity) #8842

Closed
sergju opened this issue Apr 21, 2021 · 19 comments
Assignees

Comments

@sergju
Copy link

sergju commented Apr 21, 2021

Hi.
I am searching way to change stream without black screen.
I made shutter surface transparent with
setShutterBackgroundColor(Color.TRANSPARENT);
and now don't have a problem with HLS clear content, but still have with DASH DRM content

After some investigation figured out black screen happens due to decoder reset with this condition
if (sourceDrmSession != null || codecDrmSession != null) {
// TODO: Do something better with this case.
onReset();
}

So may be exists way to avoid resetting. Or may be exist other way to avoid black screen.
Actually the streams have the same url, but different parameters. So no need to change codec and so on

@icbaker
Copy link
Collaborator

icbaker commented Apr 21, 2021

Do you see the black screen on all devices, or just specific ones?

Are you switching between two pieces of differently encrypted DRM content, or from encrypted to/from clear? If the latter, you should try using a placeholder DRM session for the clear content: https://google.github.io/ExoPlayer/drm.html#drm-sessions-for-clear-content

If this reproduces consistently across devices would you be able to provide a stream we can use to reproduce the issue? Please either upload it here or send to dev.exoplayer@gmail.com using a subject in the format "Issue #1234" (where "#1234" should be replaced with this issue number.) Please also update this issue to indicate you’ve done this.

@sergju
Copy link
Author

sergju commented Apr 21, 2021

  1. I tried only on one specific device. To try on other device I need to prepare test application.
    I will do it later.
  2. 2 pieces of differently encypted. Yes, I saw this issue with encrypted -> clear

@icbaker
Copy link
Collaborator

icbaker commented Apr 21, 2021

2. 2 pieces of differently encypted. Yes, I saw this issue with encrypted -> clear

Not sure I understand this - you only see the issue when switching from encrypted to clear content?

In that case, try the placeholder DRM session approach I linked.

@jrocharodrigues
Copy link

Hi @icbaker ,
I have a similar use case, and problem in my app, of switching between DRMprotected and clear streams.

Rigth now, without using the placeholder DRM Session, i see a black screen when switching from DRM Protected Content -> Clear Content.

I've tried the approach you've suggested, by by passing true to MediaItem.Builder.setDrmSessionForClearPeriods when building the media item, and adding .setUseDrmSessionsForClearContent(TRACK_TYPE_VIDEO) to the DefaultDrmSessionManager.Builder().
But instead of stopping the black screen when switching from DRM Protected Content -> Clear Content, now it also shows an black screen when switching from Clear Content -> DRM Protected Content.

Am i missing something to implement the approach you suggested?

Thank you

@icbaker
Copy link
Collaborator

icbaker commented May 5, 2021

@jrocharodrigues This doesn't answer your question, but you shouldn't need to both configure the DRM fields of MediaItem and provide a custom DefaultDrmSessionManager. If you're passing a DrmSessionManager instance in (maybe to MediaSourceFactory#setDrmSessionManager()?) then the DRM fields on MediaItem will be ignored - they're only used inside DefaultDrmSessionManagerProvider which is used if you don't call any DRM methods on MediaSourceFactory.

Am i missing something to implement the approach you suggested?

There isn't really enough info here to say I'm afraid. I suggest you file a new issue with clear repro steps and preferably a minimal reproducible example that demonstrates the problem in a way that we can build locally.

This could be an Android Studio project on GitHub, or zipped up and sent to dev.exoplayer@gmail.com using a subject in the format "Issue #1234", where "#1234" should be replaced with your issue number. Please also update this issue to indicate you’ve done this.

It's quite likely this is device specific (that's generally been our experience with these black-screen-during-DRM-transitions issues) - so if you have access to a device from a different manufacturer to experiment with that would also be worth a try (and let us know in the issue what devices you've tried and what you've seen).

@jrocharodrigues
Copy link

@icbaker Thank you for your answer.

@jrocharodrigues This doesn't answer your question, but you shouldn't need to both configure the DRM fields of MediaItem and provide a custom DefaultDrmSessionManager. If you're passing a DrmSessionManager instance in (maybe to MediaSourceFactory#setDrmSessionManager()?) then the DRM fields on MediaItem will be ignored - they're only used inside DefaultDrmSessionManagerProvider which is used if you don't call any DRM methods on MediaSourceFactory.

Ok, that explains why the behavior didn't change until i changed the configuration on the DRMSessionManager.

There isn't really enough info here to say I'm afraid. I suggest you file a new issue with clear repro steps and preferably a minimal reproducible example that demonstrates the problem in a way that we can build locally.

I'll do some more tests in different devices to check if it's a device specific issue, and if needed i'll open a new issue.

Best regards

@jrocharodrigues
Copy link

Hi @icbaker i've done some more testing in different devices, with the sample app, playing the playlist "Clear -> Enc -> Clear -> Enc -> Enc", from the provided samples.
What i see is that on android TV devices (which was the ones i've tested before because my app is for android TV), i see a black screen when switching from DRM to clear content, but on smart
phones i don't see the black screen.

The devices i tested were:
Andrdoid TV: Xiaomi Mibox 3 and Chromecast with Google TV
Smartphones: Google Pixel 4 and Samsung J3

Is this behaviour correct or should i open a issue for this?

@icbaker
Copy link
Collaborator

icbaker commented May 6, 2021

@jrocharodrigues I can reproduce what you're seeing on Chromecast with Google TV (and not on a Pixel 3a XL).

I crafted the following JSON with clipping to make the transitions more obvious (because tears of steel starts/ends with a black screen, so seeing black flashes was really hard):

{
  "name": "DRM transitions (playlists)",
  "samples": [
    {
      "name": "Clear -> Clear",
      "playlist": [
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "clip_start_position_ms": 10000,
          "clip_end_position_ms": 20000
        },
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "clip_start_position_ms": 20000,
          "clip_end_position_ms": 30000
        }
      ]
    },
    {
      "name": "Clear -> Clear (use secure)",
      "playlist": [
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "drm_session_for_clear_content": true,
          "clip_start_position_ms": 10000,
          "clip_end_position_ms": 20000
        },
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "drm_session_for_clear_content": true,
          "clip_start_position_ms": 20000,
          "clip_end_position_ms": 30000
        }
      ]
    },
    {
      "name": "Clear -> Clear (use secure & set scheme)",
      "playlist": [
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "drm_session_for_clear_content": true,
          "drm_scheme": "widevine",
          "clip_start_position_ms": 10000,
          "clip_end_position_ms": 20000
        },
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "drm_session_for_clear_content": true,
          "drm_scheme": "widevine",
          "clip_start_position_ms": 20000,
          "clip_end_position_ms": 30000
        }
      ]
    },
    {
      "name": "Enc -> Clear",
      "playlist": [
        {
          "uri": "https://storage.googleapis.com/wvmedia/cenc/h264/tears/tears_sd.mpd",
          "drm_scheme": "widevine",
          "drm_license_uri": "https://proxy.uat.widevine.com/proxy?provider=widevine_test",
          "clip_start_position_ms": 10000,
          "clip_end_position_ms": 20000
        },
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "clip_start_position_ms": 20000,
          "clip_end_position_ms": 30000
        }
      ]
    },
    {
      "name": "Enc -> Clear (use secure)",
      "playlist": [
        {
          "uri": "https://storage.googleapis.com/wvmedia/cenc/h264/tears/tears_sd.mpd",
          "drm_scheme": "widevine",
          "drm_license_uri": "https://proxy.uat.widevine.com/proxy?provider=widevine_test",
          "drm_session_for_clear_content": true,
          "clip_start_position_ms": 10000,
          "clip_end_position_ms": 20000
        },
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "drm_session_for_clear_content": true,
          "clip_start_position_ms": 20000,
          "clip_end_position_ms": 30000
        }
      ]
    },
    {
      "name": "Enc -> Clear (use secure & set scheme)",
      "playlist": [
        {
          "uri": "https://storage.googleapis.com/wvmedia/cenc/h264/tears/tears_sd.mpd",
          "drm_scheme": "widevine",
          "drm_license_uri": "https://proxy.uat.widevine.com/proxy?provider=widevine_test",
          "drm_session_for_clear_content": true,
          "clip_start_position_ms": 10000,
          "clip_end_position_ms": 20000
        },
        {
          "uri": "https://storage.googleapis.com/wvmedia/clear/h264/tears/tears_sd.mpd",
          "drm_scheme": "widevine",
          "drm_session_for_clear_content": true,
          "clip_start_position_ms": 20000,
          "clip_end_position_ms": 30000
        }
      ]
    }
  ]
},

I found the following:

  1. Clear -> Clear: Transition is 'seamless' (there's a small glitch, but it's probably because i didn't clip at a keyframe). One insecure decoder is used throughout without being released.
  2. Clear -> Clear (use secure): Exactly the same as (1), including an insecure decoder being used throughout.
    • That's an ExoPlayer bug - I'd expect a single secure decoder to be used throughout. The problem here is that all the DRM info passed to MediaItem.Builder is ignored if the scheme is not set (see next item).
  3. Clear -> Clear (use secure & set scheme): Transition has a black screen. A secure decoder is used, but it's released & reacquired at the transition (causing the black flash).
    • By setting the scheme to the 'wrong' value of "widevine" (this is clear content), we can work around the bug from (2) and the drm_session_for_clear_content field value isn't ignored.
    • But then there's another ExoPlayer bug: Why is the secure decoder released & reacquired at the transition?
  4. Enc -> Clear: Transition has a black screen. A secure decoder is used for the first item but not for the second, which is expected because we don't set drm_session_for_clear_content.
    • Whether the black screen is itself a bug, I'm not sure it's a bug in ExoPlayer - afaik the behaviour when disconnecting a surface from a secure decoder is undefined - and some implementations seem to show the black screen resulting in this problem.
  5. Enc -> Clear (use secure): Same behaviour as (4) due to same bug highlighted in (2).
  6. Enc -> Clear (use secure & set scheme): Same behaviour as (3) due to the same bug discussed there.

Fixing the bug highlighted in (2) and (5) means possibly changing MediaItem.Builder to allow setting DRM fields without setting the DRM UUID - which is technically simple but a little tricky from an API design perspective because we do need a UUID at some point, even to create these placeholder sessions. Or we document more carefully this requirement to set the UUID (both in JSON and when configuring the MediaItem).

The bug highlighted in (3) and (6) looks similar to me to #8568 (comment), where a secure decoder gets re-initialised while 'playing' an empty mid-roll ad. I'll confess that was languishing on my TODO list - I'll aim to take a look at it/this soon (though it likely won't be in the next week). If I conclude the root cause is the same then I'll dupe them together, but I'll keep them separate for now.

@icbaker icbaker changed the title How to change stream without black screen with DRM content. MediaItem.Builder#setDrmSessionForClearPeriods is ignored if DRM UUID is not set + secure decoder is unexpectedly re-initialized during playlist item transition May 6, 2021
@jrocharodrigues
Copy link

@icbaker Thank you for answer.
I didn't test with the set scheme "hack" but the rest corresponds to the behaviour i have.
I'll keep watching both issues to know the progress of the fixes.

BRs

@icbaker
Copy link
Collaborator

icbaker commented May 14, 2021

I did some more testing. I removed the clipping (which I'd added to make the black flashes more obvious) and observed that the secure decoder is no longer re-initialized during transitions.

This is because the clipping caused the second period to start at a discontinuity (since I clipped to arbitrary point, and didn't hit a key frame), and so we went into this code here:

if (readingPeriodHolder.prepared
&& readingPeriodHolder.mediaPeriod.readDiscontinuity() != C.TIME_UNSET) {
// The new period starts with a discontinuity, so the renderers will play out all data, then
// be disabled and re-enabled when they start playing the next period.
setAllRendererStreamsFinal(
/* streamEndPositionUs= */ readingPeriodHolder.getStartPositionRendererTime());
return;
}

When the clipping is removed, the second piece of content starts with a keyframe, and the renderer is never disabled so the video decoder is kept during the transition. This should mean there's no black flash - but I can't actually confirm that with this content, because it starts & ends with black...

I assume both reporters on this issue are only seeing the black flash when the second piece of content starts with a discontinuity?


As highlighted in the original content, secure decoders are treated differently to clear ones when being the renderer is disabled. Clear ones are kept while the renderer is disabled, while secure ones are immediately released:

if (sourceDrmSession != null || codecDrmSession != null) {
// TODO: Do something better with this case.
onReset();
} else {

I discussed with the team about why this is, and the story behind the TODO there. It turns out it's related to ExoPlayer#setForegroundMode(boolean). Brief summary:

  • ExoPlayer can be put into 'foreground' mode meaning that even when playback is stopped with Player#stop (not paused) resources like codecs are not released (see more info at the javadoc linked above).
  • A renderer is disabled without being reset/released in two situations:
    • When transitioning between media periods where the second period starts with a discontinuity (the case relevant for this issue).
    • When the player is in 'foreground mode' and stopped (without foreground mode, the renderer & underlying MediaCodec will be released when the player is stopped).

The TODO is related to the 'foreground mode' case, and a concern about DRM license refreshes continuing to (arguably pointlessly) happen while the player is stopped. The license refresh period is out of ExoPlayer's control (it's defined by the license policy) and these refreshes could be a considered a waste of precious network resources. So the decision was taken to simply release secure codecs in this case (and their associate DRM sessions) to prevent spurious license refreshes. The (somewhat unintended) side-effect of this is that when transitioning into a period that starts with a discontinuity the secure codecs are also released (of course the time the renderer spends 'disabled' in this case is very small, so the DRM license refresh issue isn't a concern).

The intention to fix the TODO was to create a new DRM 'mode' where license refreshes are blocked - and this could be used while in this disabled state.

I'm not convinced that's necessary - I think it's possible to argue the network resources used by the license refreshes are included in the 'limited resources' referred to in the setForegroundMode(boolean) javadoc linked above, and so it's reasonable for these refreshes to continue to happen while the player is stopped in foreground mode.

Having understood what's going on here, I retract my statement above that this is a bug in ExoPlayer. I think it's more like a limitation that only arises in a specific situation (transitioning into a period that starts with a discontinuity). I'm going to experiment with removing the TODO and associated branch, so that secure decoders will always be kept while a renderer is disabled. If that seems to work in a range of different cases I'll submit the change.


The MediaItem confusion also tracked by this issue is harder to resolve, because we do need a DRM UUID to create sessions, and it's almost certainly better for it to be the same UUID as used by the actual encrypted content, it's just unfortunate that the current behaviour when it's missing is to silently ignore all the other DRM-related fields. The first thing I can do is call out this caveat here, which I will do:
https://exoplayer.dev/drm.html#drm-sessions-for-clear-content

I will also look into possibly making MediaItem.Builder fail loudly when passed an 'invalid' combination of fields, instead of silently ignoring some.

@icbaker icbaker added enhancement and removed bug labels May 14, 2021
@jrocharodrigues
Copy link

jrocharodrigues commented May 17, 2021

@icbaker

I assume both reporters on this issue are only seeing the black flash when the second piece of content starts with a discontinuity?

I tested with live and on demand streams and i see the black screen in both (when switching from enc -> clear). I think that at least the on demand ones should both start with an key frame and the shouldn't have a discontinuity.
I've tested with a break point on the lines you've highlighted and it didn't enter there.
Is there anyway to confirm this?

Right now i'm little confused and with what is expected and what needs to be fixed :)

I made some more tests with this samples that don't start with black, and still see the same behavior i reported:
It works ok on pixel 4, but i see a black screen on android tv devices.
This the case probably related to #8568 right?

{
        "name": "Transitions Samples",
        "playlist": [
          {
            "uri": "https://bitmovin-a.akamaihd.net/content/art-of-motion_drm/mpds/11331.mpd",
            "drm_scheme": "widevine",
            "drm_license_uri": "https://widevine-proxy.appspot.com/proxy",
            "drm_session_for_clear_content": true
          },
          {
            "uri": "https://bitmovin-a.akamaihd.net/content/MI201109210084_1/mpds/f08e80da-bf1d-4e3d-8899-f0f6155f6efa.mpd",
            "drm_scheme": "widevine",
            "drm_session_for_clear_content": true
          }
        ]
      }

@icbaker
Copy link
Collaborator

icbaker commented May 18, 2021

@jrocharodrigues Thanks for providing some content to reproduce the problem. However when I play that I don't see the video decoder being re-initialized - it keeps a secure decoder throughout the transition, so it doesn't reproduce the problem. I'm using a version of ExoPlayer built from dev-v2 (but I would expect it to be very similar to 2.14.0).

What version of ExoPlayer are you using? There have been some recent fixes in this area.


I assume both reporters on this issue are only seeing the black flash when the second piece of content starts with a discontinuity?

I tested with live and on demand streams and i see the black screen in both (when switching from enc -> clear). I think that at least the on demand ones should both start with an key frame and the shouldn't have a discontinuity.
I've tested with a break point on the lines you've highlighted and it didn't enter there.
Is there anyway to confirm this?

I guess firstly you can confirm the 'cause' of the black flash is a decoder release/re-acquire. If you're using the demo app, or have the EventLogger attached in your app, you should see log lines like this:

videoDecoderInitialized
videoDecoderReleased

In the 'good' case (no black flash) I'd expect to see a single videoDecoderInitialized at the start of playback and then a final videoDecoderReleased at the end, and not see any during the transition.

In the 'bad' case (black flash) I'd also expect to see videoDecoderReleased immediately followed by videoDecoderInitialized at the time of the transition.

Then working out why the decoder is being released is trickier, there's no simple recipe I can give you, just some patience and a debugger (or give me reproducible media and I'll try and work it out).


Right now i'm little confused and with what is expected and what needs to be fixed :)

Yeah, fair enough - there are a lot of branches to this problem and we've also made some recent improvements so there's different behaviour on different versions of the library...

I'll try a brief summary (apologies if any of this is re-hashing basic info) :)

Root cause

Some devices show a black screen when detaching a surface from a secure decoder. The surface gets detached when the decoder is released. The secure decoder is (obviously, unavoidably) released when transitioning from using a secure decoder to an insecure one. That means the simplest 'repro case' for this problem (as you've seen) is playing encrypted content followed by clear content. However as detailed below, the problem can still occur when transitioning between two pieces of content that should both use a secure decoder (in certain circumstances).

ExoPlayer's general workaround

To avoid the secure codec being released when transitioning from encrypted to clear content, ExoPlayer provides the drm_session_for_clear_content option to 'force' the device to use a secure decoder for clear content. The theory being that if we use a secure decoder for the clear content, we stand a better chance of being able to re-use the same decoder when transitioning from encrypted to clear content, and thus avoid detaching the surface and triggering the black flash.

Caveats with the workaround (API design)

The flag is specified per MediaItem, because the original problematic case we aimed to solve was clear ads embedded in DRM'd content. These are represented by a single MediaItem so it makes sense to specify the option there. However as discussed above this leads to a confusing API when playing a playlist of clear & encrypted items, where you have to specify a DRM scheme for the clear items (which perhaps seems slightly nonsensical) otherwise the flag is a silent no-op.

I've opened a separate issue to track the silent-dropping behaviour, so we can keep this one focussed: #8957

Cases not solved by the workaround

However there are some cases where the decoder is still released during the transition. This means the problem can still occur between two secure periods, or a secure period & a clear period using drm_session_for_clear_content:

  1. When the following period starts with a discontinuity.
    • The behaviour differs between secure and insecure decoders (the TODO mentioned above). Insecure decoders aren't released while secure ones are.
  2. When the current decoder can't play the following content

(1) is fixed by removing the TODO, but that change breaks when resuming stopped media using foreground mode - so it's not an easy fix. I think it's still worth pursuing, but it may take some time to come up with the right solution. I'm going to repurpose this issue to track it.

As I mentioned, (2) isn't really solvable by ExoPlayer in the general case.

There may be other cases where the decoder is released during the transition - as we discover them I'll track them as separate issues if it seems like there's an ExoPlayer change we can make to resolve it.

Other approaches/related work

Looking into the future, I think we should also be asking device manufacturers/SoC vendors if the 'black flash' can be avoided. This obviously doesn't help for all the devices already out in the wild with this behaviour, but it would help reduce the frequency of the problem, and mean that we can detach the surface without triggering a flash.

@icbaker icbaker changed the title MediaItem.Builder#setDrmSessionForClearPeriods is ignored if DRM UUID is not set + secure decoder is unexpectedly re-initialized during playlist item transition Secure decoder is re-initialized during transition into content that starts with a discontinuity May 18, 2021
@jrocharodrigues
Copy link

Hi @icbaker , thanks for your answer, it's pretty clear for me now.

What version of ExoPlayer are you using? There have been some recent fixes in this area.

I was testing with 2.13.3, but now i tested with both 2.14.0 and dev-v2

In the 'good' case (no black flash) I'd expect to see a single videoDecoderInitialized at the start of playback and then a final videoDecoderReleased at the end, and not see any during the transition.
In the 'bad' case (black flash) I'd also expect to see videoDecoderReleased immediately followed by videoDecoderInitialized at the time of the transition.

Maybe i'm missing some change in my tests, because i see the logs:

videoDecoderInitialized
videoDecoderReleased

When transitioning between tracks, even when on the pixel 4, where i don't see the black screen (maybe it's to fast?)...

Besides the items in the playlist and setting the shutter color to transparent do i have to change anything else?
I've attached the patch file of my changes
transition_test_patch.txt

@icbaker
Copy link
Collaborator

icbaker commented May 18, 2021

Maybe i'm missing some change in my tests, because i see the logs:

videoDecoderInitialized
videoDecoderReleased

When transitioning between tracks, even when on the pixel 4, where i don't see the black screen (maybe it's to fast?)...

I would expect the video decoder re-initialization (or lack of) to be consistent across all devices (which is what you're seeing) - the difference between devices is about how they behave when the surface is detached from the secure decoder as part of this re-initialization. That's where it seems some devices (like the Pixel 4 and 3a XL that I've tested with) don't show a black screen, while other devices (like Chromecast with Google TV) do. In other words:

  • If you see the decoder re-initialized in the logs you might see a black flash (depending on the device).
  • But if you see a black flash you must see a decoder re-initialized in the logs (I'd be very surprised if the black flash occurred while keeping the decoder active with the surface attached - and it would probably need to be investigated as a completely separate problem).

Besides the items in the playlist and setting the shutter color to transparent do i have to change anything else?

I wasn't testing with a shutter color change, I just copied your JSON into the demo app and played it. Thanks for the patch, I just tried with the shutter color change too and still didn't see the decoder re-initialize at the transition though.

Would you be able to provide the zip file produced by adb bugreport after reproducing the black flash transition? Please either upload it here or send to dev.exoplayer@gmail.com using a subject in the format "Issue #1234" (where "#1234" should be replaced with this issue number.) Please also update this issue to indicate you’ve done this.

@jrocharodrigues
Copy link

@icbaker I think i might have found the difference.
I made some more tests and found two different behaviours:

  1. Transition between item by using the nextTrack/previousTrack controls

    • Logs show:
    videoDecoderInitialized
    videoDecoderReleased
    
    • Android Tv Devices shows black screen (pixel 4 doesn't)
  2. Automatic transition by waiting for the first item to end

    • Logs don't the videoDecoderReleased log

    • No black screens on any devices (as expected)

I was testing the transition by using the nextTrack/previousTrack controls to skip to the next/previous item, because it is closer to the real use case of my app (tune next/previous live channel).

Is this the expected behaviour? Do you also see video decoder being release when using the next track button?

@icbaker
Copy link
Collaborator

icbaker commented May 20, 2021

Ah! I was only ever letting the transition happen during normal playback rather than using next/previous track :)

Using the next/previous track buttons I see the secure decoder released/re-initialized at the transition point (and this doesn't happen with an insecure decoder). I think this is because when seeking to a different period we always disabled & re-enable the decoder. I'm not 100% sure this is discontinuity related though - I'll tweak the issue title to reflect that.

Removing the TODO and associated logic, so secure & insecure decoders are treated the same (as mentioned at the bottom of #8842 (comment)) avoids this re-initialization, so should fix the 'black flash' too.

Unfortunately as I said above, that breaks other things - so there's more work needed here before we can make that change.

@icbaker icbaker changed the title Secure decoder is re-initialized during transition into content that starts with a discontinuity Secure decoder is re-initialized if renderer is disabled during a transition (e.g. due to seeking or a discontinuity) May 20, 2021
@jrocharodrigues
Copy link

@icbaker Ok, thank you.
Glad we're finally aligned in terms of test results :)
I'll keep watching the issues o track the progress.
Best regards

ojw28 pushed a commit that referenced this issue May 27, 2021
This change introduces a third 'state' for `DefaultDrmSessionManager`:
It's been fully released (prepareCount == 0) but at least one of its
sessions is still active.

In this state new acquisitions are rejected (`(pre)acquireSession()`
calls will fail) but the machinery to support the existing sessions
(ExoMediaDrm and MediaDrmHandler) is kept until they're all released.

This change will allow us to remove the TODO in MediaCodecRenderer
that resolves Issue: #8842.

PiperOrigin-RevId: 376193952
ojw28 pushed a commit that referenced this issue Jun 1, 2021
A renderer is disabled (without being reset) in two situations:
* When transitioning into a period that starts with a discontinuity
* When stopping the player with setForegroundMode(true)

Before this change the behaviour of `MediaCodecRenderer` when disabled
(but not reset) depended on whether the content being decoded had an
associated `DrmSession`:
* For content without an associated DRM session the MediaCodec instance
  was kept alive.
* For content with an associated DRM session, the MediaCodec instance
  was released. This was to prevent the DRM session from staying alive
  and continuing to make license refresh network requests while the
  player was stopped in 'foreground mode'.

This change removes the second bullet, and keeps MediaCodec instances
alive in both the secure and insecure case. This will result in the
DRM machinery making occasional license refresh network requests (at
a frequency defined by the license policy) while the player is stopped
and in 'foreground mode'. This network usage is considered to be a
'limited resource' as described by the `ExoPlayer#setForegroundMode`
javadoc.

This means that switches between secure content (or between secure and
clear content when `MediaItem.drmConfiguration.sessionForClearTypes`
indicates a secure decoder should be used for clear content) should
keep the same video decoder, thus avoiding the 'black flash' that occurs
on some devices when switching the surface away from a secure decoder.

Issue: #8842

#minor-release

PiperOrigin-RevId: 376825501
@icbaker
Copy link
Collaborator

icbaker commented Jun 2, 2021

This is fixed by the commits referenced above - i'm hoping to include them in the 2.14.1 release.

@icbaker icbaker closed this as completed Jun 2, 2021
ojw28 pushed a commit that referenced this issue Jun 10, 2021
This change introduces a third 'state' for `DefaultDrmSessionManager`:
It's been fully released (prepareCount == 0) but at least one of its
sessions is still active.

In this state new acquisitions are rejected (`(pre)acquireSession()`
calls will fail) but the machinery to support the existing sessions
(ExoMediaDrm and MediaDrmHandler) is kept until they're all released.

This change will allow us to remove the TODO in MediaCodecRenderer
that resolves Issue: #8842.

PiperOrigin-RevId: 376193952
ojw28 pushed a commit that referenced this issue Jun 10, 2021
A renderer is disabled (without being reset) in two situations:
* When transitioning into a period that starts with a discontinuity
* When stopping the player with setForegroundMode(true)

Before this change the behaviour of `MediaCodecRenderer` when disabled
(but not reset) depended on whether the content being decoded had an
associated `DrmSession`:
* For content without an associated DRM session the MediaCodec instance
  was kept alive.
* For content with an associated DRM session, the MediaCodec instance
  was released. This was to prevent the DRM session from staying alive
  and continuing to make license refresh network requests while the
  player was stopped in 'foreground mode'.

This change removes the second bullet, and keeps MediaCodec instances
alive in both the secure and insecure case. This will result in the
DRM machinery making occasional license refresh network requests (at
a frequency defined by the license policy) while the player is stopped
and in 'foreground mode'. This network usage is considered to be a
'limited resource' as described by the `ExoPlayer#setForegroundMode`
javadoc.

This means that switches between secure content (or between secure and
clear content when `MediaItem.drmConfiguration.sessionForClearTypes`
indicates a secure decoder should be used for clear content) should
keep the same video decoder, thus avoiding the 'black flash' that occurs
on some devices when switching the surface away from a secure decoder.

Issue: #8842

#minor-release

PiperOrigin-RevId: 376825501
@jrocharodrigues
Copy link

Just tested it and it works very well :) Thank you

@google google locked and limited conversation to collaborators Aug 2, 2021
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