Author: adrian Date: Mon Jul 12 22:19:09 2010 New Revision: 31725 Log: Improve handling of the "help" suboption in coreaudio: Avoid flagging it as an error and continue intialization if it is selected. Modified: trunk/libao2/ao_coreaudio.c Modified: trunk/libao2/ao_coreaudio.c ============================================================================== --- trunk/libao2/ao_coreaudio.c Mon Jul 12 20:04:05 2010 (r31724) +++ trunk/libao2/ao_coreaudio.c Mon Jul 12 22:19:09 2010 (r31725) @@ -400,18 +400,20 @@ OSStatus err; UInt32 size, maxFrames, b_alive; char *psz_name; AudioDeviceID devid_def = 0; -int device_id; +int device_id, display_help = 0; const opt_t subopts[] = { {"device_id", OPT_ARG_INT, &device_id, NULL}, + {"help", OPT_ARG_BOOL, &display_help, NULL}, {NULL} }; // set defaults device_id = 0; - if (subopt_parse(ao_subdevice, subopts) != 0) { + if (subopt_parse(ao_subdevice, subopts) != 0 || display_help) { print_help(); + if (!display_help) return 0; }
On Mon, Jul 12, 2010 at 10:19:09PM +0200, adrian wrote:
Improve handling of the "help" suboption in coreaudio: Avoid flagging it as an error and continue intialization if it is selected.
How is that an improvement? Before it would at least quit for audio files, I doubt anyone really wants MPlayer to play something when they asked for help (even though that's currently not easily avoidable). I also think it is inconsistent with almost all other MPlayer ao/vos
On Mon, Jul 12, 2010 at 22:50, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:
How is that an improvement? Before it would at least quit for audio files, I doubt anyone really wants MPlayer to play something when they asked for help (even though that's currently not easily avoidable). I also think it is inconsistent with almost all other MPlayer ao/vos
No, it wouldn't quit. It would only fail to initialize the audio output and continue anyway - without audio. In case you've selected multiple audio outs it would use the next one. In the context of the init function, returning 0 means the output failed to initialize, which I think is wrong to return in case nothing is actually wrong. Like in this case, where the user wants to see the help and not watch a video without sound. Of course, best would be if it just printed the help and the exited, just like with most other help option. But then it also should parse the suboptions before any initialization so that the help can be displayed even if no audio stream is being played and before any other initialization messages. The other similar lists I could find (-vf format=fmt=help and -pphelp) are special cases and that seems overkill to do for ao_coraudio. There isn't anything comparable I could find since all other lists are more top-level than ao_coreaudio. In this situation, I think that this commit is actually an improvement. It avoids the unexpected behavior of failing to initialize the audio output you probably want to use and then either using none or another one. Sure, the situation is far from ideal but all other options require much more work and so this seemed to be the best compromise for the moment. One option would be to exit after showing the help but I'm not sure how clean that would be and it would be inconsistent with all other audio outputs, which show the help and then continue with either no or the next audio output. Greetings, Adrian
On Mon, Jul 12, 2010 at 11:31:08PM +0200, Adrian Stutz wrote:
On Mon, Jul 12, 2010 at 22:50, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:
How is that an improvement? Before it would at least quit for audio files, I doubt anyone really wants MPlayer to play something when they asked for help (even though that's currently not easily avoidable). I also think it is inconsistent with almost all other MPlayer ao/vos
No, it wouldn't quit. It would only fail to initialize the audio output and continue anyway - without audio.
Continue anyway without audio for audio files? How would that be possible?
On Tue, Jul 13, 2010 at 08:13, Reimar Döffinger <Reimar.Doeffinger@gmx.de> wrote:
Continue anyway without audio for audio files? How would that be possible?
Yes, in case you're playing a file with only an audio stream, it would quit. But if you're playing a file that has a video stream as well or if you've selected more then one audio out then it will continue to play the file. So, for the case of audio-only files, the effect is the same as using e.g. -vo help - but disregarding all the initialization messages that are printed before the audio output is initialized and, for the consistency you talk about, ugly error messages at the end:
Failed to initialize audio driver 'coreaudio:help' Could not open/initialize audio device -> no sound. Audio: no sound Video: no video
Again, I agree with you this isn't ideal. But it isn't ideal for any help that has to be triggered by suboption parsing that only kicks in during component initialization. I still think this commit is an improvement and also think it's more consistent with what's happening: MPlayer is trying to find an audio output and tries to initialize the selected ones until it finds one that works. Signaling that the initialization failed because the user requested a help to be displayed is wrong. It either should cleanly display the help and not continue with initialization (and not initialize anything before it) or then just regard the help display as a side effect and continue - as I did with this commit, because it's the better short-term solution. In which case should a help command lead to error messages like the ones above? Greetings Adrian
participants (3)
-
adrian -
Adrian Stutz -
Reimar Döffinger