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

Aurelien Jacobs aurel
Wed Jun 25 01:01:40 CEST 2008


Diego Biurrun wrote:

> On Wed, Jun 18, 2008 at 01:00:37PM +0200, Diego Biurrun wrote:
> > On Fri, Jun 06, 2008 at 11:35:28PM +0200, Michael Niedermayer wrote:
> > > On Fri, Jun 06, 2008 at 04:25:30PM +0200, Diego Biurrun wrote:
> > > > On Thu, Jun 05, 2008 at 05:20:46PM +0200, 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.
> > > > 
> > > > Then how about adding the declarations to imgconvert.h and installing
> > > > that header depending on swscale being enabled or not?
> > > 
> > > imgconvert.h is a internal header ...
> > 
> > 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.

> Index: libavcodec/Makefile
> ===================================================================
> --- libavcodec/Makefile	(revision 13801)
> +++ libavcodec/Makefile	(working copy)
> @@ -366,8 +366,9 @@
>  
>  OBJS-$(HAVE_XVMC)                      += xvmcvideo.o
>  
> -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.

> Index: libavcodec/imgconvert.c
> ===================================================================
> --- libavcodec/imgconvert.c	(revision 13801)
> +++ libavcodec/imgconvert.c	(working copy)
> @@ -2087,7 +2087,8 @@
>      return 0;
>  }
>  
> -#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.

Aurel




More information about the ffmpeg-devel mailing list