[FFmpeg-devel] [PATCH] make img_convert symbol conditional on lavc version, not libswscale

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Thu, Jun 05, 2008 at 04:58:25PM +0200, Diego Biurrun wrote:
>> On Thu, Jun 05, 2008 at 04:35:05PM +0200, Michael Niedermayer wrote:
>>> On Thu, Jun 05, 2008 at 09:52:13AM +0200, Diego Biurrun wrote:
>>>> On Thu, Jun 05, 2008 at 12:38:11AM -0700, Baptiste Coudurier wrote:
>>>>> Diego Biurrun wrote:
>>>>>> On Tue, Jun 03, 2008 at 01:46:18PM +0200, Diego Biurrun wrote:
>>>>>>> Currently we declare img_convert() in avcodec.h conditional to
>>>>>>>
>>>>>>> #if LIBAVCODEC_VERSION_INT < ((52<<16)+(0<<8)+0)
>>>>>>>
>>>>>>> However, in imgconvert.c, img_convert is defined conditional to
>>>>>>>
>>>>>>> #ifndef CONFIG_SWSCALE
>>>>>>>
>>>>>>> so that img_convert() is not available when compiling with swscale
>>>>>>> enabled although it is declared in avcodec.h.
>>>>>>>
>>>>>>> Here is a patch to change the condition in imgconvert.c, which I believe
>>>>>>> is the correct solution.
>>>>>> I will commit this tomorrow unless I hear objections.
>>>>> imgresample.c uses img_convert, is it safe ?
>>>> That is indeed a problem, but separate from the one my patch addresses.
>>>> The header promises the symbol conditional on lavc version, so the
>>>> implementation must IMO follow.  We cannot make a condition based on
>>>> CONFIG_SWSCALE in avcodec.h because installed headers do not #include
>>>> config.h.
>>> Iam not completely sure what is the correct awnser but
>>>  --enable-swscale means "use the new API", one should not use that and then
>>>  complain that the old isnt available anymore ...
>> But our header says that it is still available and the condition in the
>> header contradicts the condition in the implementation.  So something
>> must be buggy...
> 
> well then, it should not be in the header when sws is enabled.
> 
> 
>>> thus IMHO the patch is wrong in the sense that either --enable-swscale should
>>> not be used or the code using lavc should use swscale instead of the old API.
>> Debian and Mandriva use the following patch which enables both
>> libswscale and imgrescale:
>>
>> http://svn.debian.org/wsvn/pkg-multimedia/unstable/ffmpeg/debian/patches/015_reenable-img_convert.diff?op=file&rev=0&sc=0
>>
>> I've attached it for convenience.
>>
>>> besides this the patch may or may not be totally buggy ive not checked this
>>> but i think using the old scaler while the new is enabled has not been
>>> considered when the code was written ...
>> Is enabling img_convert really a problem?  Can't the scalers be used
>> side by side?
> 
> Its duplicated code and bloat at least and as said i do not know if it works
> at all.
> As i said the distros should not be using the old API when they disable it
> during configure. 
> --enable-swscale really means 
> enable new scaler
> disable old scaler
> disable old API as its not compatible with the new scaler
> 
> So either
> A. do not use --enable-swscale
> B. change all applications one by one to the new API _before_ adding
>   --enable-swscale
> 

For information, I use the new swscale and ported some applications to
it, but Im lazy to port old ones which do not require accelerated
functions, so I simply removed #ifdef SWSCALE, and everything seems to
work correctly. I can perfectly use img_convert directly.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA