[FFmpeg-devel] Patch: CrystalHD decoder support

[Aurelien Jacobs]

On Tue, Dec 28, 2010 at 09:46:17AM -0800, Philip Langdale wrote:
> On Tue, 28 Dec 2010 10:03:25 +0000 (UTC)
> Carl Eugen Hoyos <cehoyos at ag.or.at> wrote:
> 
> > Philip Langdale <philipl <at> overt.org> writes:
> > 
> > ...
> > 
> > > +#define _GNU_SOURCE
> > 
> > I suspect you should explain why this is needed.
> 
> It's needed to get usleep(). I will add a comment.

the usleep man page don't even mention _GNU_SOURCE, so no, it is not
needed. Here is what is used in other places in ffmpeg:
#define _XOPEN_SOURCE 600
In ffmpeg we try to stay POSIX compliant and we avoid all the
non-standard GNU extensions.

> > ...
> > 
> > > +#ifdef CONFIG_FASTMEMCPY
> > > +#include "libvo/fastmemcpy.h"
> > > +#else
> > > +#define fast_memcpy(d, s, l) memcpy(d, s, l)
> > > +#endif
> > 
> > I would suggest to remove this hunk.
> 
> Do you think there's insufficient benefit in fast_memcpy to bother
> taking advantage of it?

Is there any advantage at all ?
fast_memcpy() might even be slower than memcpy() depending on the CPU
and the libc you use. And if fast_memcpy() is faster than memcpy() in
some situations, then the optimizations should be ported inside the
libc instead of duplicating it in every single projects.
In any cases, if you want to push such an "optimization", you surly need
to show some timming comparison to advocate it.

Aurel