falconindy / ponymix

CLI volume control for PulseAudio
MIT License
179 stars 27 forks source link

Toggling/muting/unmuting target with --device flag requires --source flag (for sources), but not --sink (for sinks) #31

Open SuperFluffy opened 9 years ago

SuperFluffy commented 9 years ago

When directly targetting a device with the --device flag to toggle its state (or muting/unmuting it), the --source flag is required for sources, but the --sink flag is not equivalently required for sinks. Demonstration:

# For the output / sink device
% ponymix -d alsa_output.pci-0000_00_1b.0.analog-stereo toggle
85
# For the input / source device
% ponymix -d alsa_input.pci-0000_00_1b.0.analog-stereo toggle 
ponymix: no match found for device: alsa_input.pci-0000_00_1b.0.analog-stereo

% ponymix --source -d alsa_input.pci-0000_00_1b.0.analog-stereo toggle
36
falconindy commented 9 years ago

What are you suggesting as a better behavior? This Just Works™ for sinks because I've decided that devtype should default to "sink" -- this is generally what you want because it controls output volume (what I consider to be the most common usecase). Removing the default means command lines get more verbose.

SuperFluffy commented 9 years ago

Yeah, I was just looking at the source, and noticed that opt_devtype = DEVTYPE_SINK; in main(). I guess the structure of GetDevice in pulse.cc is dictated by the PulseAudio API (through get_device)?

Otherwise I'd suggest that if the -d DEVICE flag is set, and neither --source nor --sink are specified, that the device type should be infered via its name? I am not really sure if pulseaudio allows this, but since the devices are classified as either sinks or sources, and since the devices are uniquely identified by their names, this should be possible?

EDIT: Well, I suppose this conflicts with the current behaviour that you can set the monitor of the sink just by specifying the source name:

% ponymix -d alsa_output.pci-0000_00_1b.0.analog-stereo --source toggle
100

The above command actually toggles the state of alsa_output.pci-0000_00_1b.0.analog-stereo.monitor. I'd prefer the command fails, because if I wanted to toggle the monitor, I'd actually specifiy its full name.

falconindy commented 9 years ago

Makes sense. I don't know of any way to infer device type directly from the name, and the pulse API doesn't offer this idea of a "blind" lookup either. However, it does seem like we could just loop over the internal vectors of devices in the PulseClient until we find a match. Interested in writing a patch?

SuperFluffy commented 9 years ago

Yeah, I will look at it. This will help me brush up my C++.

Though this will take me a bit of time, if that's alright with you. :)

falconindy commented 9 years ago

Fine by me! Thanks!

SuperFluffy commented 9 years ago

This turned out to be pretty straight forward. --source and --sink are still useful, although I am not completely happy with them being sillently ignored when supplying -d DEVICE.