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

ExoPlayer stuck in buffering after re-adding the surface view a few time #2703

Closed
PaulWoitaschek opened this issue Apr 19, 2017 · 11 comments
Closed
Labels

Comments

@PaulWoitaschek
Copy link

PaulWoitaschek commented Apr 19, 2017

ExoPlayer gets stuck in buffering after re-adding the surface view a few time.

I created a minimal example that demonstrates that problem. I used ExoPlayer 2.3.1 and tested it on a Nexus 5X with Android 7.1.2 and on a Moto G4 Play with Android 6.

I found this issue because I am removing the surface from ExoPlayer in a recyclerview when the view gets recycled but while trying to find the cause of this I created this demo without all the complexity.

It just prepares an ExoPlayer and when the user clicks on the screen it attaches / detaches the SurfaceView.

I logged all events and it gets stuck in buffering state:

04-19 08:11:47.199 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=idle
04-19 08:11:47.513 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=buffering
04-19 08:11:47.888 D/player: onTimelineChanged
04-19 08:11:47.941 D/player: onLoadingChanged
04-19 08:11:49.023 D/player: onTracksChanged
04-19 08:11:50.001 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=ready
04-19 08:11:55.792 D/player: onLoadingChanged
04-19 08:12:05.599 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=buffering
04-19 08:12:05.664 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=ready
04-19 08:12:09.579 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=buffering
04-19 08:12:09.623 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=ready
04-19 08:12:12.021 D/player: onPlayerStateChanged with playWhenRead=true and playbackState=buffering
class MainActivity : AppCompatActivity() {

  override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)

    val container = findViewById(android.R.id.content) as ViewGroup

    val surface = SurfaceView(this)
    container.addView(surface)

    val videoTrackSelectionFactory = AdaptiveTrackSelection.Factory(DefaultBandwidthMeter())
    val trackSelector = DefaultTrackSelector(videoTrackSelectionFactory)
    val player = ExoPlayerFactory.newSimpleInstance(this, trackSelector, DefaultLoadControl())
    player.addListener(object : ExoPlayer.EventListener {
      override fun onTracksChanged(trackGroups: TrackGroupArray?, trackSelections: TrackSelectionArray?) {
        Log.d("player", "onTracksChanged")
      }

      override fun onPlayerError(error: ExoPlaybackException?) {
        Log.d("player", "onPlayerError")
      }

      override fun onPlayerStateChanged(playWhenReady: Boolean, playbackState: Int) {
        val state = when (playbackState) {
          ExoPlayer.STATE_BUFFERING -> "buffering"
          ExoPlayer.STATE_ENDED -> "ended"
          ExoPlayer.STATE_READY -> "ready"
          ExoPlayer.STATE_IDLE -> "idle"
          else -> "unknownState$playbackState"
        }
        Log.d("player", "onPlayerStateChanged with playWhenRead=$playWhenReady and playbackState=$state")
      }

      override fun onLoadingChanged(isLoading: Boolean) {
        Log.d("player", "onLoadingChanged")
      }

      override fun onPositionDiscontinuity() {
        Log.d("player", "onPositionDiscontinuity")
      }

      override fun onTimelineChanged(timeline: Timeline?, manifest: Any?) {
        Log.d("player", "onTimelineChanged")
      }
    })


    val dataSourceFactory = DefaultDataSourceFactory(this, packageName)
    val dashUri = Uri.parse("http://www-itec.uni-klu.ac.at/ftp/datasets/DASHDataset2014/BigBuckBunny/15sec/BigBuckBunny_15s_simple_2014_05_09.mpd")
    val mediaSource = DashMediaSource(dashUri, dataSourceFactory, DefaultDashChunkSource.Factory(dataSourceFactory), null, null)
    player.prepare(mediaSource)
    player.setVideoSurfaceView(surface)
    player.playWhenReady = true

    container.setOnClickListener {
      if (container.childCount == 0) container.addView(surface)
      else container.removeView(surface)
    }
  }
}
@ojw28
Copy link
Contributor

ojw28 commented Apr 19, 2017

Any chance of a simple example in regular Java? Working out an unfamiliar (albeit fairly obvious, by the looks of things) language and having to mess around installing Android Studio extensions isn't that efficient for us :). It's also worth trying your sample against the latest dev-v2, as we've made some changes around surface handling there (with a few more yet to land).

@PaulWoitaschek
Copy link
Author

PaulWoitaschek commented Apr 19, 2017

I could not find any snapshot repo. Here the top version in Java. Its really trivial and just adds and removes views like I described.

public class A2 extends AppCompatActivity {

  protected void onCreate(@Nullable Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    
    final ViewGroup container = (ViewGroup) this.findViewById(android.R.id.content);
    final SurfaceView surface = new SurfaceView(this);
    container.addView(surface);
    
    Factory videoTrackSelectionFactory = new Factory(new DefaultBandwidthMeter());
    final DefaultTrackSelector trackSelector = new DefaultTrackSelector(videoTrackSelectionFactory);
    DefaultLoadControl loadControl = new DefaultLoadControl();
    SimpleExoPlayer player = ExoPlayerFactory.newSimpleInstance(this, trackSelector, loadControl);
    
    player.addListener(new EventListener() {
      public void onTracksChanged(@Nullable TrackGroupArray trackGroups, @Nullable TrackSelectionArray trackSelections) {
        Log.d("player", "onTracksChanged");
      }

      public void onPlayerError(@Nullable ExoPlaybackException error) {
        Log.d("player", "onPlayerError");
      }

      public void onPlayerStateChanged(boolean playWhenReady, int playbackState) {
        String state;
        switch (playbackState) {
          case ExoPlayer.STATE_IDLE:
            state = "idle";
            break;
          case ExoPlayer.STATE_BUFFERING:
            state = "buffering";
            break;
          case ExoPlayer.STATE_READY:
            state = "ready";
            break;
          case ExoPlayer.STATE_ENDED:
            state = "ended";
            break;
          default:
            state = "unknownState" + playbackState;
        }
        Log.d("player", "onPlayerStateChanged with playWhenRead=" + playWhenReady + " and playbackState=" + state);
      }

      public void onLoadingChanged(boolean isLoading) {
        Log.d("player", "onLoadingChanged");
      }

      public void onPositionDiscontinuity() {
        Log.d("player", "onPositionDiscontinuity");
      }

      public void onTimelineChanged(@Nullable Timeline timeline, @Nullable Object manifest) {
        Log.d("player", "onTimelineChanged");
      }
    });
    
    DefaultDataSourceFactory dataSourceFactory = new DefaultDataSourceFactory(this, this.getPackageName());
    Uri dashUri = Uri.parse("http://www-itec.uni-klu.ac.at/ftp/datasets/DASHDataset2014/BigBuckBunny/15sec/BigBuckBunny_15s_simple_2014_05_09.mpd");
    DashMediaSource mediaSource = new DashMediaSource(dashUri, dataSourceFactory, new com.google.android.exoplayer2.source.dash.DefaultDashChunkSource.Factory(dataSourceFactory), null, null);
    player.prepare(mediaSource);
    player.setVideoSurfaceView(surface);
    player.setPlayWhenReady(true);
    
    container.setOnClickListener(new OnClickListener() {
      public final void onClick(View it) {
        if (container.getChildCount() == 0) {
          container.addView(surface);
        } else {
          container.removeView(surface);
        }
      }
    });
  }
}

@ojw28
Copy link
Contributor

ojw28 commented Apr 19, 2017

Thanks. I cannot reproduce the issue with the latest dev-v2. Could you give that a try to see if you see the same behavior there? I just pushed some recent changes to dev-v2 to make sure you have all of the same changes I tested with, so be sure to pull them if you give it a try.

@gajendrakumartwinwal
Copy link

gajendrakumartwinwal commented Apr 24, 2017

Facing same issue while reattching playerview after detached from parent ViewGroup.
Even i have tried on your lasted development branch by changing below code you can reproduce the same by replacing two file.
Steps:

  1. Download the zip file then extract it. there are two file in the zipped folder
  2. I have changed these two files in your dev bracnch "dev-V2-r2.3.1" on date "24/04/17"
  3. So just replace these two file in your "dev-V2-r2.3.1" branch and then
  4. Just play any video and press dettachandattach button that i haved added in xml and see the video and audio syncing issue.

Would be great help if you can short it out. Waiting for your replay.

PFA
ExoplayerChanges.zip

@PaulWoitaschek
Copy link
Author

Any news on this?

I made a sample project here:

https://github.com/PaulWoitaschek/ExoPlayerAttachingBug/blob/master/app/src/main/java/de/paul_woitaschek/exoplayerbug/MainActivity.java

It immediately starts a video. When you click on the screen it removes the view. When you click it again it re-adds the view.

After a few times the video stops playing.
Tested with ExoPlayer 2.4.0 and Nexus 5x, Android 7.1.2, + Android Emulator, latest android

@PaulWoitaschek
Copy link
Author

It's a real blocker for me as I can't use ExoPlayer inside a RecyclerView as it does exactly that. Any more info I can provide?

@ojw28
Copy link
Contributor

ojw28 commented May 10, 2017

I took a look at this and was able to reproduce, thanks. It's a fundamental limitation of MediaCodec (in the Android platform) that it needs a Surface attached to it at all times. When the SurfaceView is removed from the view the underlying Surface is destroyed. This in turn forces ExoPlayer to destroy its MediaCodec instance, because there's no longer a Surface for it to be attached to.

When the SurfaceView is added back to the view hierarchy its underlying Surface is created again. ExoPlayer creates a new MediaCodec instance, however since it's a new instance it can only start decoding from the next key-frame. It does this and displays the next key-frame immediately, then waits for the playback position to reach the position of that key-frame before rendering subsequent frames. This is why even after a single remove/add cycle, you observe that the first frame that's displayed is from the future and frozen for a while.

If you remove/add multiple times the player skips ahead 1 key-frame each time. If you do this enough, the player gets stuck in a weird state where it's buffered a long way into the future compared to the correct playback position, but by successively rendering all of the key-frames it actually doesn't have any frame to render. The player doesn't think it needs to buffer, but it's also in a state where it cannot transition to the playing state because it hasn't rendered a frame.

The fundamental limitation of MediaCodec is a real pain, unfortunately. There are however things we can do in the library, and things you can do in your application:

On the library side:

  • We should delay rendering the next key-frame until the playback position actually reaches the corresponding time. This will change the behavior from seeing that frame frozen for a while to seeing a blank surface for a while. Neither is a good user experience, but the latter has the benefit that the video renderer is prevented from skipping arbitrarily far ahead as a result of multiple remove/add cycles. This will prevent the stuck buffering state and allow playback to always continue properly from the next key-frame regardless of how many remove/add cycles occur, so will in general produce more predictable behavior.

On the application side, there are a bunch of things you can do to try and provide a better user experience.

  • On API level 23 and above you can solve this problem using DummySurface, which we recently added to the library. This gives proper seamless re-join. It wont work prior to API level 23, because the approach relies on MediaCodec.setOutputSurface. The basic idea is to use the DummySurface as a means of ensuring there's always a surface attached to the MediaCodec. Rather than calling player.setVideoSurfaceView, manage the Surface lifecycle yourself. Something like this should work well from your activity:
    SurfaceManager manager = new SurfaceManager(player);
    surface.getHolder().addCallback(manager);

With:

 private static final class SurfaceManager implements SurfaceHolder.Callback {

    private final SimpleExoPlayer player;
    private DummySurface dummySurface;

    public SurfaceManager(SimpleExoPlayer player) {
      this.player = player;
    }

    @Override
    public void surfaceCreated(SurfaceHolder holder) {
      player.setVideoSurface(holder.getSurface());
    }

    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
      // Do nothing.
    }

    @Override
    public void surfaceDestroyed(SurfaceHolder holder) {
      if (dummySurface == null) {
        dummySurface = DummySurface.newInstanceV17(false);
      }
      player.setVideoSurface(dummySurface);
    }

    public void release() {
      if (dummySurface != null) {
        dummySurface.release();
        dummySurface = null;
      }
    }

  }

In the future, we hope to be able to hook DummySurface up automatically inside of the player for cases where it helps.

  • For pre-API-23 there are complicated solutions involving off-screen rendering into GL textures. It would take quite a bit of time and effort to put together sample code, however.

  • One easier option is to disable and re-enable the video renderer, but this causes re-buffering and so isn't ideal. Nevertheless, you can do it something like this:

  private static final class SurfaceManager implements SurfaceHolder.Callback {

    private final SimpleExoPlayer player;
    private final DefaultTrackSelector trackSelector;

    public SurfaceManager(SimpleExoPlayer player, DefaultTrackSelector trackSelector) {
      this.player = player;
      this.trackSelector = trackSelector;
    }

    @Override
    public void surfaceCreated(SurfaceHolder holder) {
      player.setVideoSurface(holder.getSurface());
      trackSelector.setRendererDisabled(VIDEO_RENDERER_INDEX, false);
    }

    @Override
    public void surfaceChanged(SurfaceHolder holder, int format, int width, int height) {
      // Do nothing.
    }

    @Override
    public void surfaceDestroyed(SurfaceHolder holder) {
      player.setVideoSurface(null);
      trackSelector.setRendererDisabled(VIDEO_RENDERER_INDEX, true);
    }

  }

@PaulWoitaschek
Copy link
Author

Thanks for your response! What is VIDEO_RENDERER_INDEX? I'll try the combination of these 2 SurfaceManagers tomorrow.

The optimal behaviour would be for the playback just to immediately continue and not stuck. Also after each attach / detach - scroll up / down there is a different frame when paused which looks quite awkward.

@ojw28
Copy link
Contributor

ojw28 commented May 11, 2017

It's the index of the video renderer that you're disabling. You can query the type of each renderer using ExoPlayer.getRendererType(), which should allow you to figure out which index to use.

@ojw28
Copy link
Contributor

ojw28 commented May 11, 2017

We're going to use #677 to track the library side change mentioned above.

@ojw28
Copy link
Contributor

ojw28 commented May 24, 2017

Closing because #677 is tracking the library side change.

@ojw28 ojw28 closed this as completed May 24, 2017
@google google locked and limited conversation to collaborators Sep 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants