[FFmpeg-devel] [RFC] libavfilter audio framework - split patches
S.N. Hemanth Meenakshisundaram
smeenaks
Mon Jul 12 01:35:48 CEST 2010
Stefano Sabatini wrote:
> 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.
>
Hi,
I can do the 2nd option itself and make both AVFilterPicRef and
AVFilterSamplesRef point to a common AVFilterBuffer. However, a couple
of questions/observations about this:
1. Then every audio buffer would also carry around w, h and format
(PixelFormat) fields. Is this ok?
2. From the code, it looks like w and h can be safely removed from the
buffer struct since it is never used in any of the filters. All filters
use only the w and h values in the referencing PicRef struct. I would
only need to remove the code in defaults.c (get_video_buffer) that
assigns the w and h values of the buffer struct. Can I do this (as a
separate patch of course)?
3. The PixelFormat field however would need to be moved to PicRef
(PicRef currently has no PixelFormat field) and a couple of filters -
overlay, vsrc_buffer both use the format field in addition to the
defaults.c code.
4. Changing the name AVFilterPic to AVFilterBuffer wouldn't be a big
deal because no filter uses these and only a few lines of framework code
in avfilter.c and defualts.c would need changing to work with the new name.
Please let me know what can be done. It looks like a common buffer
struct with changes 2 and 4 should be a small patch that won't break any
existing video filters.
Regards,
Hemanth
More information about the ffmpeg-devel
mailing list