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.

Sign up for GitHub

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

Jump to bottom

Close enable screensaver while playing audio files, #4954 #4964

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Close enable screensaver while playing audio files, #4954 #4964

wants to merge 2 commits into from

Conversation

low-batt
Copy link
Contributor

This commit will:

  • Add new settings preventDisplaySleepForAudio and preventDisplaySleepForVideo

  • Change the PlayerCore checkCurrentMediaIsAudio method into an isAudio PlaybackInfo property

  • Add a Require display to stay on while actively playing video checkbox to the settings window General tab

  • Add a similar checkbox for playing audio

  • Change the PlayerCore.checkStatusForSleep method to support the new settings

  • Add logging to SleepPreventer

  • I have read CONTRIBUTING.md

  • This implements/fixes issue Enable Screensaver While Playing Audio Files in IINA Media Player  #4954.

Description:

@low-batt
Close enable screensaver while playing audio files, #4954
1d014e7 
This commit will:
- Add new settings preventDisplaySleepForAudio and
  preventDisplaySleepForVideo
- Change the PlayerCore checkCurrentMediaIsAudio method into an isAudio
  PlaybackInfo property
- Add a "Require display to stay on while actively playing video"
  checkbox to the settings window General tab
- Add a similar checkbox for playing audio
- Change the PlayerCore.checkStatusForSleep method to support the new
  settings
- Add logging to SleepPreventer
@low-batt low-batt requested a review from uiryuu May 30, 2024 03:03
@low-batt low-batt linked an issue May 30, 2024 that may be closed by this pull request
Enable Screensaver While Playing Audio Files in IINA Media Player #4954
Open
@uiryuu
Copy link
Member

uiryuu commented May 30, 2024

I don't think there's a need for allowing to go in screensaver for audio and not for video. Then the options could be simplified into:

  • Prevent screensaver
    • Not apply when playing audio file or in music mode

This way we can also handle music mode correctly, since maybe someone will play a video file but actually only want the audio, so they enter music mode. Of cause you can even differentiate audio file and music mode and split them into 2 options.

@low-batt low-batt marked this pull request as draft May 30, 2024 03:52
@low-batt
Copy link
Contributor Author

I will make that change. I struggled with what to name this setting. I based it on how Apple describes the setting we are using, kIOPMAssertionTypeNoDisplaySleep:

The idle display will not sleep when enabled, and consequently the system will not idle sleep.

I do think the wording needs to make it clear that this only kicks in when playback is active.

@low-batt
Copy link
Contributor Author

I think I have to make more changes to properly implement this. Is the functionality we normally want for audio/music mode "allow screen saver, but prevent system sleep"? If so, maybe instead of kIOPMAssertionTypeNoDisplaySleep we should switch to using beginActivity and the flags idleDisplaySleepDisabled and idleSystemSleepDisabled. When not suppressing the screen saver for audio we would still not allow the system to sleep. The beginActivity method does not report errors, so we would be loosing the current error reporting we do. Not sure what that method would do if macOS was broken as was encountered in the past.

Another question is whether we should be calling beginActivity with userInitiated when playback is active.

Thoughts?

@uiryuu
Copy link
Member

uiryuu commented Jun 1, 2024

I think using ProcessInfo methods to control the system behavior is clearly the better way than what we are currently doing. We can definitely try using that in the upcoming beta versions.

Another question is whether we should be calling beginActivity with userInitiated when playback is active.

I don't have a strong opinion on this, but maybe I slightly prefer .background. Need to hear other devs' opinions.

@low-batt
Copy link
Contributor Author

low-batt commented Jun 1, 2024

I will prepare an update to this that includes switching to ProcessInfo. We definitely do not want to use .background. Playing audio is a real time operation. Failing to post a buffer in time can result stuff like pops and clicks.

Definitely run this past the other developers, especially @saagarjha.

@low-batt
Close enable screensaver while playing audio files, #4954
994d32b 
This commit will:
- Add new settings preventScreenSaver and
  allowScreenSaverForAudio
- Change the PlayerCore checkCurrentMediaIsAudio method into an isAudio
  PlaybackInfo property
- Add a "Prevent screen saver from starting while playing"
  checkbox with a "Not while in Music Mode or only playing audio"
  subordinate checkbox to the settings window General tab
- Change the PlayerCore.checkStatusForSleep method to support the new
  settings
- Change SleepPreventer to use ProcessInfo activities instead of
  IOPMAssertionCreateWithName
- Add logging to SleepPreventer
- Remove the "Cannot prevent display sleep!" alert along with the
  associated suppressCannotPreventDisplaySleep preference.

This allows the user to control whether IINA prevents the screen saver
from starting when in music mode or just playing audio or allow the
screen saver to start and the display to power off and only prevent the
system from sleeping.

By default IINA will prevent the screen saver from starting when in
music mode or playing audio to match the current behavior.
@low-batt low-batt marked this pull request as ready for review June 5, 2024 01:46
@low-batt
Copy link
Contributor Author

low-batt commented Jun 5, 2024

This is what the new settings look like:
prevent-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uiryuu uiryuu Awaiting requested review from uiryuu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable Screensaver While Playing Audio Files in IINA Media Player
2 participants
@low-batt @uiryuu