[vlc-devel] [PATCH] switcher: Fix compilation with ffmpeg-0.10.2

[Richard Grenville]

On Tue, Mar 20, 2012 at 10:25:10PM -0400, Rafaël Carré wrote:
> Le 2012-03-20 21:58, Richard Grenville a écrit :
> > Removes the deprecated symbols used in switcher.c to make it compile
> > correctly against ffmpeg-0.10.2
> > 
> > I left the deprecated function call avcodec_encode_audio() unchanged
> > since I guess I'm not capable to convert it to use the new
> > avcodec_encode_audio2(), it requires too many changes.
> > 
> > Originally posted on: https://bugs.gentoo.org/show_bug.cgi?id=409001
> > 
> > I have no knowledge about the ffmpeg API, so please review the patch
> > carefully, as I could not guarantee it's 100% correct.
> 
> This breaks building with older versions.
> 

Sorry. Yes, that breaks the compatibility indeed.

Well, start from the beginning, someone found on Gentoo vlc-2.0.1 fails
to compile with --enable-switcher:
https://bugs.gentoo.org/show_bug.cgi?id=409001

The error messages:
---
switcher.c: In function 'VideoGetBuffer':
switcher.c:847:34: error: 'FF_I_TYPE' undeclared (first use in this function)
switcher.c:847:34: note: each undeclared identifier is reported only once for each function it appears in
switcher.c:856:34: error: 'FF_P_TYPE' undeclared (first use in this function)
switcher.c:935:10: error: 'FF_B_TYPE' undeclared (first use in this function)
---

I read the error messages and found it's probably caused by a few
deprecated symbols FF_*_TYPE used in ./modules/stream_out/switcher.c, so
I wrote a patch to replace FF_*_TYPE with AV_PICTURE_TYPE_* in switch.c,
replaced some deprecated function calls in switcher.c by the way, found
it works, and submitted here.

But later I found the problem is much more complicated. Some lines from
my /usr/include/libavcodec/avcodec.h (ffmpeg-0.10.2):

---
#if FF_API_OLD_FF_PICT_TYPES
/* DEPRECATED, directly use the AV_PICTURE_TYPE_* enum values */
#define FF_I_TYPE  AV_PICTURE_TYPE_I  ///< Intra
#define FF_P_TYPE  AV_PICTURE_TYPE_P  ///< Predicted
#define FF_B_TYPE  AV_PICTURE_TYPE_B  ///< Bi-dir predicted
#define FF_S_TYPE  AV_PICTURE_TYPE_S  ///< S(GMC)-VOP MPEG4
#define FF_SI_TYPE AV_PICTURE_TYPE_SI ///< Switching Intra
#define FF_SP_TYPE AV_PICTURE_TYPE_SP ///< Switching Predicted
#define FF_BI_TYPE AV_PICTURE_TYPE_BI
#endif
---

And /usr/include/libavcodec/version.h :
---
#define LIBAVCODEC_VERSION_MAJOR 53
/* ... */
#define FF_API_OLD_FF_PICT_TYPES (LIBAVCODEC_VERSION_MAJOR < 54)
---

So ffmpeg should already have replaced FF_*_TYPE with the correct
AV_PICTURE_TYPE_*, why this is not happening? Eventually I found these
things in vlc's git tree, ./modules/codec/avcodec.h, which switch.c
includes, too:

---
#if LIBAVCODEC_VERSION_MAJOR < 54
#   define AV_PICTURE_TYPE_B        FF_B_TYPE
#   define AV_PICTURE_TYPE_I        FF_I_TYPE
#   define AV_PICTURE_TYPE_P        FF_P_TYPE
---

They are introduced in commit 8f24e725ff9945975f6d7232477cff7f7a8e2cff
"avcodec: update to libavcodec 54", by Rafaël Carré, aka, you. Together
with ffmpeg-0.10's code in avcodec.h and version.h, they generated a
circular macro definition, replace A with B, then replace B with A...

And gcc, when it encounters a set of circular macro definitions, chooses
to ignore both macro definitions, instead of print out an error
message. So the preprocessor did not replace FF_*_TYPE with
AV_PICTURE_TYPE_*, thus causing the error, but manually replacing
FF_*_TYPE with AV_PICTURE_TYPE_* does work.

I don't actually know what your code in ./modules/codec/avcodec.h is
trying to do, but seems it's indeed causing some troubles. Could you
please look into this issue, and find a graceful and compatible
solution? Thanks.

> > ---
> >  modules/stream_out/switcher.c |   21 ++++++++++-----------
> >  1 files changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/modules/stream_out/switcher.c b/modules/stream_out/switcher.c
> > index 372eef8..50c961f 100644
> > --- a/modules/stream_out/switcher.c
> > +++ b/modules/stream_out/switcher.c
> > @@ -292,7 +292,6 @@ static int Open( vlc_object_t *p_this )
> >      p_stream->pf_send   = Send;
> >      p_stream->p_sys     = p_sys;
> >  
> > -    avcodec_init();
> >      avcodec_register_all();
> 
> This code is probably incorrect (not reentrant iirc)

I removed avcodec_init() because /usr/include/libavcodec/avcodec.h from
ffmpeg-0.10.2 told me so:

---
#if FF_API_AVCODEC_INIT
/**
 * @deprecated this function is called automatically from avcodec_register()
 * and avcodec_register_all(), there is no need to call it manually
 */
attribute_deprecated
void avcodec_init(void);
#endif
---

However I didn't pay attention to the compatiblity with older ffmpeg
versions. I always use a fully updated system and don't quite understand
what "older versions" mean. :-)