[FFmpeg-devel] [PATCH] dct-test compile fix

Måns Rullgård mans
Mon Jun 23 17:50:18 CEST 2008


Ronald S. Bultje wrote:
> Hi,
>
> On Mon, Jun 23, 2008 at 8:20 AM, M?ns Rullg?rd <mans at mansr.com> wrote:
>> Ronald S. Bultje wrote:
>>> most recent svn fails to compile dct-test for me:
>>>
>>> In file included from dct-test.c:35:
>>> dsputil.h: In function 'copy_block2':
>>> dsputil.h:696: warning: implicit declaration of function 'AV_WN16'
>>> dsputil.h:696: warning: implicit declaration of function 'AV_RN16'
>>> dsputil.h: In function 'copy_block4':
>>> dsputil.h:707: warning: implicit declaration of function 'AV_WN32'
>>> dsputil.h:707: warning: implicit declaration of function 'AV_RN32'
>>> dct-test.c: In function 'main':
>>> dct-test.c:537: error: 'mm_flags' undeclared (first use in this function)
>>> dct-test.c:537: error: (Each undeclared identifier is reported only once
>>> dct-test.c:537: error: for each function it appears in.)
>>>
>>> There's two problems manifested here. First, avutil.h does not include
>>> intreadwrite.h, where the AV_[RW]N{16,32} macros are defined.
>>
>> Are these macros used in avutil.h?  If not, avutil.h should not
>> include intreadwrite.h.  More likely, dsputil.h should be including
>> intreadwrite.h, since it appears to be using things from it.
>
> The logic is that dsputil.h has a single include, avcodec.h.
> avcodec.h, again, has a single include, avutil.h. I think the idea is
> that avutil.h combines all include headers from libavutil/, so I tried
> to keep in line there.

That's not the idea.  The idea is that every header file includes
whatever it needs.

> Either way is fine with me, of course.

Good.

>>> The
>>> second problem is that mm_flags is conditionally defined in dsputil if
>>> HAVE_MMX is defined, but dct-test.c does not include config.h and thus
>>> it is never defined.
>>
>> This area is a mess.  It was obviously written with only x86 in mind.
>> A general cleanup would be welcome.
>
> I think you're right. So, part 1 is: where should mm_flags be defined?
> It's an extern int in dsputil.h, and then all archs define it for
> themselves. Logical, because the values differ per arch, but messy
> indeed. Should I move declaration to dsputil.c? Without HAVE_MMX, it
> will indeed still fail to compile.
>
> I guess in short: any pointers welcome and I'll do the work.

I'm thinking we should try to unify the way in which these things are
handled on various architectures.  I'm afraid I don't have a clear
idea of how I'd like it though.

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




More information about the ffmpeg-devel mailing list