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

[kiriuja]

> > +    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?

You probably noticed that mp_msg is only called when -identify
is on. If you put the function call inside it, it won't work
when -identify is off.

> > -/** \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 problem of English and writing style is not local to just
that one line, it's all over the place. If you would like to
improve it, first specify the rules that should be followed and
then submit a patch. Fixing it line by line when opportunity
comes up clearly won't do.

--
kiriuja