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

[Diego Biurrun]

On Wed, Jun 25, 2008 at 01:01:40AM +0200, Aurelien Jacobs wrote:
> Diego Biurrun wrote:
> 
> > On Wed, Jun 18, 2008 at 01:00:37PM +0200, Diego Biurrun wrote:
> > > 
> > > Here is an alternative version that adds a configure option to disable
> > > or enable the old scaler.  This way distros do not have to patch the
> > > source should they wish to have both things side by side.
> > > 
> > > I moved the function declarations to a new header file that is installed
> > > if the old scaler is enabled.
> 
> So you propose to have a header which is installed conditionally ?
> This looks odd. And I can't see what you're trying to achieve with this
> new header. The only difference is that applications using the old
> API will fail to compile (missing header) when ffmpeg is compiled without
> old scaler, while current situation is that it fails to link (missing
> symbol). This don't really makes a difference IMO.
> Old apps won't work with an ffmpeg built without old scaler anyway.
> So I think you don't need to mess with the headers at all.

Adding a new header #include is trivial, porting applications to the new
API is not.

I never said my proposed patch is a beauty, most definitely not.  But
while we still have both APIs, we should not ignore the problems the
distros are experiencing.

> > --- libavcodec/Makefile	(revision 13801)
> > +++ libavcodec/Makefile	(working copy)
> > @@ -366,8 +366,9 @@
> >  
> > -ifndef CONFIG_SWSCALE
> > +ifdef CONFIG_OLD_SCALER
> >  OBJS += imgresample.o
> 
> This don't seem to be a good idea.
> compiling both imgresample and swscale will provide the same symbols
> in libswscale and libavcodec.
> The swscale compatible API in imgresample should never be built when
> swscaler is enabled.

Good idea or not, this is what distributions currently do.  They need
both APIs and are applying much more hackish patches than this one.

> > --- libavcodec/imgconvert.c	(revision 13801)
> > +++ libavcodec/imgconvert.c	(working copy)
> > @@ -2087,7 +2087,8 @@
> >  
> > -#if LIBAVCODEC_VERSION_INT < ((52<<16)+(0<<8)+0)
> > +#ifdef CONFIG_OLD_SCALER
> 
> IMO, there's no reason to remove the LIBAVCODEC_VERSION_INT test.
> This API is deprecated and due to be removed in next major version,
> whatever hack you will do to make it usable until then.

Maybe, but the problem is that right now some of the .c and .h files
disagree about the conditionals.  Parts of the code are compiled
depending on LIBAVCODEC_VERSION_INT, others on CONFIG_SWSCALE.  What we
have right now *is* buggy.

Diego