[MPlayer-dev-eng] [PATCH] SMPlayer's audio equalizer slave command patch
Reimar Döffinger
Reimar.Doeffinger at gmx.de
Tue Oct 12 22:29:46 CEST 2010
On Tue, Oct 12, 2010 at 10:15:27PM +0200, Adrian Stutz wrote:
> Index: command.c
> ===================================================================
> --- command.c (revision 32482)
> +++ command.c (working copy)
> @@ -3439,6 +3439,22 @@
> af_init(mpctx->mixer.afilter);
> build_afilter_chain(sh_audio, &ao_data);
> break;
> + case MP_CMD_AF_CMDLINE:
> + if (!sh_audio)
> + break;
> + char* af_name = strdup(cmd->args[0].v.s);
> + char* af_args = strdup(cmd->args[1].v.s);
> + af_instance_t* af = af_get(mpctx->mixer.afilter, af_name);
I don't see the point for the strdups.
Also, adding (initialized) variable declarations in the middle
of a case without opening a new block (using {}) is a really bad idea,
a lot of people are not aware of the slightly tricky semantics (and
tools like coverity will complain about it).
Also (I notice a few things are copied from the surrounding code.
Well, unfortunately for you that is quite a mess)
1) checking for !sh_audio when actually using mixer.afilter does
not exactly make sense
2) it should use sh_audio->afilter, not mixer.afilter
3) it probably should check that one is not NULL
> + if (af != NULL)
> + af->control(af, AF_CONTROL_COMMAND_LINE, af_args);
> + else
> + mp_msg(MSGT_CPLAYER, MSGL_WARN,
> + "Filter '%s' not found in chain.\n", af_name);
> + af_init(mpctx->mixer.afilter);
This almost certainly should be
af_reinit(sh_audio->afilter, af);
It should at least have much better performance when modifying
only the last filter in a long filter chain.
More information about the MPlayer-dev-eng
mailing list