openairplay / airplay2-receiver

AirPlay 2 Receiver - Python implementation
2.13k stars 133 forks source link

Add Windows Support for ap2.utils.get_volume() and set_volume() #32

Closed TheSpookyCat closed 3 years ago

TheSpookyCat commented 3 years ago

I'm opening this PR with a dummy commit because I have two solutions however both have their own dependency.

I understand not wanting to bloat the repo with requirements, which is why I am wondering if it would be preferrable to require a utility added to PATH or dropped in by a user. I'd love to hear someone else's opinion as oppose to just going ahead and adding a dependency as part of my PR.

Edit: I was unable to find any simple way of controlling master volume on windows via the command line. Let me know if you know of one.

systemcrash commented 3 years ago

Just add a Py lib - it can be conditionally imported, depending on the platform. ( I think there's code already in the code base which does something like conditional imports, look at that for inspiration )

Then others can drive-by test the PR 😄

TheSpookyCat commented 3 years ago

Needs testing by others now but has worked for me without issue. There are some caveats with this way of doing things

systemcrash commented 3 years ago

Oh, sweet!

Do you think there is a programmatic way of identifying which process ap2 runs under, to 'be sure'?

Does this mean that Windows can't use pyaudio (which uses portaudio)? Just curious.

I think that if py/portaudio and windows are separate, then this PR would need a bit of code to distinguish those out there who use windows+portaudio, and, in your case, just use windows (with another sound subsystem).

I don't run windows, but I think some other windows users might welcome this.

Edit: Did a code cleanup, so a simple rebase will do

glmnet commented 3 years ago

Pff, I've written almost the same thing. Yes works great!

regarding using the process id, it's a bit "tricky" because the process id is the one of the audio process, not the one running the RTSP, so either we need to get the process number on the volume call or pass the volume via the pipe to the audio process

glmnet commented 3 years ago

I tested this and is ok. The pid binding can be done later

glmnet commented 3 years ago

Here is a proposal for setting volume by pid https://github.com/LewdNeko/airplay2-receiver/pull/1

TheSpookyCat commented 3 years ago

Honestly I'm not a fan of using globals but at the moment I can't think of a more stateful way of setting PID. Shouldn't cause any issues anyway, I'd say this is now pretty damn reliable in terms of controlling the correct application. Another caveat of checking via name is when I run it as a Windows Service using NSSM it doesn't run as python.exe. I'd be happy to see this merged into master.