On Mon, May 16, 2011 at 01:11:10AM +0100, Måns Rullgård wrote: > Diego Biurrun <[email protected]> writes: > > > 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? > > No, but had you pointed out that the only places it is included, fft.h > is included immediately before, it would have been.
Very well, noted for the future - is that an OK for the patch? > However, this particular header makes no sense including on its own. > What is the point in making it appear to work? I want to add checkheaders (and a few other targets) to FATE, they get continuously broken otherwise. But obviously a prerequisite for that is having the checkheaders target pass in the first place. > If you want to juggle #include lines, remove the useless ones at the end > of avutil.h and fix the fallout instead. That would remove a huge > number of false dependencies. I'll create a branch for that, but I cannot promise an ETA. Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
