On Sun, May 15, 2011 at 11:34:35PM +0100, Måns Rullgård wrote:
> Diego Biurrun <[email protected]> writes:
> 
> > On Sun, May 15, 2011 at 07:21:59PM +0100, Måns Rullgård wrote:
> >> Diego Biurrun <[email protected]> writes:
> >> >
> >> > --- a/libavcodec/fft-internal.h
> >> > +++ b/libavcodec/fft-internal.h
> >> > @@ -19,6 +19,8 @@
> >> >  #ifndef AVCODEC_FFT_INTERNAL_H
> >> >  #define AVCODEC_FFT_INTERNAL_H
> >> >  
> >> > +#include "fft.h"
> >> > +
> >> >  #if CONFIG_FFT_FLOAT
> >> 
> >> Are you sure this doesn't break something?  That header is one of those
> >> slightly magical ones.
> >
> > It does not break FATE.  What else do you want me to test for?
> 
> I want you to read and understand what the file does, not just blindly
> run some tests and hope they'd catch whatever might have gone wrong.

It is safe.  First, fft-internal uses struct definitions and similar
from fft.h, so it implicitly depends on that header.  Second, the only
risky part in fft.h is the CONFIG_FFT_FLOAT definition that might conflict
with a similar definition elsewhere.

However, the definition in fft.h is conditional on CONFIG_FFT_FLOAT
being undefined and the only places where fft-internal.h is #included,
mdct.c and fft.c, are template files #included from their fixed/float
counterparts that set the CONFIG_FFT_FLOAT definition appropriately
before anything else.

Is that enough to convince you?

> >> > --- a/libswscale/x86/swscale_template.h
> >> > +++ b/libswscale/x86/swscale_template.h
> >> > @@ -18,10 +18,13 @@
> >> >  #ifndef SWSCALE_X86_SWSCALE_TEMPLATE_H
> >> >  #define SWSCALE_X86_SWSCALE_TEMPLATE_H
> >> >  
> >> > +#include <stdint.h>
> >> > +
> >> > +#include "libavutil/mem.h"
> >> > +
> >> 
> >> Probably OK, but the _template files really shouldn't be tested for
> >> standalone build.  It doesn't make sense.
> >
> > This one may look like a template, but it is not. 
> 
> Grrrr

So we need to find a catchier name for it.  I propose asm_constants.h.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to