[FFmpeg-devel] [PATCH] vf_tinterlace: add vertical low-pass-filter option to mode 4 and 5
Stefano Sabatini
stefasab at gmail.com
Sat Dec 29 10:52:07 CET 2012
On date Friday 2012-12-28 00:11:52 +0000, Mark Himsley encoded:
> On 25/12/2012 10:52, Stefano Sabatini wrote:
> > On date Saturday 2012-12-22 23:25:19 +0000, Mark Himsley encoded:
> >> On 22/12/2012 12:10, Stefano Sabatini wrote:
> >>> On date Friday 2012-12-21 12:58:07 +0000, Mark Himsley encoded:
> >>>> On 19/12/2012 17:17, Mark Himsley wrote:
> >>>>> Low-pass filtering is required when creating an interlaced destination
> >>>>> from a progressive source which contains high-frequency vertical detail.
> >>>>> Filtering will reduce interlace 'twitter' and Moire patterning.
> >>>>
> >>>> minor update to correct the consts.
> >>>
> >>> Sorry for the slow reply.
> >>
> >> I'm grateful for the review.
> >>
> >>>> + * Other than low-pass filtering, this functions identicaly to
> >>>> + * copy_picture_field() above.
> >>>
> >>> Note: would it make sense to move this to a separate filter?
> >>
> >> I don't think so. I can't currently think of a use-case.
> >> Were this filter to be separate from tinterlace, to insert just before
> >> the tinterlace filter, then it would have to do twice as many
> >> calculations - it would calculate all the lines that were about to be
> >> dropped in the interleave interlace. I do not think that is a good idea.
> >>
> >> Updated patch attached.
> >>
> >
> > Wrong patch?
> >
>
> True. This is what I should have sent.
>
> --
> Mark
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 17e2af7..c2ead91 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -3923,9 +3923,25 @@ Perform various types of temporal field interlacing.
> Frames are counted starting from 1, so the first input frame is
> considered odd.
>
> -This filter accepts a single option @option{mode} specifying the mode,
> -which can be specified either by specyfing @code{mode=VALUE} either
> -specifying the value alone. Available values are:
> +This filter accepts up to two options in the form of @var{key}=@var{value}
remove "up to two" in case we add more options
> +pairs separated by ":".
> +Alternatively, the @var{mode} option can be specified as a value alone,
> +optionally followed by a ":" and a @var{key}=@var{value} pair.
> +
> +A description of the accepted options follows.
> +
> + at table @option
> +
> + at item mode
> +Specifies the mode of the interlacing. This option can also be
> +specified as a value alone. See below for a list of values for this option.
> +
> + at item flags
> +Specifies flags influencing the filter process.
> +
> + at end table
> +
> +Available values for @var{mode} are:
>
> @table @samp
> @item merge, 0
> @@ -3965,6 +3981,21 @@ compatibility reasons.
>
> Default mode is @code{merge}.
>
> +Available value for @var{flags} is:
> +
> + at table @samp
> + at item low_pass_filter
> +Enable vertical low-pass filtering in the filter.
> +Vertical low-pass filtering is required when creating an interlaced
> +destination from a progressive source which contains high-frequency
> +vertical detail. Filtering will reduce interlace 'twitter' and Moir@'e
> +patterning.
> +
> +Vertical low-pass filtering can only be enabled for @var{mode}
> + at code{interleave_top} and @code{interleave_bottom}.
> +
> + at end table
Better to put the possible values inside the @item description.
> +
> @section transpose
>
> Transpose rows with columns in the input video and optionally flip it.
> diff --git a/libavfilter/vf_tinterlace.c b/libavfilter/vf_tinterlace.c
> index 843a66c..8926f77 100644
> --- a/libavfilter/vf_tinterlace.c
> +++ b/libavfilter/vf_tinterlace.c
> @@ -45,6 +45,7 @@ enum TInterlaceMode {
> typedef struct {
> const AVClass *class;
> enum TInterlaceMode mode; ///< interlace mode selected
> + int flags; ///< flags for the tinterlace filter
> int frame; ///< number of the output frame
> int vsub; ///< chroma vertical subsampling
> AVFilterBufferRef *cur;
> @@ -55,6 +56,7 @@ typedef struct {
>
> #define OFFSET(x) offsetof(TInterlaceContext, x)
> #define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +#define TINTERLACE_FLAG_VLPF 01
>
> static const AVOption tinterlace_options[] = {
> {"mode", "select interlace mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=MODE_MERGE}, 0, MODE_NB-1, FLAGS, "mode"},
> @@ -65,6 +67,8 @@ static const AVOption tinterlace_options[] = {
> {"interleave_top", "interleave top and bottom fields", 0, AV_OPT_TYPE_CONST, {.i64=MODE_INTERLEAVE_TOP}, INT_MIN, INT_MAX, FLAGS, "mode"},
> {"interleave_bottom", "interleave bottom and top fields", 0, AV_OPT_TYPE_CONST, {.i64=MODE_INTERLEAVE_BOTTOM}, INT_MIN, INT_MAX, FLAGS, "mode"},
> {"interlacex2", "interlace fields from two consecutive frames", 0, AV_OPT_TYPE_CONST, {.i64=MODE_INTERLACEX2}, INT_MIN, INT_MAX, FLAGS, "mode"},
> + {"flags", "set flags", OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = 0}, 0, INT_MAX, 0, "flags" },
> + {"low_pass_filter", "enable vertical low-pass filter", 0, AV_OPT_TYPE_CONST, {.i64 = TINTERLACE_FLAG_VLPF}, INT_MIN, INT_MAX, FLAGS, "flags" },
>
> {NULL}
> };
> @@ -142,8 +146,15 @@ static int config_out_props(AVFilterLink *outlink)
> tinterlace->black_linesize[i] * h);
> }
> }
> - av_log(ctx, AV_LOG_VERBOSE, "mode:%d h:%d -> h:%d\n",
> - tinterlace->mode, inlink->h, outlink->h);
> + if ((tinterlace->flags & TINTERLACE_FLAG_VLPF)
> + && !(tinterlace->mode == MODE_INTERLEAVE_TOP
> + || tinterlace->mode == MODE_INTERLEAVE_BOTTOM)) {
> + av_log(ctx, AV_LOG_WARNING, "filter not required for mode:%d\n", tinterlace->mode);
Nit: "low_pass_filter flag ignored with mode %d\n"
> + tinterlace->flags &= !TINTERLACE_FLAG_VLPF;
> + }
> + av_log(ctx, AV_LOG_VERBOSE, "mode:%d filter:%s h:%d -> h:%d\n",
> + tinterlace->mode, (tinterlace->flags & TINTERLACE_FLAG_VLPF) ? "on" : "off",
> + inlink->h, outlink->h);
>
> return 0;
> }
> @@ -159,12 +170,14 @@ static int config_out_props(AVFilterLink *outlink)
> * @param interleave leave a padding line between each copied line
> * @param dst_field copy to upper or lower field,
> * only meaningful when interleave is selected
> + * @param flags context flags
> */
> static inline
> void copy_picture_field(uint8_t *dst[4], int dst_linesize[4],
> const uint8_t *src[4], int src_linesize[4],
> enum AVPixelFormat format, int w, int src_h,
> - int src_field, int interleave, int dst_field)
> + int src_field, int interleave, int dst_field,
> + int flags)
> {
> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(format);
> int plane, vsub = desc->log2_chroma_h;
> @@ -184,8 +197,30 @@ void copy_picture_field(uint8_t *dst[4], int dst_linesize[4],
> srcp += src_linesize[plane];
> if (interleave && dst_field == FIELD_LOWER)
> dstp += dst_linesize[plane];
> - av_image_copy_plane(dstp, dst_linesize[plane] * (interleave ? 2 : 1),
> + if (flags & TINTERLACE_FLAG_VLPF) {
> + // Low-pass filtering is required when creating an interlaced destination from
> + // a progressive source which contains high-frequency vertical detail.
> + // Filtering will reduce interlace 'twitter' and Moire patterning.
> + int srcp_linesize = src_linesize[plane] * k;
> + int dstp_linesize = dst_linesize[plane] * (interleave ? 2 : 1);
> + for (int h = lines; h > 0; h--) {
> + const uint8_t *srcp_above = srcp - src_linesize[plane];
> + const uint8_t *srcp_below = srcp + src_linesize[plane];
> + if (h == lines) srcp_above = srcp; // there is no line above
> + if (h == 1) srcp_below = srcp; // there is no line below
> + for (int i = 0; i < linesize; i++) {
> + // this calculation is an integer representation of
> + // '0.5 * current + 0.25 * above + 0.25 + below'
> + // '1 +' is for rounding. */
> + dstp[i] = (1 + srcp[i] + srcp[i] + srcp_above[i] + srcp_below[i]) >> 2;
> + }
> + dstp += dstp_linesize;
> + srcp += srcp_linesize;
> + }
> + } else {
> + av_image_copy_plane(dstp, dst_linesize[plane] * (interleave ? 2 : 1),
> srcp, src_linesize[plane]*k, linesize, lines);
> + }
> }
> }
LGTM otherwise, thanks.
--
FFmpeg = Fabulous and Fast Murdering Patchable Erudite Game
More information about the ffmpeg-devel
mailing list