[MPlayer-dev-eng] [PATCH] Optional parameter for switch_audio

[The Wanderer]

Reimar Döffinger wrote:

> Hi, On Tue, Apr 19, 2005 at 11:16:08PM -0400, kiriuja wrote:
> 
>> Updated patch per Reimar's suggestion.
>> 
>> Returns the currently played -aid in all cases.
> 
> Any comments? Looks fine to me...

Just something random, which may reflect only an ignorance of standard
coding styles, and a cosmetic issue:

> +    case MP_CMD_SWITCH_AUDIO : {
> +        int v = demuxer_switch_audio(demuxer, cmd->args[0].v.i);
> +        if (identify)
> +          mp_msg(MSGT_GLOBAL, MSGL_INFO, "ID_AUDIO_TRACK=%d\n", v);
> +    } break;

Why create the variable "v" at all, instead of simply calling
demuxer_switch_audio directly into the mp_msg call? Unless my
understanding of C is off, v will disappear after the end of the case
anyway, so there doesn't seem to be any benefit to storing the value...
unless it's that the function call, which does in fact do something
other than return information, is kept separate from the informative
line which isn't directly related.

>  void demux_mkv_seek (demuxer_t *demuxer, float rel_seek_secs, int flags);
>  
> -/** \brief Given a matroska track number, find the subtitle number that mplayer would ask for.
> +/** \brief Given a matroska track number and type, find the id that mplayer would ask for.

As long as we're changing this line anyway, why not capitalize
"Matroska" and "MPlayer" (and possibly "ID")? (Unnecessary, but keeping
the documentation good English is my primary contribution... and code
comments are a form of documentation, doxygen comments especially, so if
the opportunity comes up to make improvements without creating clutter
why not take advantage of it?)

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

A government exists to serve its citizens, not to control them.