search

TeamNewPipe / NewPipe

A libre lightweight streaming front-end for Android.
https://newpipe.net
GNU General Public License v3.0
31.84k stars 3.08k forks source link

Crash when going fullscreen (especially Android TVs) #9023

Closed dolorosus closed 1 year ago

dolorosus commented 2 years ago

Checklist

Affected version

v0.24.0

Steps to reproduce the bug

  1. Start any video and try to switch to fullscreen mode results in an error message (see screenshot).
  2. No chance to continue the video, 'cause only an remote control is attached, but got no focus to the play button. Only chance is going back and restart the video which brings us back 1.

Expected behavior

entering fullscreen mode

Actual behavior

see attached [screenshot]

Screenshots/Screen recordings

Screenshot 2022-09-25 210614

Logs

No response

Affected Android/Custom ROM version

AndroidTV 9 and FireTV

Affected device model

No response

Additional information

No response

BartFarley commented 2 years ago

I don't get it. You folks are trying to figure out where this bug came from, and theorizing what might fix it. I don't understand why you don't go right to the code from the previous version which didn't have this full screen bug. 23.3

What changed? Who changed it? Who rammed 24.0 through even though the code for the main player was "experimental"

I know who. We've seen this behavior before.

BTW, it's not an Android TV thing. Same bug on my everyday old tablet. Won't "Play All" on playlists. Deja Vu

opusforlife2 commented 2 years ago

I don't understand why you don't

@BartFarley Probably because it's far more complicated than you think. A hundred little things are changed between releases. At the time of merging code, you're only looking at one thing, but after a bunch of other PRs are merged, it's not easy at all to cleanly change back the code.

Who rammed 24.0 through

The release candidate for this version was left out for a ridiculously long period. Far longer than most other releases of the app. It's not our fault the majority of users don't come help test release candidates, and only show up later to complain when things go bad. It's no one's fault actually, since this is a FOSS app and we're volunteers. You could volunteer to help test as well.

I know who. We've seen this behavior before.

You're skirting personal attack territory. Don't.

elparra72 commented 2 years ago

Arguing is not fixing this issue. Something is clear. In version 23.4 it works properly. In version 24 there are two workarounds:

1.- Use an external video player as VLC. Resolution limiteebto 720p because audio is missed with higher resolutions (in my case)

2'- Set 3GP by default and change resolution when video starts.

Questions is:

Why full screen mode works in 3GP and not in other formats?

Probably this low resolution format was ignored and routine not modified for a long.

Is it possible to back to version 23.4 code and implement new improves without the part related to 'full screen' to avoid this issue?

Thee are more issues however solving one by one will be faster to get an stable release.

Thanks.

Obtener Outlook para Androidhttps://aka.ms/AAb9ysg

From: opusforlife2 @.> Sent: Tuesday, November 1, 2022 7:55:42 AM To: TeamNewPipe/NewPipe @.> Cc: elparra72 @.>; Comment @.> Subject: Re: [TeamNewPipe/NewPipe] No fullscreen possible with Android TV (Issue #9023)

I don't understand why you don't

@BartFarleyhttps://github.com/BartFarley Probably because it's far more complicated than you think. A hundred little things are changed between releases. At the time of merging code, you're only looking at one thing, but after a bunch of other PRs are merged, it's not easy at all to cleanly change back the code.

Who rammed 24.0 through

The release candidate for this version was left out for a ridiculously long period. Far longer than most other releases of the app. It's not our fault the majority of users don't come help test release candidates, and only show up later to complain when things go bad. It's no one's fault actually, since this is a FOSS app and we're volunteers. You could volunteer to help test as well.

I know who. We've seen this behavior before.

You're skirting personal attack territory. Don't.

— Reply to this email directly, view it on GitHubhttps://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1298101372, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A34HU7PENSFHCOFRQXBQLLTWGC5G5ANCNFSM6AAAAAAQVGWPY4. You are receiving this because you commented.Message ID: @.***>

Stypox commented 2 years ago

Could you try this one? app-debug.zip

It is based off a comparison of the code that runs once the fullscreen button is pressed in the current version and in the old version. The only real difference I could notice was in the VideoDetailFragment's addVideoPlayerView (old) vs tryAddVideoPlayerView (new) functions. I tried to add some code to remove the surface view from the parent as soon as possible in the new code, as it happened in the old code.

IMMalebolgia commented 2 years ago

Could you try this one? app-debug.zip

No change in behavior on my FireTV 4K Max, videos still start playing in the small window and do not automatically go into fullscreen mode as previous versions did. I am still able to select the fullscreen icon and that works for a video or 2 but then the app crashes shortly after the video starts.

This version also appears to be a bit more unstable and crashes out even compared to 24.0

j-seeley commented 2 years ago

Could you try this one? app-debug.zip

It is based off a comparison of the code that runs once the fullscreen button is pressed in the current version and in the old version. The only real difference I could notice was in the VideoDetailFragment's addVideoPlayerView (old) vs tryAddVideoPlayerView (new) functions. I tried to add some code to remove the surface view from the parent as soon as possible in the new code, as it happened in the old code.

I'll try it this weekend and report back. Been busy.

elparra72 commented 2 years ago

Same full screen issue. It looks like video listed in subscriptions are fine now. Tested in a Xiaomi P1 - AndroidTV 10

Obtener Outlook para Androidhttps://aka.ms/AAb9ysg

From: j-seeley @.> Sent: Saturday, November 5, 2022 5:04:17 AM To: TeamNewPipe/NewPipe @.> Cc: elparra72 @.>; Comment @.> Subject: Re: [TeamNewPipe/NewPipe] No fullscreen possible with Android TV (Issue #9023)

Could you try this one? app-debug.ziphttps://github.com/TeamNewPipe/NewPipe/files/9940447/app-debug.zip

It is based off a comparison of the code that runs once the fullscreen button is pressed in the current version and in the old version. The only real difference I could notice was in the VideoDetailFragment's addVideoPlayerView (old) vs tryAddVideoPlayerView (new) functions. I tried to add some code to remove the surface view from the parent as soon as possible in the new code, as it happened in the old code.

I'll try it this weekend and report back. Been busy.

— Reply to this email directly, view it on GitHubhttps://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1304396224, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A34HU7JZLJCTRR3AJYMKJW3WGXMEDANCNFSM6AAAAAAQVGWPY4. You are receiving this because you commented.Message ID: @.***>

dolorosus commented 2 years ago

If you got "no application can handle this action" when you try to import a backup, try to switch off SAF (Settings->Download "Storage Access Framework").

j-seeley commented 2 years ago

Could you try this one? app-debug.zip It is based off a comparison of the code that runs once the fullscreen button is pressed in the current version and in the old version. The only real difference I could notice was in the VideoDetailFragment's addVideoPlayerView (old) vs tryAddVideoPlayerView (new) functions. I tried to add some code to remove the surface view from the parent as soon as possible in the new code, as it happened in the old code.

I'll try it this weekend and report back. Been busy.

Nothing's working now. Darn. No videos playing in v 23.3 or your latest app-debug.zip. Seems YT is trying again to render alternatives useless.

NeloKa61 commented 2 years ago

I alreasy tested app-debug zip same behavior as the last one(0.24.0) now going to install 0.24.1 as new test on B860H V5.0 Android 9 TV.sorry late for update been busy week.

NeloKa61 commented 2 years ago

tested 0.24.1 ( I don't know this help or not) but same behavior Screenshot_20221108-090028 Half a screen is video player when full screen button is clicked.same response as app-debug.zip But the app-debug.zip work first time (entering full screen but cannot minimize once or I cannot enter fullscreen again(back to half video player screen)

glbyers commented 2 years ago

New to the thread, but i've been experiencing the same issue since 0.24.0 on a Sony Bravia TV. I've managed to enable debug mode on my TV & remotely connect via adb. I grabbed logs whilst I ran the last app-debug build posted above & went full screen, but it's very chatty so i've tried to extract relative logs (i'm reluctant to post entire debugs in a public forum). Does the attached help, or do you need more?

22205.txt

opusforlife2 commented 2 years ago

This is a very long shot, but could you all try https://github.com/TeamNewPipe/NewPipe/issues/9396#issuecomment-1312950357?

glbyers commented 2 years ago

This is a very long shot, but could you all try #9396 (comment)?

A long shot indeed. I spent many hours testing, which frequently involved complete removal including data and cache. So that's a no go from me. Sorry!

glbyers commented 2 years ago

OK. I've gotten a lot further along with my attempts to debug this. I've got Android Studio up and running, with debug builds sent directly to my TV. If I set a breakpoint on line 1258 of setVideoSurface in com.google.android.exoplayer2.ExoPlayerImpl.java & go fullscreen, the thread "ExoPlayer:PlaceholderSurface" halts, but video and audio will continue on in fullscreen with some of the controls still present (photo incoming). For reference, the exact line is;

setVideoOutputInternal(surface);

If I step over that method call, exoplayer will bomb out with the following (partial) stack trace;

E/Player: ExoPlayer - onPlayerError() called with:
    com.google.android.exoplayer2.ExoPlaybackException: Unexpected runtime error
        at com.google.android.exoplayer2.ExoPlayerImpl.setVideoOutputInternal(ExoPlayerImpl.java:2491)
        at com.google.android.exoplayer2.ExoPlayerImpl.setVideoSurface(ExoPlayerImpl.java:1258)
        at org.schabi.newpipe.player.playback.SurfaceHolderCallback.surfaceDestroyed(SurfaceHolderCallback.java:53)
        at android.view.SurfaceView.updateSurface(SurfaceView.java:715)
        at android.view.SurfaceView.onWindowVisibilityChanged(SurfaceView.java:270)
        at android.view.View.dispatchDetachedFromWindow(View.java:19620)
        at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3806)
        at android.view.ViewGroup.removeViewInternal(ViewGroup.java:5431)
        at android.view.ViewGroup.removeViewInternal(ViewGroup.java:5402)
        at android.view.ViewGroup.removeView(ViewGroup.java:5333)
        at org.schabi.newpipe.player.ui.MainPlayerUi.removeViewFromParent(MainPlayerUi.java:208)

Are there any specific members of the surface instance you'd like me to inspect to debug further at this breakpoint?

20221115_011635

IMMalebolgia commented 2 years ago

I am in a different location and did not have a Fire 4k Max at this address so I bought a new one to do the testing and went ahead and installed 0.24.1 as a baseline but oddly enough the automatic fullscreen is working on this device even though it did not work on the other Fire 4k Max. I don't know the exact OS version on the other one but this one is running: Fire OS 7.6.0.8 (PS7608/3614) 6370055.1

I will not be at the other location for several weeks but will do a full reset and fresh install of NewPipe to see if that has any effect there.

glbyers commented 2 years ago

I've resolved this in my debug build (from master branch) by disabling media tunneling. In Player.java, i've just flipped the boolean. Working flawlessly for me now.

        // enable media tunneling
        if (DEBUG && PreferenceManager.getDefaultSharedPreferences(context)
                .getBoolean(context.getString(R.string.disable_media_tunneling_key), false)) {
            Log.d(TAG, "[" + Util.DEVICE_DEBUG_INFO + "] "
                    + "media tunneling disabled in debug preferences");
        } else if (DeviceUtils.shouldSupportMediaTunneling()) {
            trackSelector.setParameters(trackSelector.buildUponParameters()
                    .setTunnelingEnabled(false));
        } else if (DEBUG) {
            Log.d(TAG, "[" + Util.DEVICE_DEBUG_INFO + "] does not support media tunneling");
        }
glbyers commented 2 years ago

FWIW, this has also fixed a long standing bug I've experienced since I've been running NewPipe (~6 months), where NewPipe almost always crashes when trying to launch a video after having just completed one; effectively meaning I've only ever been able to play a single video before it bombed out. It's no longer doing that.

glbyers commented 2 years ago

FWIW, others experiencing similar issues on non Bravia VH1 model TV, run the standard debug build and disable media tunneling in settings/debug & see if that fixes your issue.

AudricV commented 2 years ago

8875 will make this setting available on all devices and on all build types, it would be great if you can test it too.

peat80 commented 2 years ago

Probably a good idea to merge #8875 soon.

I have tested on my Sony Bravia A90J disabling the media tunnelling on a debug build also helps there. With tunneling enabled it will exit the app as soon as you start a second video. Without tunneling it works like on the shield where the first video does not start in fullscreen and if you open another one it goes in fullscreen as expected.

Stypox commented 2 years ago

@peat80 yes, that definitely needs to be done. Also, we should disable media tunneling by default on affected devices (like it happens currently). Anyway, media tunneling isn't really the problem as previous versions of NewPipe also used media tunneling, but though they worked without issues.

Here is another APK for you to test. I am really sorry for not having been able to solve this bug so far. The player code related to fullscreen handling is still a mess, sometimes I wonder whether everything should just be rewritten from scratch... Expect strange shenanigans with this APK (e.g. wrongly-sized controls), but let's hope that at least the player does not crash anymore when going fullscreen. app-debug.zip

peat80 commented 2 years ago

That debug build also crashes for me until i disable media tunneling by hand on my Bravia A90J. So this tv model should probably also blacklisted.

glbyers commented 2 years ago

Anyway, media tunneling isn't really the problem as previous versions of NewPipe also used media tunneling, but though they worked without issues.

That's not true, at least in my case on a Sony Bravia VH1. Whilst playing the first video has worked in previous versions, it almost always crashed on attempting to play a second video. I suspect others may have been experiencing a similar issue, but although a nuisance, wasn't enough to make NewPipe unusable. ie. you could simply relaunch the player to watch the next video. This has plagued me in every release since I started using NewPipe, which was version 0.23.0. You just likely haven't received many bug reports for it, as there are already existing bug reports and it doesn't prevent the app from being usable, so myself at least, I've just been watching from the sidelines hoping a future release would bring stability. From v0.24.0+ though, the app had become unusable as it's no longer able to launch the first video fullscreen.

A recurring theme with all of my debug logs trying to get 0.24+ running was the following message, right before the exoplayer2 stack trace;

W/ACodec  (22205): cannot change tunneled surface
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205): Playback error
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):   com.google.android.exoplayer2.ExoPlaybackException: Unexpected runtime error
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):       at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:616)
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):       at android.os.Handler.dispatchMessage(Handler.java:103)
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):       at android.os.Looper.loop(Looper.java:214)
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):       at android.os.HandlerThread.run(HandlerThread.java:67)
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):   Caused by: java.lang.IllegalStateException
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):       at android.media.MediaCodec.native_setSurface(Native Method)
11-12 21:51:08.508 E/ExoPlayerImplInternal(22205):       at android.media.MediaCodec.setOutputSurface(MediaCodec.java:2049)

I traced this back to the setting of a PlaceHolderSurface whilst transitioning from a windowed player to fullscreen.

If you look at the source for ACodec.cpp (from AOSP); there is this;

    // we do not support changing a tunneled surface after start
    if (mTunneled) {
        ALOGW("cannot change tunneled surface");
        return INVALID_OPERATION;
    }

So it would seem, the illegal state exception in this case was INVALID_OPERATION.

As to why some of us have been able to run a first video in the v0.23 releases, that's harder to say. Maybe there was some exception handling in the previous releases that was enough to get past the initial exception/s, but not enough to prevent the crashes in subsequent videos? I haven't tested debug builds of 0.23 to see if disabling tunneling there would result in the ability to launch subsequent videos, or to capture debug logs, but I can if you'd like?

mbradshawdev commented 2 years ago

Hi, I'm currently enjoying full screen videos on my cheapo AndroidTV device. I really hope this info can help devs and other users!

NewPipe 0.24.1 sideloaded from Aptoide TV today (20 Nov 2022) NewPipe setttings -> Video and audio -> Turn on "Start main player in full screen"

Tv remote only:

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue (top right corner above related vids), full screen button, error with large image of video, scroll down (image disappears), scroll down a bit then back up and image of video re-appears, select it and enter/ok, full screen now works (hopefully), sometimes related videos (or blank space if they're set to not display) stay visible but skip to next video and back to hide.

USB mouse:

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue, full screen button, full screen now works (hopefully), sometimes related videos stay on top but skip to next video and back to hide.

Tv Remote + Pop-up enabled

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue, select pop-up view, select main video press enter/ok, full screen now works (hopefully).

. . After the first video the app often crashed but with auto-queue and Draw over other apps permission granted via ADB I'm having a pretty good experience. Thanks so much for everyone's hard work, time and effort!

ADB command to enable pop-up view / grant draw over other apps permission via adb (I used remote ADB Shell): pm grant org.schabi.newpipe android.permission.SYSTEM_ALERT_WINDOW

Avout me: OS: Linux Android 9 - 28 Region/content: en-US/US Brand/manufacturer: Kogan/Changhong Device/product: samseong/DVB93KG Model: AI PONT Hardware: m7322

function1983 commented 2 years ago

@peat80 yes, that definitely needs to be done. Also, we should disable media tunneling by default on affected devices (like it happens currently). Anyway, media tunneling isn't really the problem as previous versions of NewPipe also used media tunneling, but though they worked without issues.

Here is another APK for you to test. I am really sorry for not having been able to solve this bug so far. The player code related to fullscreen handling is still a mess, sometimes I wonder whether everything should just be rewritten from scratch... Expect strange shenanigans with this APK (e.g. wrongly-sized controls), but let's hope that at least the player does not crash anymore when going fullscreen. app-debug.zip

Hi, I tried this on my Bravia TV (Google Tivi Sony 4K 43 inch KD-43X75K). Still get error message notification when go full screen. (Video thumbnail will be displayed in full screen, nothing will happen. Have to click to go back).

elparra72 commented 2 years ago

Release 0.24.1 does not solve issue in my Xiaomi P1 AndroidTV 10. Only open video in 3GP fullscreen and then changing to 1080p or another resolution works (or using an external player as VLC).

Obtener Outlook para Androidhttps://aka.ms/AAb9ysg

From: Grant Byers @.> Sent: Sunday, November 20, 2022 6:46:08 AM To: TeamNewPipe/NewPipe @.> Cc: elparra72 @.>; Comment @.> Subject: Re: [TeamNewPipe/NewPipe] No fullscreen possible with Android TV (Issue #9023)

Would it be feasible to do something like this (simplified example) for the currently selected audio & video codecs in org.schabi.newpipe.util.DeviceUtils?

import com.google.android.exoplayer2.mediacodec.MediaCodecInfo; import com.google.android.exoplayer2.mediacodec.MediaCodecUtil; import com.google.android.exoplayer2.util.MimeTypes;

...

public static boolean shouldSupportMediaTunneling() {
    MediaCodecInfo mediaCodecInfo = null;
    try {
        mediaCodecInfo = MediaCodecUtil.getDecoderInfo(
                MimeTypes.AUDIO_AC3, false, true);
    } catch (final MediaCodecUtil.DecoderQueryException decoderQueryException) {
        Log.d(TAG, "No tunneling support for AC3 media codec: "
                + decoderQueryException.getMessage());
    }
    return (mediaCodecInfo instanceof MediaCodecInfo && mediaCodecInfo.tunneling);
}

— Reply to this email directly, view it on GitHubhttps://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1321045598, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A34HU7OCNJBACQ57GWYHXL3WJG3KBANCNFSM6AAAAAAQVGWPY4. You are receiving this because you commented.Message ID: @.***>

ROBYER1 commented 2 years ago

Hi, I'm currently enjoying full screen videos on my cheapo AndroidTV device. I really hope this info can help devs and other users!

NewPipe 0.24.1 sideloaded from Aptoide TV today (20 Nov 2022) NewPipe setttings -> Video and audio -> Turn on "Start main player in full screen"

Tv remote only:

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue (top right corner above related vids), full screen button, error with large image of video, scroll down (image disappears), scroll down a bit then back up and image of video re-appears, select it and enter/ok, full screen now works (hopefully), sometimes related videos (or blank space if they're set to not display) stay visible but skip to next video and back to hide.

I can confirm the TV remote workaround here works on my Sony Bravia TV, KDX900F I wonder if there is a way to detect the error on not being able to play the vid in fullscreen then retry playback right after which this workaround seems to do

elparra72 commented 2 years ago

This workaround does not work in my Xiaomi P1 with Android TV 10.

Obtener Outlook para Androidhttps://aka.ms/AAb9ysg

From: Rob Farthing @.> Sent: Monday, November 28, 2022 7:18:53 PM To: TeamNewPipe/NewPipe @.> Cc: elparra72 @.>; Comment @.> Subject: Re: [TeamNewPipe/NewPipe] No fullscreen possible with Android TV (Issue #9023)

Hi, I'm currently enjoying full screen videos on my cheapo AndroidTV device. I really hope this info can help devs and other users!

NewPipe 0.24.1 sideloaded from Aptoide TV today (20 Nov 2022) NewPipe setttings -> Video and audio -> Turn on "Start main player in full screen"

Tv remote only:

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue (top right corner above related vids), full screen button, error with large image of video, scroll down (image disappears), scroll down a bit then back up and image of video re-appears, select it and enter/ok, full screen now works (hopefully), sometimes related videos (or blank space if they're set to not display) stay visible but skip to next video and back to hide.

USB mouse:

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue, full screen button, full screen now works (hopefully), sometimes related videos stay on top but skip to next video and back to hide.

Tv Remote + Pop-up enabled

Clear data, launch app, turn on setting "start main player in full screen", play any video, turn on auto-queue, select pop-up view, select main video press enter/ok, full screen now works (hopefully).

. . After the first video the app often crashed but with auto-queue and Draw over other apps permission granted via ADB I'm having a pretty good experience. Thanks so much for everyone's hard work, time and effort!

ADB command to enable pop-up view / grant draw over other apps permission via adb (I used remote ADB Shell): pm grant org.schabi.newpipe android.permission.SYSTEM_ALERT_WINDOW

Avout me: OS: Linux Android 9 - 28 Region/content: en-US/US Brand/manufacturer: Kogan/Changhong Device/product: samseong/DVB93KG Model: AI PONT Hardware: m7322

I can confirm the TV remote workaround here works on my Sony TV, I wonder if there is a way to detect the error on not being able to play the vid in fullscreen then retry playback right after which this workaround seems to do

— Reply to this email directly, view it on GitHubhttps://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1329541588, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A34HU7IHAVV2KHK5O3RZ7XTWKTZQ3ANCNFSM6AAAAAAQVGWPY4. You are receiving this because you commented.Message ID: @.***>

bazipn commented 2 years ago

Hey dev team, disabling media tunneling works but I don't think the issue is caused by it. Regardless it seems to fix the issue on my android TV and seems like a viable solution for most users. Thanks.

thekitehunter commented 1 year ago

Tried it on my FireTv 4k, didn't fix the problem. but as it only seemed to be a problem with some - the 4k ones. Limiting to 1080p and turning off the option of displaying higher resolutions allows full screen ok.

vilhelmgray commented 1 year ago

Given that we know what version works, has anyone tried a git bisect to pin down the exact commit where this bug appears? That should make it more obvious to figure out what code changes brought on this issue.

Stypox commented 1 year ago

I can give instructions about how to do that, if someone is available. But most probably the PR that caused this issue is #8170, which can't be reverted unfortunately.

hwinnemoe commented 1 year ago

I did a fresh install of 24.1 on my Fire TV Stick 4K Max, and I could use full screen viewing without issues. Slowly flipping settings to my previous configuration revealed that choosing WebM as default video format causes the issue, as well as choosing resolutions that are only available in this format. Thanks @thekitehunter !

disconnect5852 commented 1 year ago

Not works on xiaomi tv stick

hwinnemoe @.***> ezt írta (időpont: 2022. dec. 11., V 19:21):

I did a fresh install of 24.1 on my Fire TV Stick 4K Max, and I could use full screen viewing without issues. Slowly flipping settings to my previous configuration revealed that choosing WebM as default video format causes the issue, as well as choosing resolutions that are only available in this format. Thanks @thekitehunter https://github.com/thekitehunter !

— Reply to this email directly, view it on GitHub https://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1345623084, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD5TLF42Q3OGJHF4SBE6ANTWMYLRDANCNFSM6AAAAAAQVGWPY4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

thekitehunter commented 1 year ago

@hwinnemoe My settings are mp4, but so far its ok when limiting resolutions to 1080p

vikxt commented 1 year ago

Hi, I'm currently enjoying full screen videos on my cheapo AndroidTV device. I really hope this info can help devs and other users!

Yep its working fine with this trick

Just enable "Start main player in fullscreen"

And when the error occurs just scroll down a little on comments and scroll to the top and press play.

Check the attached video for steps..

https://youtu.be/ZkYTisSqkYw

Newpipe 0.24.1

MOD NOTE: Don't quote a giant wall of text. Just reply to the relevant lines.

ROBYER1 commented 1 year ago

Check the attached video for steps..

https://youtu.be/ZkYTisSqkYw

Newpipe 0.24.1

Exact same process I use to get it working on my Bravia TV too, very useful video to show it happening too.

disconnect5852 commented 1 year ago

Comment trick doesnt works, nor the fullscreen switch that setting does nothing, the video not opens in full screen if its enabled

thekitehunter commented 1 year ago

I agree, in fact turning OFF full screen makes it automatically go to full scteen!

On Wed, 21 Dec 2022, 21:43 disconnect5852, @.***> wrote:

Comment trick doesnt works, nor the fullscreen switch that setting does nothing, the video not opens in full screen if its enabled

— Reply to this email directly, view it on GitHub https://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1362126858, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQAAX65AENPVLZRSLAQKSHTWON2W7ANCNFSM6AAAAAAQVGWPY4 . You are receiving this because you were mentioned.Message ID: @.***>

fliegentoeter commented 1 year ago

On my Fire TV Stick 4k vides do not crash anymore but the player does not open in full screen no matter what options are enabled. Have to open it manually ever single video... But the older versions still work, so I went back for now.

disconnect5852 commented 1 year ago

Which versions working?

fliegentoeter @.***> ezt írta (időpont: 2022. dec. 25., V 13:19):

On my Fire TV Stick 4k vides do not crash anymore but the player does not open in full screen no matter what options are enabled. Have to open it manually ever single video... But the older versions still work, so I went back for now.

— Reply to this email directly, view it on GitHub https://github.com/TeamNewPipe/NewPipe/issues/9023#issuecomment-1364673034, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD5TLF5T6ZZJR7XAZEV3TNDWPA3WPANCNFSM6AAAAAAQVGWPY4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

fliegentoeter commented 1 year ago

v0.22.2 works for me with automatic full screen and no crashes.

thekitehunter commented 1 year ago

@fliegentoeter -player not opening in full screen. I thought that I would try to fix this by resetting the 'open in full screen' option - setting it off, using it, then setting it back on. In fact just setting 'open in full screen' to 'OFF' (not on!) resulting in it now opening in full screen every time!

IMMalebolgia commented 1 year ago

I am in a different location and did not have a Fire 4k Max at this address so I bought a new one to do the testing and went ahead and installed 0.24.1 as a baseline but oddly enough the automatic fullscreen is working on this device even though it did not work on the other Fire 4k Max. I don't know the exact OS version on the other one but this one is running: Fire OS 7.6.0.8 (PS7608/3614) 6370055.1

I will not be at the other location for several weeks but will do a full reset and fresh install of NewPipe to see if that has any effect there.

Finally at the original location and did a fresh install of NewPipe 0.24.1 but still had the issue with automatic full screen. Went and did all updates to this 4k Max and then did a factory reset before doing another fresh install of 0.24.1 aling with only restoring the channels list and manually adjusting the settings but still this one behaves differently than the other one for some reason.

I don't know if there are any HW differences between them but this on is running: Fire OS 7.6.1.4 (PS7614/3227) 6410028.1.

I will take this one with me to do some A/B testing next week to see if I can find any difference and report back.

pbasista commented 1 year ago

When this issue occurs, the following messages appear in the log:

E Player  : ExoPlayer - onPlayerError() called with:
E Player  : com.google.android.exoplayer2.ExoPlaybackException: Unexpected runtime error
E Player  :      at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:621)
E Player  :      at android.os.Handler.dispatchMessage(Handler.java:101)
E Player  :      at android.os.Looper.loop(Looper.java:164)
E Player  :      at android.os.HandlerThread.run(HandlerThread.java:65)
E Player  : Caused by: java.lang.IllegalStateException
E Player  :      at android.media.MediaCodec.native_setSurface(Native Method)
E Player  :      at android.media.MediaCodec.setOutputSurface(MediaCodec.java:1987)
E Player  :      at com.google.android.exoplayer2.mediacodec.SynchronousMediaCodecAdapter.setOutputSurface(SynchronousMediaCodecAdapter.java:189)
E Player  :      at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.setOutputSurfaceV23(MediaCodecVideoRenderer.java:1490)
E Player  :      at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.setOutput(MediaCodecVideoRenderer.java:693)
E Player  :      at com.google.android.exoplayer2.video.MediaCodecVideoRenderer.handleMessage(MediaCodecVideoRenderer.java:631)
E Player  :      at com.google.android.exoplayer2.ExoPlayerImplInternal.deliverMessage(ExoPlayerImplInternal.java:1550)
E Player  :      at com.google.android.exoplayer2.ExoPlayerImplInternal.sendMessageToTarget(ExoPlayerImplInternal.java:1514)
E Player  :      at com.google.android.exoplayer2.ExoPlayerImplInternal.sendMessageInternal(ExoPlayerImplInternal.java:1489)
E Player  :      at com.google.android.exoplayer2.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:526)
E Player  :      ... 3 more

has anyone tried a git bisect to pin down the exact commit where this bug appears?

Yes. I have done it while testing on an Android TV manufactured by Philips. The first bad commit is b3f99645a39005ddfad19f61b98b272c81414470. FYI: @Stypox who is the author.

I have narrowed the issue down to the following hunk:

--- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java
+++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java
@@ -1310,22 +1309,14 @@ public final class VideoDetailFragment
         if (!isPlayerAvailable()) {
             return;
         }
-
-        final Optional<View> root = player.UIs().get(VideoPlayerUi.class)
-                .map(VideoPlayerUi::getBinding)
-                .map(ViewBinding::getRoot);
-
-        // Check if viewHolder already contains a child TODO TODO whaat
-        /*if (playerService != null
-                && root.map(View::getParent).orElse(null) != binding.playerPlaceholder) {
-            playerService.removeViewFromParent();
-        }*/
         setHeightThumbnail();

         // Prevent from re-adding a view multiple times
-        if (root.isPresent() && root.get().getParent() == null) {
-            binding.playerPlaceholder.addView(root.get());
-        }
+        new Handler().post(() -> player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
+            playerUi.removeViewFromParent();
+            binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
+            playerUi.setupVideoSurfaceIfNeeded();
+        }));
     }

     private void removeVideoPlayerView() {

When I revert it on top of the mentioned first bad commit, the issue disappears.

I have also tried the most recent dev branch and I can confirm that the issue is still present there. However, if I understand correctly, simply updating the tryAddVideoPlayerView method to use the implementation from before commit b3f99645a39005ddfad19f61b98b272c81414470 does not make much sense.

The reason is that other changes that have been merged in the meantime probably cause the original code from this hunk to skip adding any view. In particular, the root.isPresent() condition from the original code has always been false for me. As a result, the videos would not play at all.

I currently have a very limited understanding of this code. It is possible that I am missing something trivial.

IMMalebolgia commented 1 year ago

I will take this one with me to do some A/B testing next week to see if I can find any difference and report back.

I was annoyed at the inconsistent behavior so I even bought a new Fire 4kMax to test with and didn't want to restore settings so tried recreating them from scratch to see if there was any change. I found I was able to get the fullscreen to automatically start but no matter what I did shortly after starting the second video the app would crash. So I had 3 different behaviors on the 3 different devices:

  1. No automatic fullscreen and high instability
  2. Automatic fullscreen and high instability
  3. Automatic fullscreen and no instability

I brought the 2 back with me and checked the settings between 2 and 3 and this is what was different respectively. I haven't compared the settings of 1 yet but will if anyone thinks it is helpful.

Default popup resolution - 1080p/480p Default action when opening content - Video player/Always ask Action when switching to other app from main video player - None/Minimize to background player

When I used the exported config for number 3 on number 2 the behavior followed so now I have a way of making my sticks function using a specific config but there might be clues for the other folks so let me know of any other tests that might help.

Stypox commented 1 year ago

All of the commits touching that segment of code, ordered from newest to oldest. These will help us understand the various issues we had with that piece of code, and why the way they were solved is flawed.

  1. The current code:

    private void tryAddVideoPlayerView() {
if (isPlayerAvailable() && getView() != null) {
    // Setup the surface view height, so that it fits the video correctly; this is done also
    // here, and not only in the Handler, to avoid a choppy fullscreen rotation animation.
    setHeightThumbnail();
}

// do all the null checks in the posted lambda, too, since the player, the binding and the
// view could be set or unset before the lambda gets executed on the next main thread cycle
new Handler(Looper.getMainLooper()).post(() -> {
    if (!isPlayerAvailable() || getView() == null) {
        return;
    }

    // setup the surface view height, so that it fits the video correctly
    setHeightThumbnail();

    player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
        // sometimes binding would be null here, even though getView() != null above u.u
        if (binding != null) {
            // prevent from re-adding a view multiple times
            playerUi.removeViewFromParent();
            binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
            playerUi.setupVideoSurfaceIfNeeded();
        }
    });
});
}
  2. https://github.com/TeamNewPipe/NewPipe/commit/500acce178d1e9f21d811f7e9f3b3bfac6c4aeb0 from #8731 - in response to https://github.com/TeamNewPipe/NewPipe/pull/8170#issuecomment-1191217817
  3. https://github.com/TeamNewPipe/NewPipe/commit/6805c75c9cf90f9448e266eda34866136f0c71c8 from #8731 - in response to https://github.com/TeamNewPipe/NewPipe/pull/8678#issuecomment-1200483145
  4. https://github.com/TeamNewPipe/NewPipe/commit/4eddd2c3d1e246d92e8da81e335c3dc10fc1e68c from #8661 - in response to https://github.com/TeamNewPipe/NewPipe/pull/8170#issuecomment-1184735382 https://github.com/TeamNewPipe/NewPipe/pull/8170#issuecomment-1185762001 https://github.com/TeamNewPipe/NewPipe/issues/8646#issuecomment-1186609499
  5. https://github.com/TeamNewPipe/NewPipe/commit/0e8cc72b134bf5755f59cbbfd6e01180509e7c57 from #8170 - in response to https://github.com/TeamNewPipe/NewPipe/pull/8170#issuecomment-1184735382
  6. https://github.com/TeamNewPipe/NewPipe/commit/61c1da144e4b0a824d4816d9ecad6a59b53d9cf3 from #8170 - in response to https://github.com/TeamNewPipe/NewPipe/pull/8170#discussion_r865138017

  7. https://github.com/TeamNewPipe/NewPipe/commit/b3f99645a39005ddfad19f61b98b272c81414470 from #8170 - first bad commit, here is the code after that commit:

    private void addVideoPlayerView() {
if (!isPlayerAvailable()) {
    return;
}
setHeightThumbnail();

// Prevent from re-adding a view multiple times
new Handler().post(() -> player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> {
    playerUi.removeViewFromParent();
    binding.playerPlaceholder.addView(playerUi.getBinding().getRoot());
    playerUi.setupVideoSurfaceIfNeeded();
}));
}
  8. https://github.com/TeamNewPipe/NewPipe/commit/76ced59b62322537eb818cd3fea7ddd1b9c2d711 from #8170 - the initial big player splitting commit

  9. The code before https://github.com/TeamNewPipe/NewPipe/commit/76ced59b62322537eb818cd3fea7ddd1b9c2d711:

    private void addVideoPlayerView() {
if (!isPlayerAvailable() || getView() == null) {
    return;
}

// Check if viewHolder already contains a child
if (player.getRootView().getParent() != binding.playerPlaceholder) {
    playerService.removeViewFromParent();
}
setHeightThumbnail();

// Prevent from re-adding a view multiple times
if (player.getRootView().getParent() == null) {
    binding.playerPlaceholder.addView(player.getRootView());
}
}

I don't remember what led me to post actions to a Handler so that they are done on the next main loop cycle, instead of performing those actions directly. Thank you so much for doing a git bisect @pbasista, this helps a lot! Now I can just try to mess around with that code, possibly eliminating the Handler.post to solve this issue once and for good. As soon as I will have time I will do so. :-D

Stypox commented 1 year ago

Here are three testing (release) APKs for you to test, along with the code diffs for reference. They range from least changes to most changes. I tested all of these both on my phone and on an emulator, so at least they shouldn't introduce more issues. I now understand why the Handler was needed: when that function gets called when opening a video, the player is not yet available from the VideoDetailFragment, and so the method wouldn't be able to do anything. Another strange thing I noticed is that when going into/out of fullscreen, that method is called two times, which might cause issues. Let me know which ones (if any) of these APKs work. Thanks for the ongoing effort from everyone :-)

First APK: fixtvfullscreen1.zip ```diff diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 601135cbb..4e6e17f41 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -1257,7 +1257,7 @@ public final class VideoDetailFragment player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> { // sometimes binding would be null here, even though getView() != null above u.u - if (binding != null) { + if (binding != null && playerUi.getViewParent() != binding.playerPlaceholder) { // prevent from re-adding a view multiple times playerUi.removeViewFromParent(); binding.playerPlaceholder.addView(playerUi.getBinding().getRoot()); diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java index 683629c25..c24d592f6 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java @@ -983,5 +983,9 @@ public final class MainPlayerUi extends VideoPlayerUi implements View.OnLayoutCh // while DisplayMetrics from app context doesn't return DeviceUtils.isLandscape(getParentContext().orElse(player.getService())); } + + public ViewParent getViewParent() { + return binding.getRoot().getParent(); + } //endregion } ```
Second APK: fixtvfullscreen2.zip ```diff diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 601135cbb..b85c4fd66 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -1245,6 +1245,10 @@ public final class VideoDetailFragment setHeightThumbnail(); } + if (isPlayerAvailable()) { + player.UIs().get(MainPlayerUi.class).ifPresent(MainPlayerUi::removeViewFromParent); + } + // do all the null checks in the posted lambda, too, since the player, the binding and the // view could be set or unset before the lambda gets executed on the next main thread cycle new Handler(Looper.getMainLooper()).post(() -> { @@ -1259,7 +1263,9 @@ public final class VideoDetailFragment // sometimes binding would be null here, even though getView() != null above u.u if (binding != null) { // prevent from re-adding a view multiple times - playerUi.removeViewFromParent(); + if (playerUi.getViewParent() != null) { + playerUi.removeViewFromParent(); + } binding.playerPlaceholder.addView(playerUi.getBinding().getRoot()); playerUi.setupVideoSurfaceIfNeeded(); } diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java index 683629c25..c24d592f6 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java @@ -983,5 +983,9 @@ public final class MainPlayerUi extends VideoPlayerUi implements View.OnLayoutCh // while DisplayMetrics from app context doesn't return DeviceUtils.isLandscape(getParentContext().orElse(player.getService())); } + + public ViewParent getViewParent() { + return binding.getRoot().getParent(); + } //endregion } ```
Third APK: fixtvfullscreen3.zip (as similar to the code before 76ced59 as possible) ```diff diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index 601135cbb..3c7106c4f 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -1239,34 +1239,36 @@ public final class VideoDetailFragment } private void tryAddVideoPlayerView() { - if (isPlayerAvailable() && getView() != null) { - // Setup the surface view height, so that it fits the video correctly; this is done also - // here, and not only in the Handler, to avoid a choppy fullscreen rotation animation. - setHeightThumbnail(); + if (!isPlayerAvailable() || getView() == null) { + postAddVideoPlayerView(); + return; } - // do all the null checks in the posted lambda, too, since the player, the binding and the - // view could be set or unset before the lambda gets executed on the next main thread cycle + player.UIs().get(MainPlayerUi.class) + .ifPresentOrElse(this::addVideoPlayerViewUnchecked, this::postAddVideoPlayerView); + } + + private void postAddVideoPlayerView() { new Handler(Looper.getMainLooper()).post(() -> { if (!isPlayerAvailable() || getView() == null) { return; } - // setup the surface view height, so that it fits the video correctly - setHeightThumbnail(); - - player.UIs().get(MainPlayerUi.class).ifPresent(playerUi -> { - // sometimes binding would be null here, even though getView() != null above u.u - if (binding != null) { - // prevent from re-adding a view multiple times - playerUi.removeViewFromParent(); - binding.playerPlaceholder.addView(playerUi.getBinding().getRoot()); - playerUi.setupVideoSurfaceIfNeeded(); - } - }); + player.UIs().get(MainPlayerUi.class).ifPresent(this::addVideoPlayerViewUnchecked); }); } + private void addVideoPlayerViewUnchecked(final MainPlayerUi playerUi) { + if (playerUi.getViewParent() != binding.playerPlaceholder) { + playerUi.removeViewFromParent(); + } + setHeightThumbnail(); + if (playerUi.getViewParent() == null) { + binding.playerPlaceholder.addView(playerUi.getBinding().getRoot()); + playerUi.setupVideoSurfaceIfNeeded(); + } + } + private void removeVideoPlayerView() { makeDefaultHeightForVideoPlaceholder(); diff --git a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java index 683629c25..c24d592f6 100644 --- a/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/ui/MainPlayerUi.java @@ -983,5 +983,9 @@ public final class MainPlayerUi extends VideoPlayerUi implements View.OnLayoutCh // while DisplayMetrics from app context doesn't return DeviceUtils.isLandscape(getParentContext().orElse(player.getService())); } + + public ViewParent getViewParent() { + return binding.getRoot().getParent(); + } //endregion } ```
peat80 commented 1 year ago

For me all 3 APKs behave exactly as before.

On my shield tv first video plays windowed & starting a second video opens in fullscreen.

On my bravia tv first video plays windowed & starting a second video crashes the app.

All 3 tested with imported settings. 🤔