[FFmpeg-devel] [PATCH] #define LAVF_API_MAX_STREAMS

[Aurelien Jacobs]

On Tue, Aug 17, 2010 at 07:19:47AM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Aug 17, 2010, at 4:37 AM, Aurelien Jacobs <aurel at gnuage.org> wrote:
> 
> > On Tue, Aug 17, 2010 at 01:23:17AM +0100, M?ns Rullg?rd wrote:
> >> Aurelien Jacobs <aurel at gnuage.org> writes:
> >> 
> >>> Hi,
> >>> 
> >>> We currently have quite a few new API (or API deprecation) in lavf/lavc
> >>> which are waiting for a major version bump to be fully enabled. Some of
> >>> those new API are fully functionnal and ready to be used, some of them
> >>> are work in progress that don't even compile for now (eg. AVPalette
> >>> removal).
> >>> All of them are currently protected by some check like:
> >>>  #if LIBAVFORMAT_VERSION_MAJOR < 53
> >>> So we now have quite of mess of MAJOR version check all around and it's
> >>> not always easy to say which one match which API change. Moreover, it's
> >>> not easy to enable just one of those API change to conduct some tests.
> >>> Just bumping the major lead to a non-compilable source tree.
> >>> 
> >>> So I propose to add one new define for each API transition we are
> >>> working on. This would allow to trivially test each transition
> >>> individually, and also to trivially re-schedule an (incomplete)
> >>> transition to next major bump when bumping major.
> >>> 
> >>> For example this allows testing the API transition with something like:
> >>>  ./configure --extra-cflags=-DLAVF_API_MAX_STREAMS=0
> >>> 
> >>> Attached patch does this for the currently worked on MAX_STREAMS
> >>> transition, and I plan to propose patches for other transitions if this
> >>> one is accepted.
> >>> 
> >>> Aurel
> >>> 
> >>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >>> index 1f08d1a..afcb344 100644
> >>> --- a/libavformat/avformat.h
> >>> +++ b/libavformat/avformat.h
> >>> @@ -35,6 +35,10 @@
> >>> 
> >>> #define LIBAVFORMAT_IDENT       "Lavf" AV_STRINGIFY(LIBAVFORMAT_VERSION)
> >>> 
> >>> +#ifndef LAVF_API_MAX_STREAMS
> >>> +#define LAVF_API_MAX_STREAMS  (LIBAVFORMAT_VERSION_MAJOR < 53)
> >>> +#endif
> >>> +
> >>> /**
> >>>  * I return the LIBAVFORMAT_VERSION_INT constant.  You got
> >>>  * a fucking problem with that, douchebag?
> >>> @@ -622,7 +624,7 @@ typedef struct AVChapter {
> >>>     AVMetadata *metadata;
> >>> } AVChapter;
> >>> 
> >>> -#if LIBAVFORMAT_VERSION_MAJOR < 53
> >>> +#if LAVF_API_MAX_STREAMS
> >>> #define MAX_STREAMS 20
> >>> #endif
> >> 
> >> I dislike the idea of putting transitional hacks in the public
> >> headers.
> > 
> > What do you call transitional hacks ?
> > The #if LIBAVFORMAT_VERSION_MAJOR < 53 all over in public header is as
> > much of a transitional hack IMO.
> > 
> >> I also doubt the usefulness of this in the first place.
> > 
> > There are many potential use of this.
> > First let me show an example of what we could have in the end:
> > 
> > #define LAVF_API_MAX_STREAMS   (LIBAVFORMAT_VERSION_MAJOR < 53)
> > #define LAVF_API_OLD_METADATA  (LIBAVFORMAT_VERSION_MAJOR < 53)
> > #define LAVF_API_OLD_SEEK      (LIBAVFORMAT_VERSION_MAJOR < 54)
> > #define LAVF_API_PALETTE       (LIBAVFORMAT_VERSION_MAJOR < 54)
> > 
> > The transitions which are ready and fully functional would depend on
> > next major bump, and the one which are work in progress would depend on
> > next+1.
> > We could promote a transition from next+1 to next when it is finally
> > working, by just changing this one line, instead of having to grep all
> > over the tree and decide which LIBAVFORMAT_VERSION_MAJOR check belong to
> > this peculiar transition. We could also demote a transition from next to
> > next+1 at any time if we decide it not ready.
> > It would then become easy to always be in a state where we can decide to
> > bump major at any time without the fear to break anything and without
> > having to review every single LIBAVFORMAT_VERSION_MAJOR check to decide
> > which one is ready and which one must be postponed.
> > This may also allow to have a fate machine dedicated to test those API
> > transitions to ensure they always stay functional, instead of having
> > some dead code that is never test-compiled.
> > Right now, we can't even test transitions because bumping major just
> > don't work (some transitions are incomplete and don't compile).
> > 
> > Overall, this is meant to ease API transition an major bump, and to
> > increase tests code coverage.
> 
> Your patch doesn't do any of the above yet. Are you volunteering? :-).
> They are good ideas.

I volunteer to add proper defines for each of the currently ongoing
transition, to test those transitions, and to demote to one which are
not ready to v54 instead of v53.
I hope somebody will catch this on the fly to integrate it into fate...

Aurel