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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080605/0b80760a/attachment.pgp>