OpenSageTV / sagetv-miniclient

SageTV MiniClient for Android TV (and phone/tablet)
http://forums.sagetv.com/forums/showthread.php?t=62439
Apache License 2.0
19 stars 14 forks source link

When stopping video playback on live content, ijkplayer freezes and doesn't recover. #54

Closed enternoescape closed 8 years ago

enternoescape commented 8 years ago

I finally got around to installing this on my Nexus Player with a debugger attached. This issue was exactly what I was thinking it was. ijkplayer continues to ask for more data from the circular buffer and no data is available to be consumed. It looks like stopping playback has an unintentional circular dependency. The thread that eventually closes the circular buffer gets hung up on player.reset() in the method releasePlayer() in sagex/miniclient/android/video/ijkplayer/IJKMediaPlayerImpl.java because it is waiting for the last read to come back.

To test my idea, I placed just before player.reset(), dataSource.close(), and then modified sagex/miniclient/net/PushBufferDataSource.java to run the release() method. After making these changes the player no longer hangs no matter how close I am to live.

The reason I'm talking about this instead of just posting a patch or pull request is because there's a log entry in the close method under PushBufferDataSource.java that says "close() for a push is ignored, until the release() is called." It makes me think what I did could be a little hacky and I wanted to know if my changes could be implemented at a better location or if there are some consequences of this change that I wouldn't immediately be aware of.

Attached is a patch with the changes I made if you don't want to decipher what I'm describing in the second paragraph. ijkplayer_live_hang.patch.txt

stuckless commented 8 years ago

I'll review the DataSource stuff... I'm thinking that during a round of refactoring the .release() (and .close() ) is no longer called, and that's not right. This code has gone through a lot of refactoring to make it work with both ExoPlayer and IJKPlayer. Even the .release() doesn't appear to be called :(

The problem with the .close() was that ExoPlayer would end up calling .close() on every sync operation, and then call .open() again. For a Push, though, this shouldn't matter (I think).

Either way, I think you've uncovered some inconsistencies with the Push release/close handling... and I'm not sure I need the .close() and .release() methods any more.

stuckless commented 8 years ago

I'm back to this :) So there is a releaseDatasource() method that is responsible for releasing the datasource. in releasePlayer, after the stop() and before the reset, I'm now calling releaseDatasource(). this this should fix this issue. Unfortunately because the datasources are used by ExoPlayer and it has a very different datasource lifecycle, I can't just release the datasource on close.

enternoescape commented 8 years ago

That fixed it. Thanks!