[FFmpeg-devel] [PATCH] swscale: Do not misuse HAVE_* flags.

[Ramiro Polla]

Hello Michael,

On Sun, Apr 5, 2009 at 6:11 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> On Sun, Apr 05, 2009 at 06:00:19PM -0300, Ramiro Polla wrote:
>> On Sun, Apr 5, 2009 at 5:51 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > On Sun, Apr 05, 2009 at 05:28:21PM -0300, Ramiro Polla wrote:
>> >> On Sun, Apr 5, 2009 at 5:14 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> >> > On Sun, Apr 05, 2009 at 02:14:15PM -0300, Ramiro Polla wrote:
>> >> >> swscale: Do not misuse HAVE_* flags.
>> >> >> Rename COMPILE_* to SWSCALE_COMPILE_*.
>> >> >> Rename wrong uses of HAVE_* to COMPILE_*.
>> >> >
>> >> > [...]
>> >> >> @@ -1594,7 +1592,7 @@ error:
>> >> >> ? ? ?return ret;
>> >> >> ?}
>> >> >>
>> >> >> -#if COMPILE_MMX2
>> >> >> +#if SWSCALE_COMPILE_MMX2
>> >> >> ?static void initMMX2HScaler(int dstW, int xInc, uint8_t *funnyCode, int16_t *filter, int32_t *filterPos, int numSplits)
>> >> >> ?{
>> >> >> ? ? ?uint8_t *fragmentA;
>> >> >
>> >> > this should be HAVE_MMX2 when HAVE* are not changed
>> >>
>> >> SWSCALE_COMPILE_MMX2 is HAVE_MMX2 && CONFIG_GPL. If CONFIG_GPL is not
>> >> set, we shouldn't be compiling this. Besides, using SWSCALE_COMPILE_*
>> >> makes it consistent with the rest of the file.
>> >>
>> >> Or am I missing something?
>> >
>> > #if CONFIG_GPL
>> > # ? undef HAVE_MMX
>> > ...
>> > #endif
>>
>> I don't think it's a good idea to fiddle around with HAVE_* flags, and
>> this patch removes code that currently does it. Also if/when people
>> start rewriting parts of the GPL code, we will end up with some GPL
>> and some LGPL code. Keeping the HAVE_ flags intact is better to decide
>> what to build.
>
> iam currently maintainer of swscale and i prefer not to make
> the #ifdeffery worse than what it already is.

New patch attached (it doesn't depend on a previous patch like the
first one in this thread did).

IMO it does not make the ifdeffery worse, in fact I think it makes it
cleaner. COMPILE_TEMPLATE_ sounds much more intuitive than HAVE_ for
the given purpose, and HAVE_ is used for another purpose on the rest
of FFmpeg.

I read in an old thread you said you would not review MMX rewrites
[0]. In my previous reply to this thread I was still under the
impression that swscale MMX code was mostly written by other people
(like the libmpeg2 folks) and that's why it was GPL. I'm sorry if my
reasoning for the patch made it sound like I wanted to encourage
rewrites of your code. I just want (L)GPL code to be easily
selectable, and with the current if(!GPL) disable MMX approach it's
impossible. IIRC Kostya has been writing swscale code under LGPL, and
we already have some LGPL MMX optimization [1] which is not being
compiled in LGPL builds.

Ramiro Polla
[0] http://article.gmane.org/gmane.comp.video.ffmpeg.devel/66335
[1] http://git.ffmpeg.org/?p=libswscale;a=commitdiff;h=a2a1a8ef9d5939f8cb4652c637ecb82620c97829
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-don-t-misuse-HAVE_-defines-use-COMPILE_TEMPLATE_.patch
Type: text/x-diff
Size: 19499 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090724/2728a107/attachment.patch>