ralph-irving / squeezelite

Lightweight headless squeezebox player for Lyrion Media Server
https://sourceforge.net/projects/lmsclients/files/squeezelite/
Other
390 stars 98 forks source link

ALSA Hardware Volume Scaling/Remapping #52

Closed kefabean closed 5 years ago

kefabean commented 6 years ago

Hi Ralph,

I'm looking for a way to provide a command line parameter that would allow the ALSA volume controls to be set to a percentage of what is shown in Squeezeserver/iPeng. The reason is that I have Squeezelite plugged in to a USB-DAC (Audiolab M-DAC) controlling the DAC hardware volume control via USB. The following is my runtime (with additional logic to pin squeezelite on to a dedicated core!!!):

/usr/bin/squeezelite -n remote -o hw:CARD=MDAC -s localhost -C 5 -r 96000 -V PCM -d all=info

This works great but I would like to be able to lock (attenuate) the maximum output to say 30% of the 65536 full gain output so as not to accidentally blow speakers by turning volume up too high, or accidently setting 'Output level is Fixed at 100% in Squeezeserver. It would also allow the full volume range show in iPeng/Squeezeserver to control the restricted hardward volume range. Ideally I'd be able to achieve this without resorting to ALSA plugins (eg. softvol).

I'm keen to seek your advice in terms of whether this requires relatively simple changes to main.c and output_alsa.c and the associated headers. I'm new to C but happy to dig in to look at what code changes would be required. The crux of the change as I see it would involve the following change to output_alsa.c (with other minor changes to read the command-line parameters etc:

void set_volume(unsigned left, unsigned right) {
    float ldB, rdB;

    if (!alsa.volume_mixer_name) {
        LOG_DEBUG("setting internal gain left: %u right: %u", left, right);
        LOCK;
        output.gainL = left * alsa.volume_scaling / 100;
        output.gainR = right * alsa.volume_scaling / 100;
        UNLOCK;
        return;
    } else {
        LOCK;
        output.gainL = FIXED_ONE * alsa.volume_scaling / 100;
        output.gainR = FIXED_ONE * alsa.volume_scaling / 100;
        UNLOCK;
    }

    // convert 16.16 fixed point to dB
    ldB = 20 * log10( left * alsa.volume_scaling / 6553600.0F );
    rdB = 20 * log10( right * alsa.volume_scaling / 6553600.0F );

    set_mixer(false, ldB, rdB);
}

However, I'm concerned about some of the cross-fade/replay gain logic interfering. Can you see any issue with my approach? Is there a better way to achieve what I'm trying to?

Appreciate your thoughts! Thanks, Dan.

ralph-irving commented 6 years ago

Have you tried the LMS Volume Lock plugin? https://forums.slimdevices.com/showthread.php?106217-Peter-Watkins-Plugins&p=863899&viewfull=1#post863899

kefabean commented 6 years ago

Hi Ralph - many thanks for getting back to me. This is certainly better than my current solution which has a TCP client on port 9090 subscribed to volume notifications, essentially reseting the volume if it goes over the max. (My volume client is performing other functions too - namely providing a rotary encoder on a raspberry pi to provide a tactile physical volume control over the network from another part of the house). The issue with my approach is that the speakers go pop if the client gets disconnected.

I've tested the plugin and it does work, however, much like my client, it momemtarily allows the volume to be set much higher before the plugin resets the volume back down again - this could cause problems for speakers! Also it only allows a fraction of the volume scale to be used on the SqueezeServer web console, or via iPeng, which doesn't provide a great user experience.

Any pointers on modifying squeezelite, or should I put this in the too-hard pile? Your thoughts would be greatly appreciated. Am happy to learn a little C, but maybe not if it requires a thorough rework! ;-)

Cheers, Dan.

kefabean commented 6 years ago

(btw - fixed an error in my code snippet above!)

ralph-irving commented 6 years ago

The fade routines in output.c use the values of output.gainL and output.gainR to calculate the fade levels. I'd recommend you disconnect your speakers during testing and use alsamixer to watch the hardware volume changes. You can see the alsamixer PCM slider move when you change the volume in the LMS webgui when using -V PCM with squeezelite, so you can confirm it moves before you start testing your changes.

kefabean commented 6 years ago

Thanks your feedback was immensely helpful. I was able to track down the necessary changes - looks like i needed to use set_mixer instead of set_volume which makes total sense. I've created this pull request with my changes. However, I'm concerned there's a regesssion where things stop working if the ALSA device is removed then readded (eg. DAC is power cycled). Needs a bit of further investigation #53

ralph-irving commented 6 years ago

Does reverting this commit https://github.com/ralph-irving/squeezelite/commit/54f9206955ccc71b2a5faf4fecfdba44c8dcb045 fix it?

kefabean commented 6 years ago

I successfully reverted the commit but I don't think this is the culprit. Previously I wasn't controlling the ALSA mixer with squeezelite (or I didn't power cycle the DAC). Playback actually correctly resumes after power cycling the ALSA device as expected.

The issue seems to be that the ALSA mixer device going stale after it (the DAC) is power cycled (streaming continues successfully but I am no longer able to adjust the volume).

I'm guessing that the mixer needs to be somehow probed and reinitialised in the same way that the pcm output is reinitialised.

Not sure how to achieve this and I'm probably at the limit of my knowledge here (although keen to learn more!). Again, any pointers would be greatly appreciated, although understand this might not be at the top of your priority list.

Cheers, Dan.

ralph-irving commented 6 years ago

Since reverting 54f9208 does not fix the problem I won't apply your pull request 53. I posted a comment with a modified patch for PR53 which fixes building for non ALSA targets. Can you delete pull request 53 and create a new PR based my proposed change provided it still works as you intended and without reverting the commit above?

ralph-irving commented 6 years ago

In output_alsa.c there's a loop at line 673 to handle a usb dac off and on. Once the device returns you can check if volume_mixer_name is defined and if so, close the mixer handle if not null and call mixer_init_alsa similiar to the code starting at line 995. It might work.

kefabean commented 6 years ago

Yep absolutely, will delete and create a new PR.

ralph-irving commented 6 years ago

Thanks. Including the patch here for reference. 53alt.patch.txt

kefabean commented 6 years ago

Patch included in my latest PR. I think I have fixed the issue above to ensure the mixer is reinitialised after a USB DAC power cycle. Please take a look and see whether this makes sense.

Unfortunately although it probably now works as expected, it doesn't fully resolve my usecase. as I need to be able to control the volume after a DAC power cycle even when Squeezelite is not playing (eg. so I can control other non-Squeezelite digital inputs).

Currently the mixer is only reinitialsed once Squeezelite playback resumes due to the wait loop (in lines 723-733 of output_alsa.c).

I can achieve what I need by commenting out these and also lines 874-894, but this will break the power trigger scripts and GPIO control so haven't made any further modifications until I can figure out a better way.

kefabean commented 6 years ago

Hmmm, there appears to be a slight issue where the playback stops or is silent when a new track starts until you adjust the volume (either through alsamixer directly or via squeezeserver), at which point it immediately resumes. Not sure what could be causing this, but suggest you hold off on the PR while I investigate (unless there's anything obvious that springs to mind).

kefabean commented 6 years ago

Just to let you know I'm still testing this. Currently it all seems okay, I think I may have had some spotify or bandwidth issues previously. I'll continue to test for another week or so to see whether it throws anything up.

Dan.