[FFmpeg-devel] [PATCH 1/2] libavfilter: Support the forks ABI for buffer sinks

[Michael Niedermayer]

On Fri, Aug 31, 2012 at 11:44:50PM +0200, Stefano Sabatini wrote:
> On date Friday 2012-08-31 22:38:29 +0200, Michael Niedermayer encoded:
> > With this change avconv compiled against libav and linked to ffmpegs libs
> > will run through the whole fate testsuite without any crashes.
> > 857 tests pass, the remaining tests fail one way or another, which is
> > to be expected as avconv is not a drop in replacement for ffmpeg
> > The testsuite used was the ffmpeg fate testsuite, not libavs.
> > 
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavfilter/allfilters.c  |    6 ++++++
> >  libavfilter/buffersink.c  |    8 ++++++++
> >  libavfilter/sink_buffer.c |   39 ++++++++++++++++++++++++++++++++++++---
> >  3 files changed, 50 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> > index 9e68d4c..6842ec9 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -64,8 +64,11 @@ void avfilter_register_all(void)
> >      REGISTER_FILTER (ANULLSRC,    anullsrc,    asrc);
> >      REGISTER_FILTER (FLITE,       flite,       asrc);
> >  
> 
> > +#if !AV_HAVE_INCOMPATIBLE_FORK_ABI
> >      REGISTER_FILTER (ABUFFERSINK, abuffersink, asink);
> > +#endif
> 
> After musing a few minutes I have to conclude that I'm lost at
> correctly interpreting this:
> INCOMPATIBLE (FORK ABI) => support the fork ABI, which is incompatible 
> or
> (INCOMPATIBLE FORK) ABI => support the ABI of the incompatible fork
> 
> From my understanding abuffersink is compiled only if has not an
> incompatible (i.e. has a compatible) fork ABI, otherwise only
> ffabuffersink (ff* version) is compiled in.

ffabuffersink is always available under the ffabuffersink name

by default ffabuffersink is also available under the abuffersink name
as this was what it was previously

but if AV_HAVE_INCOMPATIBLE_FORK_ABI is set then instead the variant
from libav is available under the abuffersink name.


> 
> If this interpretation is correct then I'd suggest:
> INCOMPATIBLE_FORK_ABI => !LIBAV_ABI_COMPATIBILITY, and the previous
> code becomes:
> 
> #if AV_HAVE_LIBAV_ABI_COMPATIBILITY
>       REGISTER_FILTER (ABUFFERSINK, abuffersink, asink);
> #endif

sounds like 0=1 to me


> 
> which seems easier to grasp to me.
> 
> >      REGISTER_FILTER (ANULLSINK,   anullsink,   asink);
> > +    REGISTER_FILTER (FFABUFFERSINK, ffabuffersink, asink);
> >  
> >      REGISTER_FILTER (ALPHAEXTRACT, alphaextract, vf);
> >      REGISTER_FILTER (ALPHAMERGE,  alphamerge,  vf);
> > @@ -140,7 +143,10 @@ void avfilter_register_all(void)
> >      REGISTER_FILTER (SMPTEBARS,   smptebars,   vsrc);
> >      REGISTER_FILTER (TESTSRC,     testsrc,     vsrc);
> >  
> > +#if !AV_HAVE_INCOMPATIBLE_FORK_ABI
> >      REGISTER_FILTER (BUFFERSINK,  buffersink,  vsink);
> > +#endif
> > +    REGISTER_FILTER (FFBUFFERSINK,ffbuffersink,vsink);
> >      REGISTER_FILTER (NULLSINK,    nullsink,    vsink);
> >  
> >      /* multimedia filters */
> > diff --git a/libavfilter/buffersink.c b/libavfilter/buffersink.c
> > index e66a099..50cd6d4 100644
> > --- a/libavfilter/buffersink.c
> > +++ b/libavfilter/buffersink.c
> > @@ -141,7 +141,11 @@ int av_buffersink_read_samples(AVFilterContext *ctx, AVFilterBufferRef **pbuf,
> >  }
> >  
> >  AVFilter avfilter_vsink_buffer = {
> > +#if AV_HAVE_INCOMPATIBLE_FORK_ABI
> > +    .name      = "buffersink",
> > +#else
> >      .name      = "buffersink_old",
> > +#endif
> >      .description = NULL_IF_CONFIG_SMALL("Buffer video frames, and make them available to the end of the filter graph."),
> >      .priv_size = sizeof(BufferSinkContext),
> >      .uninit    = uninit,
> > @@ -156,7 +160,11 @@ AVFilter avfilter_vsink_buffer = {
> >  };
> >  
> >  AVFilter avfilter_asink_abuffer = {
> > +#if AV_HAVE_INCOMPATIBLE_FORK_ABI
> > +    .name      = "abuffersink",
> > +#else
> >      .name      = "abuffersink_old",
> > +#endif
> >      .description = NULL_IF_CONFIG_SMALL("Buffer audio frames, and make them available to the end of the filter graph."),
> >      .priv_size = sizeof(BufferSinkContext),
> >      .uninit    = uninit,
> > diff --git a/libavfilter/sink_buffer.c b/libavfilter/sink_buffer.c
> > index 95ba590..81bded0 100644
> > --- a/libavfilter/sink_buffer.c
> > +++ b/libavfilter/sink_buffer.c
> > @@ -143,7 +143,7 @@ int av_buffersink_get_buffer_ref(AVFilterContext *ctx,
> >      int ret;
> >      *bufref = NULL;
> >  
> > -    av_assert0(!strcmp(ctx->filter->name, "buffersink") || !strcmp(ctx->filter->name, "abuffersink"));
> > +    av_assert0(!strcmp(ctx->filter->name, "buffersink") || !strcmp(ctx->filter->name, "abuffersink") || !strcmp(ctx->filter->name, "ffbuffersink") || !strcmp(ctx->filter->name, "ffabuffersink"));
> 
> Nit: please split overly long line (and vertical align may help
> readability), same below.

will do


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

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120901/af7aac1e/attachment.asc>