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

Prevent device from dimming or sleeping screen while video/audio playing #4160

Merged
merged 2 commits into from
Dec 12, 2023

Conversation

mcclure
Copy link
Collaborator

@mcclure mcclure commented Dec 10, 2023

We got reports on Mastodon that the media3 upgrade broke this previously functional behavior. In my testing, this restores the v23 behavior.

Note, @Gryzor suggested we do this with a Wake Lock. After looking at the docs, I conclude that (1) a wake lock is not what v23 was using, and is not appropriate for video but (2) a wake lock is appropriate for audio and possibly other purposes, and we should consider adding one in the v25 timeframe. I will file a followup issue explaining what I mean by this later today.

…hile video/audio are playing. Patch restores v23 behavior
@connyduck
Copy link
Collaborator

Shouldn't the screen stay on only when a video is playing?

@mcclure
Copy link
Collaborator Author

mcclure commented Dec 10, 2023

Shouldn't the screen stay on only when a video is playing?

Yes, you are correct. However, it turns out keeping the screen on is simple, and sleeping the screen but continuing to play audio is complicated. That is why I propose we merge this for 24.1 (restoring the 23.0 behavior) and then merge a better fix with special handling for audio in 25.0. I have a full explanation in the issue here (it is complicated).

@mcclure
Copy link
Collaborator Author

mcclure commented Dec 11, 2023

Update see #4162, my unsuccessful WIP for setWakeMode.

If we cannot get setWakeMode working on the 24.1 timeframe, I do believe it is better behavior to allow the audio to continue playing while keeping the screen on, than to allow the audio to cut off after 1 minute when the screen naturally sleeps.

@Gryzor
Copy link
Collaborator

Gryzor commented Dec 11, 2023

@mcclure Agree. I think the Wake Look is no longer "used" for the behavior you want here, and instead the keep screen on is "the way". This shows you how long it has been since I had to deal with one! ;)

@Tak
Copy link
Collaborator

Tak commented Dec 11, 2023

I agree that the pre-24 behavior was "good enough", and imo we shouldn't invest too much work in letting the screen blank while continuing to play audio (unless it's a personal interest, of course) - Tusky isn't a music player or podcast app or whatever

@connyduck
Copy link
Collaborator

Ok so it is ok for you all that the screen will stay on when imges are shown? Then I will merge this. (I personally think it is weird and could bring us more complaining users).

@mcclure
Copy link
Collaborator Author

mcclure commented Dec 12, 2023

Ok so it is ok for you all that the screen will stay on when imges are shown? Then I will merge this. (I personally think it is weird and could bring us more complaining users).

The screen does not stay on when images are shown, see here (and I did test this). We only request KEEP_SCREEN_ON for audio, video, gifv.

No one complained about always-on screen with audio in v23— I am one of the few people who posts audio on Mastodon and I never noticed Tusky was keeping the screen on during audio. So I think yes screen staying on during audio is ok for now.

@connyduck
Copy link
Collaborator

But what if you swipe to an image? Shouldn't the flag be removed then? Or does it have no effect once it is set?

@mcclure
Copy link
Collaborator Author

mcclure commented Dec 12, 2023

But what if you swipe to an image? Shouldn't the flag be removed then? Or does it have no effect once it is set?

That's… a good question.
One moment, I will test.

@mcclure
Copy link
Collaborator Author

mcclure commented Dec 12, 2023

Did @connyduck's test and can confirm, if you find a post with both an image and a video and swipe between them, it does the wrong thing.

If you start on an image and swipe to a video it will sleep (incorrectly), if you start on a video and swipe to an image it will not sleep (incorrectly).

I think this is worth holding off this patch for, I need to go to bed now but will attempt to fix the swipe bug tomorrow.

@mcclure mcclure merged commit b9a593d into tuskyapp:exoplayer-awake-naive Dec 12, 2023
3 checks passed
@mcclure
Copy link
Collaborator Author

mcclure commented Dec 12, 2023

Github what the hell this was not merged

@mcclure
Copy link
Collaborator Author

mcclure commented Dec 12, 2023

Sigh new PR #4168

connyduck added a commit that referenced this pull request Dec 13, 2023
…ing (but really) (#4168)

For reasons not totally clear to me, Github marked #4160 as "merged" and
will not let me reopen it.

I believe this should be included for 24.1 because it fixes a 24.0
regression in the media player.

I have tested this newest commit in a number of ways, and in my testing
I find (1) when viewing an image, it sleeps after about a minute (2)
when viewing video, it stays awake indefinitely (3) this is true whether
the image/video was opened directly, or reached by swiping from another
attachment. I have not tested swiping to/from audio but I am confident
it will work the same.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants