Opened 20 years ago

Closed 20 years ago

Last modified 20 years ago

#69 closed enhancement (fixed)

PATCH: Add support to select an ALSA mixer channel index.

Reported by: eyager@… Owned by: reimar
Priority: normal Component: ao
Version: HEAD Severity: minor
Keywords: Cc:
Blocked By: Blocking:
Reproduced by developer: no Analyzed by developer: no

Description

ao_alsa.c currently does not allow a user to specify the mixer index. The
standard way of addressing an alsa mixer device is <channel_name,mixer_index>.
Currently with the -mixer-channel option only the channel_name is implemented
and the mixer_index is completely unsupported and is hard set at zero. In the
case of soundcards with multiple PCM devices many controls have the same name,
and mixer index value determines which control is getting addressed.

The attached patch allows users to set the ALSA mixer index via the
-mixer-channel option.

Examples:

-mixer-channel PCM --- Selected mixer is PCM index 0 (alsamixer "PCM")

Works the same as without the patch

-mixer-channel PCM,1 --- Selected mixer is PCM index 1 (alsamixer "PCM 1")

FAILS without the patch

Error handling:

-mixer-channel FOO, --- Missing operand, FOO index 0 selected.
-mixer-channel FOO,a --- Non-numeric operand, FOO index 0 selected.
-mixer-channel F00,1a -- Letters after index discarded. FOO index 1 selected.

If the mixer channel and/or index does not exist. Mplayer will switch to soft
volume control as usual.

Attachments (5)

patch (3.1 KB ) - added by eyager@… 20 years ago.
Patch to libao2/ao_alsa.c and manpage for mixer index support.
patch.2 (1.9 KB ) - added by eyager@… 20 years ago.
Revised patch to libao2/ao_alsa.c and manpage for mixer index support.
patch.3 (3.2 KB ) - added by eyager@… 20 years ago.
Revised patch libao2/ao_alsa.c
final (2.6 KB ) - added by eyager@… 20 years ago.
Revised patch libao2/ao_alsa.c
alsa_mix_index.diff (2.5 KB ) - added by reimar 20 years ago.
some last minor corrections

Download all attachments as: .zip

Change History (16)

by eyager@…, 20 years ago

Attachment: patch added

Patch to libao2/ao_alsa.c and manpage for mixer index support.

comment:1 by eyager@…, 20 years ago

comment:2 by reimar, 20 years ago

I have a few nitpicks about that patch:
1) strpbrk is overkill, use strchr instead

2) instead if atoi use strtol, it allows for testing if parsing was successful:
mix_index = strtol(test_mix_index, &test_mix_index, 0);
if (test_mix_index[0]) then error...

especially as your assumption that numbers can only contain digits is disputable
(ok, only in theory but still) ;-)

3) for consistency place a space before and after '=' in assignments

4) Never ever use printf in MPlayer code! mp_msg is the right function for
printing messages, in this case:
mp_msg(MSGT_AO, MSGL_ERR, ...printf-style arguments...);

5) Although MPlayer could use better documentation, I think yours is a bit too
much. Don't document the trivial! (actually I don't think that the code you
added needs any comments)

6) I personally prefer constructs like str[0]=... over *str=..., e.g.
test_mixer_index[0] = 0;
test_mixer_index++;

instead of

*(test_mix_index++)=0;

but I leave that for you to decide...

by eyager@…, 20 years ago

Attachment: patch.2 added

Revised patch to libao2/ao_alsa.c and manpage for mixer index support.

comment:3 by eyager@…, 20 years ago

attachments.isobsolete: 01

Revised the patch as suggested.

comment:4 by reimar, 20 years ago

+ if (test_mix_index){
+ mp_msg(MSGT_AO,MSGL_ERR,"alsa-control: non numeric or null mixer index.
Defaulting to 0\n");
+ }

I didn't test, but I think this must be *test_mix_index (it's just testing if the
character after the parsed part is the end of the string)

I think it should work, but did you also test with the Gui?

by eyager@…, 20 years ago

Attachment: patch.3 added

Revised patch libao2/ao_alsa.c

comment:5 by eyager@…, 20 years ago

attachments.isobsolete: 01

The code for setting a specific alsa mixer channel within the gui doesn't seem
to be working at the moment. A bugreport is going to have to be filed.
Looking at the source it appears the gui needs to have the contents of
mixer_channel left untouched, so I had to copy mixer_channel into a temporary
string for parsing rather than modifying it directly.

comment:6 by reimar, 20 years ago

Owner: changed from alex@… to Reimar.Doeffinger@…

I hope you don't get the impression that I intented to bug you forever ;-)
Still I'd like to point out a few things.
1) I think you should keep tho NOTE for the manpage part you had in your first
version, although I'd leav out the empty lines, as I have no idea what groff
would do to them...
2) Instead of malloc + strcopy, I'd use the strdup function, it does both.
3) the other alsa drivers (alsa9, alsa1x) aren't updated anymore and will be
deleted soon, so don't care to update them, I'll give you enough to do with this
one ;-).
4) remove the spaces and tabs from empty lines, same for the trailing whitespace
after static int mix_index;
5) I personnally don't like two consecutive empty lines (after static int
mix_index;) and an empty line inbetween two closing braces.

by eyager@…, 20 years ago

Attachment: final added

Revised patch libao2/ao_alsa.c

comment:7 by eyager@…, 20 years ago

attachments.isobsolete: 01

I finally got enough free time to resubmit the patch. Vim's autoindent feature
can be bad sometimes. :-) I've verified that it also works with Gmplayer.

comment:8 by reimar, 20 years ago

(In reply to comment #7)

I finally got enough free time to resubmit the patch. Vim's autoindent feature
can be bad sometimes. :-) I've verified that it also works with Gmplayer.

I occasionally experience the drawbacks of the autoindent feature too ;-)
Thanks for your time, I'll try to get it into CVS, might succeed somewhen around
next week.

by reimar, 20 years ago

Attachment: alsa_mix_index.diff added

some last minor corrections

comment:9 by reimar, 20 years ago

added missing initialization of mix_index
removed unneccessary parsed_mixer_channel variable and strcpy call.
setting to NULL after freeing, to make any attempt to access it afterwards
obvious.

comment:10 by reimar, 20 years ago

attachments.isobsolete: 01

comment:11 by reimar, 20 years ago

Resolution: fixed
Status: newclosed

I applied it. Thanks for your time.

Note: See TracTickets for help on using tickets.