[FFmpeg-devel] [RFC] libavfilter audio framework - split patches

[Stefano Sabatini]

On date Sunday 2010-07-11 16:47:21 +0200, Michael Niedermayer encoded:
> On Sun, Jul 11, 2010 at 06:58:15AM -0700, S.N. Hemanth Meenakshisundaram wrote:
> > S.N. Hemanth Meenakshisundaram wrote:
> >>
> >> I am sending out the working lavfi audio patches in the order Stefano 
> >> requested for easy committing.
> >
> > This has avfilter.c changes. Extra debug logs, nit fixes and fixed the code 
> > for avfilter_get_samples_ref in case of insufficient permissions.
> 
> >  avfilter.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> > f7a14f4c3e438f73480a2d034f869a48d0554881  avfilter.c.diff
> > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> > index 38ca3b1..6d265aa 100644
> > --- a/libavfilter/avfilter.c
> > +++ b/libavfilter/avfilter.c
> > @@ -60,6 +60,22 @@ void avfilter_unref_pic(AVFilterPicRef *ref)
> >      av_free(ref);
> >  }
> >  
> > +AVFilterSamplesRef *avfilter_ref_samples(AVFilterSamplesRef *ref, int pmask)
> > +{
> > +    AVFilterSamplesRef *ret = av_malloc(sizeof(AVFilterSamplesRef));
> > +    *ret = *ref;
> > +    ret->perms &= pmask;
> > +    ret->buffer->refcount++;
> > +    return ret;
> > +}
> 
> duplicate of avfilter_ref_pic()
> 
> 
> > +
> > +void avfilter_unref_samples(AVFilterSamplesRef *ref)
> > +{
> > +    if (!(--ref->buffer->refcount))
> > +        ref->buffer->free(ref->buffer);
> > +    av_free(ref);
> > +}
> 
> duplicate of avfilter_unref_pic()
> 
> 
> > +
> >  void avfilter_insert_pad(unsigned idx, unsigned *count, size_t padidx_off,
> >                           AVFilterPad **pads, AVFilterLink ***links,
> >                           AVFilterPad *newpad)
> > @@ -97,7 +113,9 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad,
> >      link->dst     = dst;
> >      link->srcpad  = srcpad;
> >      link->dstpad  = dstpad;
> > +    link->type    = src->output_pads[srcpad].type;
> >      link->format  = PIX_FMT_NONE;
> > +    link->aformat = SAMPLE_FMT_NONE;
> >  
> >      return 0;
> >  }
> > @@ -210,6 +228,20 @@ AVFilterPicRef *avfilter_get_video_buffer(AVFilterLink *link, int perms, int w,
> >      return ret;
> >  }
> >  
> > +AVFilterSamplesRef *avfilter_get_samples_ref(AVFilterLink *link, int perms, int size,
> > +                                              int64_t channel_layout, enum SampleFormat sample_fmt, int planar)
> > +{
> > +    AVFilterSamplesRef *ret = NULL;
> > +
> > +    if (link_dpad(link).get_samples_ref)
> > +        ret = link_dpad(link).get_samples_ref(link, perms, size, channel_layout, sample_fmt, planar);
> > +
> > +    if (!ret)
> > +        ret = avfilter_default_get_samples_ref(link, perms, size, channel_layout, sample_fmt, planar);
> > +
> > +    return ret;
> > +}
> 
> duplicate of avfilter_get_video_buffer()
> 
> 
> > +
> >  int avfilter_request_frame(AVFilterLink *link)
> >  {
> >      DPRINTF_START(NULL, request_frame); dprintf_link(NULL, link, 1);
> > @@ -221,6 +253,14 @@ int avfilter_request_frame(AVFilterLink *link)
> >      else return -1;
> >  }
> >  
> > +int avfilter_request_samples(AVFilterLink *link)
> > +{
> > +    if (link_spad(link).request_samples)
> > +        return link_spad(link).request_samples(link);
> > +    else if (link->src->inputs[0])
> > +        return avfilter_request_samples(link->src->inputs[0]);
> > +    else return AVERROR(EINVAL);
> > +}
> >  int avfilter_poll_frame(AVFilterLink *link)
> >  {
> >      int i, min=INT_MAX;
> 
> duplicate avfilter_request_frame()

OK I indeed have the same concerns. I see two ways for fixing this:
implementing some form of heiry macros, *or* generalize the code.

For example let's consider this:

AVFilterSamplesRef *avfilter_ref_samples(AVFilterSamplesRef *ref, int pmask)
{
    AVFilterSamplesRef *ret = av_malloc(sizeof(AVFilterSamplesRef));
    *ret = *ref;
    ret->perms &= pmask;
    ret->buffer->refcount++;
    return ret;
}

AVFilterSamplesRef *avfilter_ref_pic(AVFilterPicRef *ref, int pmask)
{
    AVFilterPicRef *ret = av_malloc(sizeof(AVFilterPicRef));
    *ret = *ref;
    ret->perms &= pmask;
    ret->buffer->refcount++;
    return ret;
}

We may factorize function definitions like this:
#define DEFINE_AVFILTER_REF(refname, refstruct) *avfilter_ref_##refname(refstruct *ref, int pmask) \
{ \
    refstruct *ret = av_malloc(sizeof(refstruct)); \
    *ret = *ref; \
    ret->perms &= pmask; \
    ret->buffer->refcount++; \
    return ret; \
}

DEFINE_AVFILTER_REF(pic    , AVFilterPicRef    );
DEFINE_AVFILTER_REF(samples, AVFilterSamplesRef);

The alternative may be to define and use an AVFilterBuffer, define
avfilter_ref_buffer()/avfilter_unref_buffer(), and make each
AVFilterPicRef and AVFilterPicSamples refer to such an
AVFilterBuffer.

The latter would break ABI/API, but that's maybe currently not a
concern.

Regards.
-- 
FFmpeg = Fantastic and Fancy Magnificient Pitiful Elaborated God