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

Come up with good solution for attaching/detaching surface from video renderers. #1520

Closed
moagrius opened this issue May 10, 2016 · 7 comments

Comments

@moagrius
Copy link

moagrius commented May 10, 2016

Great work, great library, thanks for all your hard work.

A description of the issue itself.

This was first discovered when using ExoPlayer and backgrounded video (audio continues to play in a Service while the Activity is paused). We found that on most devices, the onPlaybackStateChange callback for STATE_ENDED was not called when the Activity was paused, but would fire as soon as the Activity resumed. On some devices (e.g., HTC One X 4.1.1), STATE_ENDED happened as expected, even with the Activity backgrounded/paused.

For example:

  1. Create an Activity that has a TextureView.
  2. Create a Service that creates an ExoPlayer instance.
  3. Start the Service from the Activity, setting the surface of the ExoPlayer to the TextureView. Attach a Listener to the ExoPlayer that logs out state changes.
  4. Play any video.
  5. Allow the video to get near the end, then hit the HOME button.
  6. The audio continues, but the onPlaybackStateChange callback for STATE_ENDED does not fire (on most devices).
  7. Resume the Activity (select it from the multi-task / app switcher).
  8. The onPlaybackStateChange callback for STATE_ENDED will now fire as the Activity resumes.

The cause is:

  1. ExoPlayerImplInternal calls doSomeWork, where it checks if each renderer is "done" by calling isEnded on each renderer. If all renderers report that they're ended (and meet some other conditions), it fires onPlaybackStateChange with STATE_ENDED.
  2. MediaCodecTrackRenderer's isEnded method returns outputStreamEnded, which is apparently not set to true in most devices, for MediaCodeVideoTrackRenderer instances, if there is not a surface to send to. When the surface is replaced, it correctly (and immediately) reports outputStreamEnded/isEnded to be true, so doSomeWork does indeed call onPlaybackStateChange to STATE_ENDED.

Steps describing how the issue can be reproduced, ideally in the ExoPlayer demo app.

  1. Create an app using a VideoPlaybackActivity https://gist.github.com/moagrius/4732021bf4ca5c8957b9e8a15429e2a5
    and a BackgroundAudioService https://gist.github.com/moagrius/f2ca1d2d5c451d8b862304e46caf69f1
  2. Add a video to assets. Update the path in VideoPlaybackActivity to that video.
  3. Run the VideoPlaybackActivity.
  4. Allow the video to get near the end, then hit the HOME button.
  5. The audio continues, but the onPlaybackStateChange callback for STATE_ENDED does not fire (on most devices).
  6. Resume the Activity (select it from the multi-task / app switcher).
  7. The onPlaybackStateChange callback for STATE_ENDED will now fire as the Activity resumes.
  8. Comment out or remove the line that set the surface to null in VideoPlaybackActivity.onSurfaceTextureDestroyed
  9. Repeat steps 3 and 4. Now, since the surface was not made unavailable to the video renderer, the onPlaybackStateChange will fire for STATE_ENDED while the Activity is paused.

If the issue only reproduces with certain content, a link to some content that we can use to reproduce. If you don’t wish to post a link publicly, please send the link by email to dev.exoplayer@gmail.com, including the issue number in the subject line.

n/a

The version of ExoPlayer being used.

1.5.7

The device(s) and version(s) of Android on which the issue can be reproduced, and how easily it reproduces. Does it happen every time, or only occassionally? If possible, please test on multiple devices and Android versions. If the issue does not reproduce on some combinations, that’s useful information!

We've tested on multiple devices, including Nexus 5, Nexus 6, Nexus 7, Samsung Galaxy 6, and HTC One X. So far, only the HTC One X on 4.1.1 did not exhibit the behavior (the video track renderer ended with or without a surface to draw too). It is predictable and reproducible (it never happens on the HTC One X, and always happens on the other devices).

A full bug report taken from the device just after the issue occurs, attached to the issue as a file. A bug report can be captured from the command line using adb bugreport > bugreport.txt. Note that the output from adb logcat or a log snippet is typically not sufficient.

n/a

Anything else you think might be relevant!

The proper way to get around this is to re-use the SurfaceTexture, so that it's always available to the MediaCodeVideoTrackRenderer, even if it's not onscreen, and return false from onSurfaceTextureDestroyed so that it is not released. That said, the behavior described above does seem like it could be corrected to behave like I think most users would expect (the audio renderer correctly reported when it had ended, which is when the "media item" as a single entity had concluded - I believe it should have reported STATE_ENDED at that time).

Thanks, and let me know if you need any more information.

@ojw28
Copy link
Contributor

ojw28 commented May 10, 2016

  • An enabled MediaCodecVideoTrackRenderer with no surface will not be able to consume video from the buffer. This will result in buffered video accumulating over time. Eventually, this will cause playback to hang due to the buffer being entirely filled with unconsumed video. So there are fairly fundamental issues with what you're trying to do, in addition to the issue you mention.
  • To solve this, you should be disabling the video renderer when the app moves to the background and enabling it again when the app moves back to the foreground. The demo app does this here. Note that since the isEnded checks are only performed on enabled renderers, the video renderer is not then taken into account to determine whether playback has finished.

@ojw28 ojw28 added the question label May 10, 2016
@moagrius
Copy link
Author

moagrius commented May 10, 2016

Thanks for the quick reply. That all makes sense. That said, I think most people's instinct will probably be to do the same thing we did initially - expect STATE_ENDED to happen when the app is backgrounded and the audio track ends. Can you think of anything that might make managing this either a little more automatic, or more obvious, or do you feel like it's really the user's responsibility to be aware and manage this?

@ojw28
Copy link
Contributor

ojw28 commented May 10, 2016

We may well do something to make it more obvious/automatic (probably the latter) in 2.x. Out of interest, what type of media are you playing (DASH/SmoothStreaming/HLS/SomethingElse)?

@moagrius
Copy link
Author

We may well do something to make it more obvious/automatic (probably the latter) in 2.x.

Awesome

Out of interest, what type of media are you playing (DASH/SmoothStreaming/HLS/SomethingElse)?

"Standard" media (H.264 MP4s)

@ojw28 ojw28 changed the title MediaCodecVideoTrackRenderer inconsistently reporting isEnded so onPlaybackStateChange STATE_ENDED does not occur Come up with good solution for attaching/detaching surface from video renderers. May 10, 2016
@spalmerin21
Copy link

I also noticed never initializing the videorenderer and just the audiorenderer will trigger the state_ended callback when the activity is paused and the service is running in the background.

Tested on a Nexus 6p running 6.0.1.

@NtcWai
Copy link

NtcWai commented Apr 19, 2017

@ojw28 how to solve this problem in version 2 ? thank you!

@ojw28
Copy link
Contributor

ojw28 commented May 11, 2017

I'm pretty sure this if fixed in 2.4.0. An enabled MediaCodecVideoTrackRenderer with no surface is now able to consume video, and the ended state should be reported correctly. There are some related issues around seamless switching from no surface to a surface (and from one surface to another) that are largely platform limitations. We'll use #677 to track trying our best to overcome those.

@ojw28 ojw28 closed this as completed May 11, 2017
@google google locked and limited conversation to collaborators Sep 9, 2017
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