[FFmpeg-devel] [PATCH] movenc.c: fix metadata writing.

[Reimar Döffinger]

On Sat, Mar 05, 2011 at 06:52:00AM -0500, Ronald S. Bultje wrote:
> Hi Reimar,
> 
> On Sat, Mar 5, 2011 at 6:16 AM, Reimar D?ffinger
> <Reimar.Doeffinger at gmx.de> wrote:
> > On Fri, Mar 04, 2011 at 03:03:35PM -0500, Ronald S. Bultje wrote:
> >> On Fri, Mar 4, 2011 at 2:05 PM, Reimar D?ffinger
> >> <Reimar.Doeffinger at gmx.de> wrote:
> >> > Fixing MKTAG still would be a rather good idea...
> >>
> >> I'm not sure if MKTAG() is broken. It doesn't specify signedness or
> >> range of the input characters.
> >
> > Can you think of even a single case where the current behaviour
> > makes even the slightest bit of sense?
> > For me the answer is no, and thus it is broken.
> 
> Are we still talking about MKTAG?
> 
> I think MKTAG does the right thing. You generally call it with hex
> values (0x80, which is positive) or ascii chars ('c', which is
> positive), and in both cases the result is valid. The only case where
> the result is invalid is when you literally feed it wrong input, e.g.
> -1. But that's not really MKTAG's fault, is it? It could certainly be
> made more dummy-proof, but again, is that really MKTAG's fault?

Failing for no good reason at all is just idiotic, and code that
does idiotic things for no good reason is wrong.
We don't leave away the () around macro arguments and {} around the
code saying, "well, it's not really the macro's fault that those
idiots forget to put a {} around it and doing put its arguments in ()".
That fact that this already caused a bug should be more than enough
reason to consider it wrong.