[MPlayer-dev-eng] [PATCH] add OpenAL backend device support Vs.2

[Reimar Döffinger]

On Tue, Sep 14, 2010 at 10:45:25AM +0200, Erik Hofman wrote:
> +          "    help\n"
> +           "        This help including list of available devices.\n"

Don't add an extra help option, it's done nowhere else, just print
the help for every parse error.

> +  if (alcIsExtensionPresent(NULL, "ALC_enumerate_all_EXT") == AL_TRUE) {
> +    s = (char *)alcGetString(NULL, ALC_ALL_DEVICES_SPECIFIER);
> +  } else {
> +    s = (char *)alcGetString(NULL, ALC_DEVICE_SPECIFIER);

Make s a const pointer instead of casting the const away, that might
hide serious bugs.
Also I doubt that the result of ALC_DEVICE_SPECIFIER is of any use,
so I think just printing nothing (or a info message) if that
extension is not available seems more reasonable.

> +    while (*(ptr += strlen(s)+1) != 0) {
> +      mp_msg(MSGT_AO, MSGL_FATAL, "  %s\n", s);
> +      s = ptr;
> +    }
> +    mp_msg(MSGT_AO, MSGL_FATAL, "  %s\n\n", s);

Huh? Did you maybe just mean:
while (*s) {
    mp_msg(MSGT_AO, MSGL_FATAL, "  %s\n", s);
    s += strlen(s) + 1;
}

> +  if (subopt_parse(ao_subdevice, subopts) != 0 || display_help) {
>      print_help();
> -    return 0;
> +    if (!display_help)
> +      return 0;

You should also fail if someone requested help, that's more consistent.

> -  dev = alcOpenDevice(NULL);
> +  dev = alcOpenDevice(ao_openal_device);
>    if (!dev) {
> -    mp_msg(MSGT_AO, MSGL_FATAL, "[OpenAL] could not open device\n");
> +    mp_msg(MSGT_AO, MSGL_INFO, "[OpenAL] Device not available, trying default\n");

If the user requested a specific device you should fail if its
not available. Second-guessing the user is not MPlayer style.