[MPlayer-dev-eng] [PATCH] New commands for loading/unloading audio filters at runtime

[Reimar Döffinger]

On Wed, Apr 07, 2010 at 10:14:51PM +0900, Jehan Pagès wrote:
> attached is the modified patch.

Looks ok to me, does someone have time to test and apply?
(I'm busy getting my new PC to work...)

> I made it a "if" because I find it
> clearer than a switch in this case (just made a 2 cases branching, if
> MP_CMD_AF_DEL, else…). I hope you don't mind (is there any performance
> difference anyway, or any other notable difference, between an if and
> a switch?).

Can't really say in general, but compilers tend to generate different code.

> To be honest, I don't really agree with this choice of compacity over
> efficiency. Ok, as you noted, this piece of code is not performance
> critical, and anyway it probably won't make any difference on any
> computer, even old ones (moreover the usage won't have people loading
> or unloading a lot of filters at the same time, I guess). But still,
> by principle, just to have a code more compact, we are adding
> comparisons at every loop. This is not my idea of the most beautiful
> code. :p But you are the boss anyway, and this is not that a big deal,
> so I won't argue. I just give my opinion on it because maybe you have
> a nice answer to enlighten my vision on beautiful code. :)

The point is only maintainability: If you have the same code twice what
usually happens over the years is that someone finds a bug in one of them
and fixes it, then someone notices a bug in the other copy and fixes it there
and suddenly you have two half-fixed pieces of code and nobody knows if they
are different intentionally or accidentally.
I know this really should not happen, but considering my experience so far
the unfortunate fact is that it does happen.
Also if speed was an issue, the right solution would be af_add/af_remove functions
that can handle multiple filters, the overhead that could be saved by that
(I suspect things like traversing the filter chain) is likely to be vastly
more than that of the if in the inner loop (particularly since the condition
is 100% predictable, and the compiler could actually extract the if outside
- or doing it manually would still avoid some code duplication, duplicating
only the while).