[FFmpeg-cvslog] r25278 - in trunk: ffmpeg.c libavutil/Makefile libavutil/assert.h libavutil/avutil.h libavutil/rational.c

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Thu, Sep 30, 2010 at 07:37:36PM -0400, Ronald S. Bultje wrote:
>> Hi,
>> 
>> 2010/9/30 M?ns Rullg?rd <mans at mansr.com>:
>> > michael <subversion at mplayerhq.hu> writes:
>> >> -#include <assert.h>
>> >> +#include "assert.h"
>> >
>> > Using the same name as a standard header is practically guaranteed to
>> > cause confusion and mysterious bugs.
>> 
>> I second this, can you please rename this to avassert.h if nothing else?
>
> as said, feel free to rename it, to me there is no ambiguity between
>
> #include <assert.h>
> and
> #include <libavutil/assert.h>
>
>> 
>> (Review would've been nice - what problem does this solve exactly?)
>
> that of our code being filled with #undef NDEBUG hacks
>
> the problem of asserts() that are in speed critical code (aka
> enabling them slows things down significantly) and asserts that are
> in speed uncritical paths being inseperable.  The thing is a devel
> wants to enable the ones that have no speed effect most of the time
> but theres a good reason not to enable the asserts that slow the
> code down
>
> This was discussed previously, no volunteers came forward to work on it.

I tried to do something like this a couple of years ago, and you
flat-out rejected it.  That put me off the idea.

> I knew mans would attack me when i commit no matter what i commit,

Maybe you should try posting patches for a review and see what
happens.  A few nits aside, I like this change.  It is the way you
went about it I find revolting.

> i was a bit surprised to see your reply though.

The dissent is stronger than you appear to realise.

-- 
M?ns Rullg?rd
mans at mansr.com