[FFmpeg-devel] [PATCH 1/5] lavfi: remplace passthrough_filter_frame with a flag.
Juanito Jusay
jusay16 at yahoo.com
Sat May 11 17:01:11 CEST 2013
M
.k' ,
Sent from my iPod
On May 11, 2013, at 10:50 PM, Stefano Sabatini <stefasab at gmail.com> wrote:
> On date Saturday 2013-05-11 15:19:06 +0200, Stefano Sabatini encoded:
>> On date Friday 2013-05-10 17:49:46 +0200, Clément Bœsch encoded:
>>> On Thu, May 09, 2013 at 01:53:23AM +0200, Clément Bœsch wrote:
>>>> With the introduction of AVFilterContext->is_disabled, we can simplify
>>>> the custom passthrough mode in filters.
>>>> ---
>>>> libavfilter/af_apad.c | 3 +--
>>>> libavfilter/avfilter.c | 6 +++---
>>>> libavfilter/avfilter.h | 19 ++++++-------------
>>>> libavfilter/version.h | 4 ++--
>>>> libavfilter/vf_overlay.c | 25 ++++---------------------
>>>> 5 files changed, 16 insertions(+), 41 deletions(-)
>>>>
>>>
>>> New version attached.
>>>
>>> --
>>> Clément B.
>>
>>> From b0c4e23be426386a2fb6a1a974a5fff751c58e2d Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
>>> Date: Thu, 9 May 2013 01:04:41 +0200
>>> Subject: [PATCH 1/5] lavfi: replace passthrough_filter_frame with a flag.
>>>
>>> With the introduction of AVFilterContext->is_disabled, we can simplify
>>> the custom passthrough mode in filters.
>>>
>>> This commit is technically a compat break, but the timeline was
>>> introduced very recently.
>>> ---
>>> libavfilter/af_apad.c | 3 +--
>>> libavfilter/af_volume.c | 2 +-
>>> libavfilter/avfilter.c | 6 +++---
>>> libavfilter/avfilter.h | 26 ++++++++++++--------------
>>> libavfilter/version.h | 4 ++--
>>> libavfilter/vf_boxblur.c | 2 +-
>>> libavfilter/vf_colorbalance.c | 2 +-
>>> libavfilter/vf_colorchannelmixer.c | 2 +-
>>> libavfilter/vf_colormatrix.c | 2 +-
>>> libavfilter/vf_cropdetect.c | 2 +-
>>> libavfilter/vf_curves.c | 2 +-
>>> libavfilter/vf_delogo.c | 2 +-
>>> libavfilter/vf_drawbox.c | 2 +-
>>> libavfilter/vf_edgedetect.c | 2 +-
>>> libavfilter/vf_gradfun.c | 2 +-
>>> libavfilter/vf_histeq.c | 2 +-
>>> libavfilter/vf_hue.c | 2 +-
>>> libavfilter/vf_lut.c | 2 +-
>>> libavfilter/vf_noise.c | 2 +-
>>> libavfilter/vf_overlay.c | 25 ++++---------------------
>>> libavfilter/vf_pp.c | 2 +-
>>> libavfilter/vf_removelogo.c | 2 +-
>>> libavfilter/vf_smartblur.c | 2 +-
>>> libavfilter/vf_unsharp.c | 2 +-
>>> 24 files changed, 41 insertions(+), 61 deletions(-)
>>>
>>> diff --git a/libavfilter/af_apad.c b/libavfilter/af_apad.c
>>> index 265b76a..c863792 100644
>>> --- a/libavfilter/af_apad.c
>>> +++ b/libavfilter/af_apad.c
>>> @@ -131,7 +131,6 @@ static const AVFilterPad apad_inputs[] = {
>>> .name = "default",
>>> .type = AVMEDIA_TYPE_AUDIO,
>>> .filter_frame = filter_frame,
>>> - .passthrough_filter_frame = filter_frame,
>>> },
>>> { NULL },
>>> };
>>> @@ -153,5 +152,5 @@ AVFilter avfilter_af_apad = {
>>> .inputs = apad_inputs,
>>> .outputs = apad_outputs,
>>> .priv_class = &apad_class,
>>> - .flags = AVFILTER_FLAG_SUPPORT_TIMELINE,
>>> + .flags = AVFILTER_FLAG_INTERNAL_TIMELINE,
>>> };
>>> diff --git a/libavfilter/af_volume.c b/libavfilter/af_volume.c
>>> index b7aec8d..79054af 100644
>>> --- a/libavfilter/af_volume.c
>>> +++ b/libavfilter/af_volume.c
>>> @@ -296,5 +296,5 @@ AVFilter avfilter_af_volume = {
>>> .init = init,
>>> .inputs = avfilter_af_volume_inputs,
>>> .outputs = avfilter_af_volume_outputs,
>>> - .flags = AVFILTER_FLAG_SUPPORT_TIMELINE,
>>> + .flags = AVFILTER_FLAG_GENERIC_TIMELINE,
>>> };
>>> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
>>> index 3f94dde..6676a58 100644
>>> --- a/libavfilter/avfilter.c
>>> +++ b/libavfilter/avfilter.c
>>> @@ -995,9 +995,9 @@ static int ff_filter_frame_framed(AVFilterLink *link, AVFrame *frame)
>>> dstctx->var_values[VAR_POS] = pos == -1 ? NAN : pos;
>>>
>>> dstctx->is_disabled = !av_expr_eval(dstctx->enable, dstctx->var_values, NULL);
>>> - if (dstctx->is_disabled)
>>> - filter_frame = dst->passthrough_filter_frame ? dst->passthrough_filter_frame
>>> - : default_filter_frame;
>>> + if (dstctx->is_disabled &&
>>> + (dstctx->filter->flags & AVFILTER_FLAG_GENERIC_TIMELINE))
>>> + filter_frame = default_filter_frame;
>>> }
>>> ret = filter_frame(link, out);
>>> link->frame_count++;
>>> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
>>> index a79c86c..8695a36 100644
>>> --- a/libavfilter/avfilter.h
>>> +++ b/libavfilter/avfilter.h
>>> @@ -385,19 +385,6 @@ struct AVFilterPad {
>>> int needs_fifo;
>>>
>>> int needs_writable;
>>> -
>>> - /**
>>> - * Passthrough filtering callback.
>>> - *
>>> - * If a filter supports timeline editing (in case
>>> - * AVFILTER_FLAG_SUPPORT_TIMELINE is enabled) then it can implement a
>>> - * custom passthrough callback to update its local context (for example to
>>> - * keep a frame reference, or simply send the filter to a custom outlink).
>>> - * The filter must not do any change to the frame in this callback.
>>> - *
>>> - * Input pads only.
>>> - */
>>> - int (*passthrough_filter_frame)(AVFilterLink *link, AVFrame *frame);
>>> };
>>> #endif
>>>
>>> @@ -446,7 +433,18 @@ enum AVMediaType avfilter_pad_get_type(const AVFilterPad *pads, int pad_idx);
>>> * to enable or disable a filter in the timeline. Filters supporting this
>>> * option have this flag set.
>>> */
>>> -#define AVFILTER_FLAG_SUPPORT_TIMELINE (1 << 16)
>>
>>> +#define AVFILTER_FLAG_GENERIC_TIMELINE (1 << 16)
>>> +/**
>>
>>> + * Same as AVFILTER_FLAG_GENERIC_TIMELINE, except that the filter will have its
>>> + * inpad->filter_frame() callback(s) naturally called (and will use
>>> + * AVFilterContext->is_disabled internally).
>>
>> "naturally" ... "internally" ... "generic" are pretty vague terms and
>> the result is confusing. Can you specify the meaning of "generic" and
>> "internal" in a more precise way (and possibly to find some less vague
>> terms may help).
>
> This is my attempt. I just replaced "generic" with "default" since
> default_filter_frame() is used, "internal" may be replaced by "auto"
> or "automatic" but I'm not really satisfied with that as well, since
> the user may guess that this is "automatically" executed by the
> framework (rather than by the filter code).
>
> Other alternatives to "generic":
> external framework default
>
> Other alternatives to "internal":
> callback
>
>
> /**
> * Some filters support a generic "enable" expression option that can be used
> * to enable or disable a filter in the timeline. Filters supporting this
> * option have this flag set. When the enable expression is false, the
> * default no-op filter_frame() function is called in place of the
> * filter_frame() callback defined on each input pad, thus the frame
> * is passed unchanged to the next filters.
> */
> #define AVFILTER_FLAG_SUPPORT_TIMELINE_DEFAULT (1 << 16)
>
> /**
> * Same as AVFILTER_FLAG_SUPPORT_TIMELINE_DEFAULT, except that the filter will have its
> * filter_frame() callback(s) called as usual even when the enable
> * expression is false. The filter will disable filtering
> * within the filter_frame() callback(s) itself, for example executing
> * code depending on the AVFilterContext->is_disabled value.
> */
> #define AVFILTER_FLAG_SUPPORT_TIMELINE_INTERNAL (1 << 17)
>
> Note that at this point I'm not against keeping the original terms
> "generic" and "internal", but even in this case I suggest to specify
> the "variant" after "timeline", as in
> AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC.
> --
> FFmpeg = Forgiving & Fiendish Muttering Power Elected Governor
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list