Disable tunneling on internal Surface related Exceptions #9620
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Redirion
added
player
|
Tested on my Sony A90J. Pressing full screen on normal release apk switches to fullscreen, stops playing and shows error toast. Pressing fullscreen on this prs apk does not switch to fullsceen, shows error toast but keeps playing audio in the background. Manually disabling media tunneling on debug builds gets the fullscreen toggle working on this tv. |
|
Thank you for testing! Is it possible to check the logs? I would be interested about the logs around the line "ExoPlayer - onPlayerError() called with:" |
|
I have also tried the artifact from this PR on an Android TV by Philips. The result is similar to what @peat80 has described. The original issue with the full-screen playback is still present, although the audio keeps playing for some reason. According to the logs, the subsequent crash seems to be related to the tunneling. Log from unsuccessful attempt to switch to the full-screen mode:Log from the subsequent crash when attempting to resume playback: |
|
Thank you @pbasista btw. please refrain commenting NewPipe issues on Google Repositories (ExoPlayer is Google). They naturally do not like what NewPipe does. |
Yes. The updated version produces the following log message: |
|
looks like the new function "isTunnelingEnabled" of ExoPlayer 2.18.2 is not usable in this context. I have changed the check. Could you try yet again please? |
|
The application now does not crash when attempting to play the video again. It still would not play though. There is a notification on the screen suggesting to check the logs. Here are the logs from an unsuccessful attempt to resume playback in full-screen mode.The mentioned stack trace is very similar to the one which causes the playback in full-screen mode to fail for the first time. It appears to be originating from the same native code in |
|
okay, I am a bit confused. "onPlaybackShutdown()" should not have been called. It should retry when detecting that the stacktrace contains the characters "Surface", which it does. I have added some more logging again to log whether the conditions for recovery are actually met. Could you test again please? Thanks again. |
Yes. With the new version, the behavior remains the same. But the logs now include the added information. Logs from an unsuccessful attempt to resume playback in full-screen mode. |
|
so we have: Both ways to check if tunneling is enabled (to be able to automatically disable it and retry) do not work. This makes this PR useless. A different approach will be needed. I will keep this PR open a bit, but I am afraid that this approach is a dead end. |
|
I have a very limited understanding of what the goal of this PR is. But ExoPlayer's most recent release notes specifically mention a newly added method If I add this line to you branch: --- a/app/src/main/java/org/schabi/newpipe/player/Player.java
+++ b/app/src/main/java/org/schabi/newpipe/player/Player.java
@@ -1426,6 +1426,7 @@ public final class Player implements PlaybackListener, Listener {
// Try to handle tunneling related exceptions
final String stackTrace = Log.getStackTraceString(error.getCause());
Log.d(TAG, "Unexpected PlaybackException! Cause: " + stackTrace);
+ Log.d(TAG, "Tunneling enabled: " + simpleExoPlayer.isTunnelingEnabled());
if (trackSelector.getParameters().tunnelingEnabled
&& stackTrace.contains("Surface")) {
trackSelector.setParameters(trackSelector.buildUponParameters()It produces the following output: |
|
that was, what I initially had... strange.
|
|
Calling After that, when the same ExoPlayer exception occurs for the second time, then calling |
|
not sure, maybe we just need to call prepare() again. Can you try again what happens now? |
|
The updated version no longer causes the entire application to crash when toggling the full-screen mode. During the first attempt, the After the first attempt, the audio keeps playing. There are some audio-related messages in the log which might help with understanding the root cause of this behavior. For instance: Full log from two subsequent attempts to toggle the full-screen mode. |
|
Kudos, SonarCloud Quality Gate passed! |
|
Latest commit does help here. First video plays windowed and second video shows an error toast but plays in fullscreen instead of crashing the app here on my bravia a90j. |
I have also tried it now on an Android TV manufactured by Philips. The behavior in my case was that there was an error toast with a note to check the logs for exception. The application did not crash. But the video playback stopped and the player window was replaced with a non-interactive view of its thumbnail with an overlay of video title, duration and last playback position. Exception detail |
|
This is the behaviour i get when try to manually press the fullscreen button on the running video. If i navigate to the right and choose a second video it plays fullscreen. |
|
Closing, as users have the control on this setting on every device and on every build variant since #8875 has been merged. This PR also doesn't seem do anything useful in the end unfortunately. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is it?
Description of the changes in your PR
This PR tries to automatically disable media tunneling on unrecoverable internal player exceptions. Analysis was possible thanks to the stack trace provided in this comment: #9023 (comment)
In the past we tried to disable media tunneling on devices that claim to support it but fail to do so. This was not only device specific but also frustrating for users that would have to create an issue on Github to hope for help with a new version release. With the new ExoPlayer Settings page that is still a PR (#8875) we have a general approach. However most users would still first create an issue before they would learn about these settings. So an automatic recovery for at least some of the devices seems useful.
Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence