[FFmpeg-devel] added reitnerlace filter

---
 doc/filters.texi             |  87 ++++++-
 libavfilter/Makefile         |   2 +
 libavfilter/allfilters.c     |   2 +
 libavfilter/reinterlace.h    | 130 ++++++++++
 libavfilter/vf_reinterlace.c | 597 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 812 insertions(+), 6 deletions(-)
 create mode 100644 libavfilter/reinterlace.h
 create mode 100644 libavfilter/vf_reinterlace.c

Hi,

On 12/27/17, Vasile Toncu <vasile.toncu@tremend.com> wrote:
> ---
>  doc/filters.texi             |  87 ++++++-
>  libavfilter/Makefile         |   2 +
>  libavfilter/allfilters.c     |   2 +
>  libavfilter/reinterlace.h    | 130 ++++++++++
>  libavfilter/vf_reinterlace.c | 597
> +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 812 insertions(+), 6 deletions(-)
>  create mode 100644 libavfilter/reinterlace.h
>  create mode 100644 libavfilter/vf_reinterlace.c
>

Its not obvious from code and documentation what this does.

Why is this needed? At least provide some scenario where this is useful.

Hello,

The reinterlace filter does various interlace/interleave/merge operations
between consecutive frames of a video. It tries too behave just like
tinterlace filter, including some new modes.

On Wed, Dec 27, 2017 at 2:50 PM, Paul B Mahol <onemda@gmail.com> wrote:

> Hi,
>
> On 12/27/17, Vasile Toncu <vasile.toncu@tremend.com> wrote:
> > ---
> >  doc/filters.texi             |  87 ++++++-
> >  libavfilter/Makefile         |   2 +
> >  libavfilter/allfilters.c     |   2 +
> >  libavfilter/reinterlace.h    | 130 ++++++++++
> >  libavfilter/vf_reinterlace.c | 597
> > +++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 812 insertions(+), 6 deletions(-)
> >  create mode 100644 libavfilter/reinterlace.h
> >  create mode 100644 libavfilter/vf_reinterlace.c
> >
>
> Its not obvious from code and documentation what this does.
>
> Why is this needed? At least provide some scenario where this is useful.
>

Thanks for the patch. Initial comments below.

Vasile Toncu (2017-12-27):
> ---
>  doc/filters.texi             |  87 ++++++-
>  libavfilter/Makefile         |   2 +
>  libavfilter/allfilters.c     |   2 +
>  libavfilter/reinterlace.h    | 130 ++++++++++
>  libavfilter/vf_reinterlace.c | 597 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 812 insertions(+), 6 deletions(-)
>  create mode 100644 libavfilter/reinterlace.h
>  create mode 100644 libavfilter/vf_reinterlace.c
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 68f54f1..370be9b 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -564,13 +564,13 @@ select RIAA.
>  @item cd
>  select Compact Disc (CD).
>  @item 50fm

> -select 50µs (FM).
> +select 50µs (FM).

Your editor seems to have broken UTF-8 text. Same at a few places below.

>  @item 75fm
> -select 75µs (FM).
> +select 75µs (FM).
>  @item 50kf
> -select 50µs (FM-KF).
> +select 50µs (FM-KF).
>  @item 75kf
> -select 75µs (FM-KF).
> +select 75µs (FM-KF).
>  @end table
>  @end table
>  
> @@ -7346,7 +7346,7 @@ If not set, the filter will use the QP from the video stream (if available).
>  @item strength
>  Set filter strength. It accepts an integer in range -15 to 32. Lower values mean
>  more details but also more artifacts, while higher values make the image smoother
> -but also blurrier. Default value is @code{0} − PSNR optimal.
> +but also blurrier. Default value is @code{0} − PSNR optimal.
>  
>  @item use_bframe_qp
>  Enable the use of the QP from the B-Frames if set to @code{1}. Using this
> @@ -13191,6 +13191,81 @@ pixel format "yuv422p" @var{hsub} is 2 and @var{vsub} is 1.
>  @table @option
>  @end table
>  
> +@section reinterlace
> +Reinterlace filter does various interlace operations with the frames of a video.
> +
> +@table @option
> +
> +@item mode
> +The mode of the filter
> +
> +The permitted values for @var{mode} are:
> +
> +@table @samp
> +@item merge, 0
> +Merges lines of two consecutive frames. Skips even frames. The output has half frame rate of the input.
> +
> +@item drop_even, 1
> +Drops even frames. The output has half frame rate of the input.
> +
> +@item drop_odd, 2
> +Drop odd frames. The output has half frame rate of the input.
> +
> +@item pad, 3
> +Merges all the frames with a black frame. The output has the same frame rate as as the input.
> +
> +
> +@item interleave_top, 4
> +Interleaves lines of two consecutive frames. Skips even frames. The output has half frame rate of the input.
> +
> +@item interleave_bottom, 5
> +Interleaves lines of two consecutive frames. Skips even frames. The output has half frame rate of the input.
> +
> +@item interlacex2, 6
> +For every frames in the input frame adds another one which is obtaining by the interlace of two consecutive frames. 
> +The output has double frame rate of the input.
> +
> +@item mergex2, 7
> +Merge every frame with the next frame. The output has the same frame rate as as the input.
> +
> +@item merge_tff, 8
> +Merges the frames of the input considering also the parity and the top_filed_first information of the frames.
> +
> +The rules for the @var{merge_tff} are the folowng: 
> +
> +    1. ensure the odd frame metadata indicates a top field, @*
> +    2. ensure the even frame metadata indicates a bottom field, @*
> +    3. move the odd frame into the upper field of the new image, @*
> +    4. move the even frame into the lower field of the new image, @*
> +    5. if frames are out of order (bottom field then top field), drop the first field @*
> +    6. if frames are duplicates (top field then top field), drop the first field @*
> +    7. if frames don't have interlace metadata, merge as if they were in the right order @*
> +
> +
> +@item merge_bff, 9
> +Merges the frames of the input considering also the parity and the top_filed_first information of the frames.
> +
> +The rules for the @var{merge_bff} are similar with those for @var{merge_tff}, albeit inverted appropriately.
> +
> +@end table
> +
> +Default mode is @code{merge, 0}.
> +
> +@item flags
> +One can add various flags to the reitnerlace filter.
> +
> +The permitted values for @var{flags} are:
> +
> +@table @option
> +@item low_pass_filter, 1
> +Before copying a line of a frame, it gots filtered using a simple low pass filter with the upper and lowwer frame lines.
> +
> +Vertical low-pass filtering can only be enabled for @option{mode}
> +@var{interleave_top} and @var{interleave_bottom}.
> +
> +@end table
> +@end table
> +
>  @c man end VIDEO FILTERS
>  
>  @chapter Video Sources
> @@ -15897,4 +15972,4 @@ movie=dvd.vob:s=v:0+#0x81 [video] [audio]
>  @end example
>  @end itemize
>  
> -@c man end MULTIMEDIA SOURCES

> +@c man end MULTIMEDIA SOURCES
> \ No newline at end of file

This looks spurious.

> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index 8916588..606dfe0 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -286,6 +286,8 @@ OBJS-$(CONFIG_TESTSRC2_FILTER)               += vsrc_testsrc.o
>  
>  OBJS-$(CONFIG_NULLSINK_FILTER)               += vsink_nullsink.o
>  
> +OBJS-$(CONFIG_REINTERLACE_FILTER)            += vf_reinterlace.o
> +
>  # multimedia filters
>  OBJS-$(CONFIG_ADRAWGRAPH_FILTER)             += f_drawgraph.o
>  OBJS-$(CONFIG_AHISTOGRAM_FILTER)             += avf_ahistogram.o
> diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> index fa7d304..fa3c3d1 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -306,6 +306,8 @@ void avfilter_register_all(void)
>  

>      REGISTER_FILTER(NULLSINK,       nullsink,       vsink);
>  
> +    REGISTER_FILTER(REINTERLACE,    reinterlace,    vf);
> +

Move it to the alphabetical place within video filters.

>      /* multimedia filters */
>      REGISTER_FILTER(ADRAWGRAPH,     adrawgraph,     avf);
>      REGISTER_FILTER(AHISTOGRAM,     ahistogram,     avf);
> diff --git a/libavfilter/reinterlace.h b/libavfilter/reinterlace.h
> new file mode 100644
> index 0000000..924f347
> --- /dev/null

> +++ b/libavfilter/reinterlace.h

This header is included only once. If there is no good reason to make it
a separate header, put the declarations in the source files.

> @@ -0,0 +1,130 @@
> +/*
> + * Copyright (c) 2017 Vasile Toncu <u pkh me>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Reinterlace filter 
> + *
> + * @author Vasile Toncu  ( toncu.vasile gmail com )
> + *
> + * @see https://en.wikipedia.org/wiki/Interlaced_video
> + */
> +
> +#include <stdint.h>
> +

> +#include "libavutil/avassert.h"
> +#include "libavutil/opt.h"
> +#include "libavutil/pixdesc.h"
> +#include "libavutil/imgutils.h"

We usually try to keep includes in alphabetical order.

> +#include "avfilter.h"
> +#include "formats.h"
> +#include "internal.h"
> +#include "video.h"
> +
> +enum FilterMode {
> +    MODE_MERGE,
> +    MODE_DROP_EVEN,
> +    MODE_DROP_ODD,
> +    MODE_PAD,
> +    MODE_INTERLEAVE_TOP,
> +    MODE_INTERLEAVE_BOTTOM,
> +    MODE_INTERLACE_X2,
> +    MODE_MERGE_X2,
> +    MODE_MERGE_TFF,
> +    MODE_MERGE_BFF,
> +    MODE_NB
> +};
> +
> +enum FilterFlags {
> +    FLAG_NOTHING    = 0x00,
> +    FLAG_VLPF       = 0x01,
> +    FLAG_NB
> +};
> +
> +
> +typedef struct {
> +    const AVClass *class;
> +    int mode;
> +    int flags;
> +
> +    AVFrame *prev_frame, *current_frame;

> +    int current_frame_index;

If this is supposed to count the number of frames in a whole stream,
then make it (u)int64_t.

> +
> +    uint8_t *black_vec[4];
> +
> +    int skip_next_frame;
> +

> +    void *thread_data;

Use the correct type.

> +
> +} ReInterlaceContext;
> +
> +#define OFFSET(x) offsetof(ReInterlaceContext, x)
> +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
> +

> +static const AVOption reinterlace_options[] = {

Declaring a static object in a header seems like a bad idea.

> +    { "mode", "set mode", OFFSET(mode), AV_OPT_TYPE_INT, {.i64=MODE_MERGE},     0, MODE_NB - 1, FLAGS, "mode" },

> +    { "merge",             "MODE_MERGE",                0, AV_OPT_TYPE_CONST, {.i64=MODE_MERGE},                INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "drop_even",         "MODE_DROP_EVEN",            0, AV_OPT_TYPE_CONST, {.i64=MODE_DROP_EVEN},            INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "drop_odd",          "MODE_DROP_ODD",             0, AV_OPT_TYPE_CONST, {.i64=MODE_DROP_ODD},             INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "pad",               "MODE_PAD",                  0, AV_OPT_TYPE_CONST, {.i64=MODE_PAD},                  INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "interleave_top",    "MODE_INTERLEAVE_TOP",       0, AV_OPT_TYPE_CONST, {.i64=MODE_INTERLEAVE_TOP},       INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "interleave_bottom", "MODE_INTERLEAVE_BOTTOM",    0, AV_OPT_TYPE_CONST, {.i64=MODE_INTERLEAVE_BOTTOM},    INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "interlacex2",       "MODE_INTERLACE_X2",         0, AV_OPT_TYPE_CONST, {.i64=MODE_INTERLACE_X2},         INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "mergex2",           "MODE_MERGE_X2",             0, AV_OPT_TYPE_CONST, {.i64=MODE_MERGE_X2},             INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "merge_tff",         "MODE_MERGE_TFF",            0, AV_OPT_TYPE_CONST, {.i64=MODE_MERGE_TFF},            INT_MIN, INT_MAX, FLAGS, "mode"},
> +    { "merge_bff",         "MODE_MERGE_BFF",            0, AV_OPT_TYPE_CONST, {.i64=MODE_MERGE_BFF},            INT_MIN, INT_MAX, FLAGS, "mode"},

The documentation field is not very user friendly. Same below.

> +
> +    { "flags", "add flag for reinterlace", OFFSET(flags), AV_OPT_TYPE_INT, {.i64=FLAG_NOTHING}, 0, 0xFF, FLAGS, "flags" },
> +    { "low_pass_filter",   "FLAG_VLPF",                 0, AV_OPT_TYPE_CONST, {.i64 = FLAG_VLPF}, INT_MIN, INT_MAX, FLAGS, "flags"},
> +    { "vlpf",              "FLAG_VLPF",                 0, AV_OPT_TYPE_CONST, {.i64 = FLAG_VLPF}, INT_MIN, INT_MAX, FLAGS, "flags"},
> +    { NULL }
> +};
> +
> +AVFILTER_DEFINE_CLASS(reinterlace);
> +
> +#define IS_ODD(value) (value & 1)
> +
> +typedef struct ReInterlaceThreadData {
> +    AVFrame *out, *first, *second;
> +    int plane;
> +    ReInterlaceContext *reinterlace;
> +
> +    int scale_w_plane12_factor;
> +    int scale_h_plane12_factor;
> +
> +} ReInterlaceThreadData;
> +

> +#define PIXEL_FORMATS AV_PIX_FMT_YUV410P, AV_PIX_FMT_YUV411P,   \

Used only once, probably better inlined.

> +        AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUV422P,                 \
> +        AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV411P,                 \
> +        AV_PIX_FMT_YUV440P, AV_PIX_FMT_YUVJ422P,                \
> +        AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ420P,               \
> +        AV_PIX_FMT_YUVJ411P, AV_PIX_FMT_YUVJ440P,               \
> +        AV_PIX_FMT_YUV444P9, AV_PIX_FMT_YUV422P9,               \
> +        AV_PIX_FMT_YUV420P9, AV_PIX_FMT_YUV444P10,              \
> +        AV_PIX_FMT_YUV422P10, AV_PIX_FMT_YUV420P10,             \
> +        AV_PIX_FMT_YUV440P10, AV_PIX_FMT_YUV444P12,             \
> +        AV_PIX_FMT_YUV422P12, AV_PIX_FMT_YUV420P12,             \
> +        AV_PIX_FMT_YUV440P12, AV_PIX_FMT_YUV444P14,             \
> +        AV_PIX_FMT_YUV422P14, AV_PIX_FMT_YUV420P14,             \
> +        AV_PIX_FMT_YUV444P16, AV_PIX_FMT_YUV422P16,             \
> +        AV_PIX_FMT_YUV420P16, AV_PIX_FMT_GRAY8,                 \
> +        AV_PIX_FMT_NONE
> +
> diff --git a/libavfilter/vf_reinterlace.c b/libavfilter/vf_reinterlace.c
> new file mode 100644
> index 0000000..ee28593
> --- /dev/null
> +++ b/libavfilter/vf_reinterlace.c
> @@ -0,0 +1,597 @@
> +/*
> + * Copyright (c) 2017 Vasile Toncu <u pkh me>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Reinterlace filter 
> + *
> + * @author Vasile Toncu  ( toncu.vasile gmail com )
> + *
> + * @see https://en.wikipedia.org/wiki/Interlaced_video
> + */
> +
> +
> +#include "reinterlace.h"
> +
> +static av_cold int init(AVFilterContext *ctx)
> +{
> +    ReInterlaceContext *reinterlace = ctx->priv;
> +

> +    reinterlace->current_frame_index = 0;
> +    reinterlace->prev_frame = NULL;
> +    reinterlace->current_frame = NULL;

Not required.

> +

> +    for (int i = 0; i < 4; i++)

Our coding style still forbids declaring the loop variable here.

> +        reinterlace->black_vec[i] = NULL;
> +
> +    reinterlace->skip_next_frame = 0;

Not required either.

> +

> +    reinterlace->thread_data = (ReInterlaceThreadData *) malloc(4 * sizeof(ReInterlaceThreadData));

Get rid of that cast, and use a function in the av_malloc() family. Same
below.

> +
> +    return 0;
> +}
> +
> +static int query_formats(AVFilterContext *ctx)
> +{
> +    //const ReInterlaceContext *reinterlace = ctx->priv;
> +
> +    // TODO add more necesary formats
> +    static const enum AVPixelFormat all_pix_fmts[] = {
> +        PIXEL_FORMATS
> +    }; 
> +
> +    AVFilterFormats *fmts_list;
> +    const enum AVPixelFormat *pix_fmts = NULL;
> +

> +    pix_fmts = all_pix_fmts;

Why?

> +    fmts_list = ff_make_format_list(pix_fmts);
> +
> +    if (!fmts_list)
> +        return AVERROR(ENOMEM);
> +
> +    return ff_set_common_formats(ctx, fmts_list);
> +}
> +
> +static int config_out_props(AVFilterLink *outlink)
> +{
> +    AVFilterContext *ctx = outlink->src;
> +    AVFilterLink *inlink = outlink->src->inputs[0];
> +    ReInterlaceContext *reinterlace = ctx->priv;
> +    int r_mode = reinterlace->mode;
> +
> +    switch (r_mode) {
> +    case MODE_MERGE:
> +        outlink->h = 2 * inlink->h;
> +        outlink->time_base  = av_mul_q(inlink->time_base , (AVRational){2,1});
> +        outlink->frame_rate = av_mul_q(inlink->frame_rate, (AVRational){1,2});
> +        outlink->sample_aspect_ratio = av_mul_q(inlink->sample_aspect_ratio, av_make_q(2, 1));  
> +        break;
> +
> +    case MODE_PAD:
> +        outlink->h = 2 * inlink->h;
> +        outlink->sample_aspect_ratio = av_mul_q(inlink->sample_aspect_ratio, av_make_q(2, 1));
> +        outlink->frame_rate = inlink->frame_rate;
> +        outlink->time_base  = inlink->time_base;
> +        break;
> +
> +    case MODE_DROP_EVEN:
> +    case MODE_DROP_ODD:
> +    case MODE_INTERLEAVE_TOP:
> +    case MODE_INTERLEAVE_BOTTOM:
> +        outlink->frame_rate = av_mul_q(inlink->frame_rate, (AVRational){1,2});
> +        outlink->time_base  = av_mul_q(inlink->time_base , (AVRational){2,1});
> +        break;
> +
> +    case MODE_INTERLACE_X2:
> +        outlink->frame_rate = av_mul_q(inlink->frame_rate, (AVRational){2,1});
> +        outlink->time_base  = av_mul_q(inlink->time_base , (AVRational){1,2});
> +        break;
> +
> +    case MODE_MERGE_X2:
> +        outlink->h = 2 * inlink->h;
> +        outlink->sample_aspect_ratio = av_mul_q(inlink->sample_aspect_ratio, av_make_q(2, 1));
> +        outlink->frame_rate = inlink->frame_rate;
> +        outlink->time_base  = inlink->time_base;
> +        break;
> +
> +    case MODE_MERGE_BFF:
> +    case MODE_MERGE_TFF:
> +        outlink->h = 2 * inlink->h;
> +        outlink->sample_aspect_ratio = av_mul_q(inlink->sample_aspect_ratio, av_make_q(2, 1));
> +        outlink->frame_rate = av_mul_q(inlink->frame_rate, (AVRational){1,2});
> +        break;
> +
> +    default:

> +        av_assert0(0);

It could be reached if the user sets an invalid integer value for the
mode => no assert, proper error message and code.

> +
> +    }
> +
> +    if (reinterlace->flags & FLAG_VLPF)
> +        if (r_mode != MODE_INTERLEAVE_TOP && r_mode != MODE_INTERLEAVE_BOTTOM)
> +            reinterlace->flags &= ~FLAG_VLPF;
> +    
> +    return 0;
> +}
> +
> +static void copy_line_lowpass(uint8_t *to, ptrdiff_t width, uint8_t *from,
> +                           uint8_t *from_up, uint8_t *from_down)
> +{

> +    for (int i = 0; i < width; i++)

Use the same type as the bounds.

> +    //for (int i = width; i--; )

Cleanup.

> +        to[i] = (1 + from[i] + from[i] + from_up[i] + from_down[i]) >> 2;
> +}
> +
> +static int filter_frame_plane(AVFilterContext *ctx, void *arg, int jobnr, int nb_jobs)
> +{
> +    // jobnr is usualy plane number
> +    ReInterlaceThreadData *rtd = arg;
> +    ReInterlaceContext *reinterlace = rtd->reinterlace;
> +    AVFrame *first = rtd->first;
> +    AVFrame *second = rtd->second;
> +    AVFrame *out = rtd->out;
> +
> +    int plane = rtd->plane;
> +    int r_mode = reinterlace->mode;
> +
> +    int x = (1 == plane || 2 == plane) ? rtd->scale_w_plane12_factor : 1; 
> +    int y = (1 == plane || 2 == plane) ? rtd->scale_h_plane12_factor : 1;
> +    int ls_offset;
> +    int offset1, offset2, offset3, offset4;
> +
> +    switch (r_mode) {
> +    case MODE_MERGE:
> +        av_image_copy_plane(out->data[plane], 2 * out->linesize[plane], 
> +            first->data[plane], first->linesize[plane], first->width / x, first->height / y);  
> +        av_image_copy_plane(out->data[plane] + out->linesize[plane], 2 * out->linesize[plane], 
> +            second->data[plane], second->linesize[plane], second->width / x, second->height / y);
> +        break;
> +
> +    case MODE_PAD:
> +        ls_offset = (reinterlace->current_frame_index & 1) ? 0 : out->linesize[plane]; 
> +        av_image_copy_plane(out->data[plane] + ls_offset, 2 * out->linesize[plane], 
> +            second->data[plane], second->linesize[plane], second->width / x, second->height / y); 
> +        av_image_copy_plane(out->data[plane] + out->linesize[plane] - ls_offset, 2 * out->linesize[plane], 
> +            reinterlace->black_vec[plane], second->linesize[plane], second->width / x, second->height / y);
> +        break;
> +
> +    case MODE_INTERLEAVE_BOTTOM:
> +    case MODE_INTERLEAVE_TOP:
> +        y = y * 2;
> +  
> +        if (reinterlace->flags & FLAG_VLPF) {
> +
> +            int lines, cols;
> +            AVFrame *from_frame;
> +            uint8_t *from, *to;
> +            int from_step, to_step;
> +
> +            lines = (MODE_INTERLEAVE_TOP == r_mode) ? (2 * out->height / y + 1) / 2 : (2 * out->height / y + 0) / 2;
> +            cols = out->width / x;
> +            from_frame = first;
> +            from = from_frame->data[plane];
> +            to = out->data[plane];
> +
> +            if (MODE_INTERLEAVE_BOTTOM == r_mode) {
> +                from = from + from_frame->linesize[plane];
> +                to = to + out->linesize[plane];
> +            }
> +
> +            from_step = 2 * from_frame->linesize[plane];
> +            to_step = 2 * out->linesize[plane];
> +
> +            // when i = lines
> +            copy_line_lowpass(to, cols, from, from, from + from_frame->linesize[plane]);
> +            to += to_step;
> +            from += from_step;
> +
> +            for (int i = lines - 2; i; i--) {
> +                //from_up = (lines == i) ? from : from - from_frame->linesize[plane];
> +                //from_down = (1 == i) ? from : from + from_frame->linesize[plane];
> +                copy_line_lowpass(to, cols, from, from - from_frame->linesize[plane], from + from_frame->linesize[plane]);
> +                to += to_step;
> +                from += from_step;
> +            }
> +
> +            // when i == 1
> +            copy_line_lowpass(to, cols, from, from - from_frame->linesize[plane], from);
> +            to += to_step;
> +            from += from_step;
> +
> +            lines = (MODE_INTERLEAVE_BOTTOM == r_mode) ? ((2 * out->height / y) + 1) / 2 : (2 * out->height / y + 0) / 2;
> +            cols = out->width / x;
> +            from_frame = second;
> +            from = from_frame->data[plane];
> +            to = out->data[plane];
> +
> +            if (MODE_INTERLEAVE_TOP == r_mode) {
> +                from = from + from_frame->linesize[plane];
> +                to = to + out->linesize[plane];
> +            }
> +
> +            from_step = 2 * from_frame->linesize[plane];
> +            to_step = 2 * out->linesize[plane];
> +
> +            // when i = lines
> +            copy_line_lowpass(to, cols, from, from, from + from_frame->linesize[plane]);
> +            to += to_step;
> +            from += from_step;
> +
> +            for (int i = lines - 2; i; i--) {
> +                //from_up = (lines == i) ? from : from - from_frame->linesize[plane];
> +                //from_down = (1 == i) ? from : from + from_frame->linesize[plane];
> +                copy_line_lowpass(to, cols, from, from - from_frame->linesize[plane], from + from_frame->linesize[plane]);
> +                to += to_step;
> +                from += from_step;
> +            }
> +
> +            // when i == 1
> +            copy_line_lowpass(to, cols, from, from - from_frame->linesize[plane], from);
> +            to += to_step;
> +            from += from_step;
> +
> +        } else {
> +            offset1 = (MODE_INTERLEAVE_TOP == r_mode) ? 0 : out->linesize[plane];
> +            offset2 = (MODE_INTERLEAVE_TOP == r_mode) ? 0 : first->linesize[plane];
> +            offset3 = (MODE_INTERLEAVE_TOP == r_mode) ? out->linesize[plane]    : 0;
> +            offset4 = (MODE_INTERLEAVE_TOP == r_mode) ? second->linesize[plane] : 0;
> +
> +            av_image_copy_plane(out->data[plane] + offset1, 2 * out->linesize[plane], 
> +                first->data[plane] + offset2, 2 * first->linesize[plane], 
> +                first->width / x, first->height / y); 
> +            av_image_copy_plane(out->data[plane] + offset3, 2 * out->linesize[plane], 
> +                second->data[plane] + offset4, 2 * second->linesize[plane], 
> +                second->width / x, second->height / y);  
> +        }
> +        break;
> +
> +    case MODE_INTERLACE_X2:
> +        y = y * 2;
> +
> +        offset1 = 0; offset2 = 0;
> +        offset3 = out->linesize[plane];
> +        offset4 = second->linesize[plane];
> +
> +        if (second->interlaced_frame && second->top_field_first) {
> +            offset1 = out->linesize[plane];
> +            offset2 = first->linesize[plane];
> +            offset3 = 0; offset4 = 0;
> +        }
> +
> +        av_image_copy_plane(out->data[plane] + offset1, 2 * out->linesize[plane], 
> +            first->data[plane] + offset2, 2 * first->linesize[plane], 
> +            first->width / x, first->height / y);
> +        av_image_copy_plane(out->data[plane] + offset3, 2 * out->linesize[plane], 
> +            second->data[plane] + offset4, 2 * second->linesize[plane], 
> +            second->width / x, second->height / y);
> +        break;
> +
> +    case MODE_MERGE_X2:
> +        if ( IS_ODD(reinterlace->current_frame_index - 1) ) {
> +            av_image_copy_plane(out->data[plane], 2 * out->linesize[plane], 
> +                second->data[plane], second->linesize[plane], second->width / x, second->height / y);  
> +            av_image_copy_plane(out->data[plane] + out->linesize[plane], 2 * out->linesize[plane], 
> +                first->data[plane], first->linesize[plane], first->width / x, first->height / y);
> +        } else {
> +            av_image_copy_plane(out->data[plane], 2 * out->linesize[plane], 
> +                first->data[plane], first->linesize[plane], first->width / x, first->height / y);  
> +            av_image_copy_plane(out->data[plane] + out->linesize[plane], 2 * out->linesize[plane], 
> +                second->data[plane], second->linesize[plane], second->width / x, second->height / y);
> +        }
> +        break;
> +
> +    case MODE_MERGE_TFF:
> +    case MODE_MERGE_BFF:
> +        offset1 = (MODE_MERGE_TFF == r_mode) ? 0 : out->linesize[plane];
> +        offset2 = (MODE_MERGE_TFF == r_mode) ? out->linesize[plane] : 0;
> +
> +        av_image_copy_plane(out->data[plane] + offset1, 2 * out->linesize[plane], 
> +            first->data[plane], first->linesize[plane], first->width / x, first->height / y);  
> +        av_image_copy_plane(out->data[plane] + offset2, 2 * out->linesize[plane], 
> +            second->data[plane], second->linesize[plane], second->width / x, second->height / y);
> +        break;
> +
> +    default:
> +        break;
> +    }
> +
> +    return 0;
> +}
> +
> +static ReInterlaceThreadData *get_ReInterlaceThreadData(AVFrame *out, AVFrame *first, AVFrame *second, 
> +                int plane, ReInterlaceContext *reinterlace, 
> +                int scale_w_plane12_factor, 
> +                int scale_h_plane12_factor)
> +{
> +    ReInterlaceThreadData *rtd = &((ReInterlaceThreadData *)reinterlace->thread_data)[plane];
> +
> +    if (!rtd)
> +        return rtd;
> +
> +    rtd->out = out;
> +    rtd->first = first;
> +    rtd->second = second;
> +    rtd->plane = plane;
> +    rtd->reinterlace = reinterlace;
> +    rtd->scale_h_plane12_factor = scale_h_plane12_factor;
> +    rtd->scale_w_plane12_factor = scale_w_plane12_factor;
> +
> +    return rtd;
> +}
> +
> +/** 
> + * alocate memory for a black frame 
> + */
> +static int init_black_buffers(ReInterlaceContext *reinterlace, AVFrame *frame, int format)
> +{
> +    int black_vec_size = frame->width * frame->height * 3;

> +    int val0 = 16;
> +    int val128 = 128;

Why?

> +
> +    if (AV_PIX_FMT_YUVJ420P == format ||
> +        AV_PIX_FMT_YUVJ422P == format || 
> +        AV_PIX_FMT_YUVJ440P == format || 
> +        AV_PIX_FMT_YUVJ444P == format) {
> +
> +        val0 = 0;
> +    
> +    }
> +
> +    for (int i = 0; i < 4; i++) {
> +        reinterlace->black_vec[i] = (uint8_t *) malloc(black_vec_size);
> +

> +        if ( !reinterlace->black_vec[i] )

Unusual spacing. Same at a few ohther places.

> +            return AVERROR(ENOMEM);
> +
> +        memset(reinterlace->black_vec[i], (0 == i || 3 == i ? val0 : val128),  black_vec_size);
> +    }
> +
> +    return 0;
> +}
> +
> +static void copy_all_planes(AVFilterContext *ctx, 
> +    ReInterlaceContext *reinterlace, 
> +    const AVPixFmtDescriptor *desc, 
> +    AVFrame *out, AVFrame *first, AVFrame *second)
> +{
> +    int scale_w_plane12_factor = 1 << desc->log2_chroma_w;
> +    int scale_h_plane12_factor = 1 << desc->log2_chroma_h;
> +
> +    for (int plane = 0; plane < desc->nb_components; plane++) {
> +
> +        ReInterlaceThreadData *rtd = get_ReInterlaceThreadData(out, first, second, 
> +            plane, reinterlace, scale_w_plane12_factor, scale_h_plane12_factor);
> +
> +        ctx->internal->execute(ctx, filter_frame_plane, rtd, NULL, FFMIN(desc->nb_components, ctx->graph->nb_threads));
> +        //filter_frame_plane(ctx, rtd, plane, desc->nb_components);
> +    }
> +}
> +
> +        
> +
> +static int filter_frame(AVFilterLink *inlink, AVFrame *in)
> +{
> +    AVFilterContext *ctx            = inlink->dst;
> +    ReInterlaceContext *reinterlace = ctx->priv;
> +    AVFilterLink *outlink           = ctx->outputs[0];
> +    const AVPixFmtDescriptor *desc  = av_pix_fmt_desc_get(outlink->format);
> +    AVFrame *out, *first, *second;
> +    int ret;
> +
> +    int r_mode = reinterlace->mode;
> +
> +    av_frame_free(&(reinterlace->prev_frame));
> +    reinterlace->prev_frame = reinterlace->current_frame;
> +    reinterlace->current_frame = in;
> +    reinterlace->current_frame_index++;
> +
> +    // we process two frames at a time, thus only even frame indexes are considered
> +    if ( IS_ODD(reinterlace->current_frame_index) ) {
> +        if (MODE_PAD == r_mode || MODE_MERGE_X2 == r_mode 
> +            || MODE_INTERLACE_X2 == r_mode || MODE_MERGE_BFF == r_mode 
> +            || MODE_MERGE_TFF == r_mode) {
> +        } else {
> +            return 0;
> +        }
> +    }
> +
> +    if (1 == reinterlace->current_frame_index) {
> +        ret = init_black_buffers(reinterlace, in, outlink->format);
> +
> +        if (ret < 0)
> +           return ret; 
> +    }
> +
> +    first   = reinterlace->prev_frame;
> +    second  = reinterlace->current_frame;
> +
> +    switch (r_mode) {
> +    case MODE_DROP_EVEN:
> +    case MODE_DROP_ODD:
> +        out = (r_mode == MODE_DROP_ODD) ? reinterlace->current_frame : reinterlace->prev_frame;
> +        out = av_frame_clone(out);
> +
> +        if (!out)
> +            return AVERROR(ENOMEM);
> +
> +        out->pts = out->pts >> 1;
> +        ret = ff_filter_frame(outlink, out);
> +        break;
> +
> +    case MODE_MERGE:
> +    case MODE_MERGE_X2:
> +    case MODE_MERGE_TFF:
> +    case MODE_MERGE_BFF:
> +        if (MODE_MERGE_X2 == r_mode && 1 == reinterlace->current_frame_index)
> +            return 0;
> +
> +        if (MODE_MERGE_BFF == r_mode || MODE_MERGE_TFF == r_mode) {
> +           if (!first)
> +                return 0;
> +
> +            if (reinterlace->skip_next_frame) {
> +                reinterlace->skip_next_frame = 0;
> +                return 0;
> +            }
> +
> +            if (1 == first->interlaced_frame && 1 == second->interlaced_frame)
> +            {
> +                if (first->top_field_first == second->top_field_first)
> +                    return 0;
> +                else if (MODE_MERGE_BFF == reinterlace->mode && first->top_field_first != 0)
> +                    return 0;
> +                else if (MODE_MERGE_TFF == reinterlace->mode && first->top_field_first != 1)
> +                    return 0;
> +            } 
> +        }
> +        
> +        out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> +
> +        if (!out)
> +            return AVERROR(ENOMEM);
> +
> +        av_frame_copy_props(out, first);
> +        out->sample_aspect_ratio    = av_mul_q(first->sample_aspect_ratio, av_make_q(2, 1));
> +        out->interlaced_frame       = 1;
> +        out->top_field_first        = MODE_MERGE_BFF == r_mode ? 0 : 1;
> +        out->height                 = outlink->h;
> +
> +        if (MODE_MERGE == r_mode) 
> +            out->pts = out->pts >> 1;
> +
> +        copy_all_planes(ctx, reinterlace, desc, out, first, second);
> +
> +        if (MODE_MERGE_BFF == r_mode || MODE_MERGE_TFF == r_mode) 
> +            reinterlace->skip_next_frame = 1;
> +
> +        ret = ff_filter_frame(outlink, out);
> +        break;
> +
> +    case MODE_PAD:
> +        out = ff_get_video_buffer(outlink, outlink->w, outlink->h * 2);
> +
> +        if (!out)
> +            return AVERROR(ENOMEM);
> +
> +        av_frame_copy_props(out, second);
> +        out->sample_aspect_ratio = av_mul_q(second->sample_aspect_ratio, av_make_q(2, 1));
> +        out->height = outlink->h;
> +
> +        copy_all_planes(ctx, reinterlace, desc, out, first, second);
> +
> +        ret = ff_filter_frame(outlink, out);
> +        break;
> +
> +    case MODE_INTERLEAVE_BOTTOM:
> +    case MODE_INTERLEAVE_TOP:

> +        out  = ff_get_video_buffer(outlink, outlink->w, outlink->h);

Missing error check.

> +        av_frame_copy_props(out, first);
> +
> +        copy_all_planes(ctx, reinterlace, desc, out, first, second);
> +
> +        out->pts = out->pts >> 1;
> +        out->interlaced_frame = 1;
> +        out->top_field_first = (MODE_INTERLEAVE_TOP == r_mode) ? 1 : 0;
> +        ret = ff_filter_frame(outlink, out);
> +        break;
> +
> +    case MODE_INTERLACE_X2:
> +        if (1 == reinterlace->current_frame_index)
> +            return 0;
> +
> +        out = av_frame_clone(first);
> +
> +        if (!out)
> +            return AVERROR(ENOMEM);
> +
> +        // output first frame
> +        out->pts = (AV_NOPTS_VALUE != first->pts ) ? first->pts * 2 : AV_NOPTS_VALUE;
> +        out->interlaced_frame = 1;      
> +        ret = ff_filter_frame(outlink, out);
> +
> +        if (ret < 0)
> +            return ret;
> +
> +        // output the second frame interlaced with first frame
> +        out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> +
> +        if (!out)
> +            return AVERROR(ENOMEM);
> +
> +        av_frame_copy_props(out, second);
> +        out->interlaced_frame = 1;
> +        out->top_field_first = !out->top_field_first;
> +        out->pts = first->pts + second->pts;
> +        out->pts = (AV_NOPTS_VALUE == first->pts || AV_NOPTS_VALUE == second->pts) ? AV_NOPTS_VALUE : out->pts;
> +
> +        copy_all_planes(ctx, reinterlace, desc, out, first, second);
> +
> +        ret = ff_filter_frame(outlink, out);
> +        break;
> +
> +    default:
> +        av_assert0(0);
> +    }
> +
> +    return ret;
> +}
> +
> +static av_cold void uninit(AVFilterContext *ctx)
> +{
> +    ReInterlaceContext *reinterlace = ctx->priv;
> +    int i;
> +
> +    for (i = 0; i < 4; i++)
> +        if (reinterlace->black_vec[i])
> +            free (reinterlace->black_vec[i]);
> +
> +    free(reinterlace->thread_data);
> +
> +}
> +
> +static const AVFilterPad reinterlace_inputs[] = {
> +    {
> +        .name         = "default",
> +        .type         = AVMEDIA_TYPE_VIDEO,
> +        .filter_frame = filter_frame,
> +    },
> +    { NULL }
> +};
> +
> +static const AVFilterPad reinterlace_outputs[] = {
> +    {
> +        .name = "default",
> +        .type = AVMEDIA_TYPE_VIDEO,
> +        .config_props = config_out_props,
> +    },
> +    { NULL }
> +};
> +
> +AVFilter ff_vf_reinterlace = {
> +    .name          = "reinterlace",
> +    .description   = NULL_IF_CONFIG_SMALL("Various interlace frame manipulations"),
> +    .priv_size     = sizeof(ReInterlaceContext),
> +    .init          = init,
> +    .uninit        = uninit,
> +    .query_formats = query_formats,
> +    .inputs        = reinterlace_inputs,
> +    .outputs       = reinterlace_outputs,
> +    .priv_class    = &reinterlace_class,
> +    .flags         = AVFILTER_FLAG_SLICE_THREADS | AVFILTER_FLAG_SUPPORT_TIMELINE_GENERIC,
> +};

Regards,

Vasile Toncu (2017-12-27):
> The reinterlace filter does various interlace/interleave/merge operations
> between consecutive frames of a video. It tries too behave just like
> tinterlace filter, including some new modes.

Why not add the new modes to the tinterlace filter, then?

> On Wed, Dec 27, 2017 at 2:50 PM, Paul B Mahol <onemda@gmail.com> wrote:

Please remember that top-posting is forbidden on this mailing-list. If
you do not know what it means, look it up.

Regards,

> > The reinterlace filter does various interlace/interleave/merge operations
> > between consecutive frames of a video. It tries too behave just like
> > tinterlace filter, including some new modes.

Why not add the new modes to the tinterlace filter, then?
>

It is because of licencing issues. I wanted the new filter to be LGPL.

Vasile Toncu (2017-12-27):
> It is because of licencing issues. I wanted the new filter to be LGPL.

Thanks for the explanation. That is a valid reason.

But as is, it would result would be duplicated code for people who do
not worry about licensing.

Are there any features that tinterlace has and this new filter has not?
If not, then I think it would be better to just replace tinterlace
entirely.

I can answer myself: tinterlace has asm optimizations. This is not for
me only to decide, but I am rather against having duplicated features
just for licensing reasons.

In this particular instance, I think you could use the GPL asm
optimizations while having the filter itself LGPL.

Regards,

On 27.12.2017 15:31, Nicolas George wrote:
> Vasile Toncu (2017-12-27):
>> It is because of licencing issues. I wanted the new filter to be LGPL.
> Thanks for the explanation. That is a valid reason.
>
> But as is, it would result would be duplicated code for people who do
> not worry about licensing.
>
> Are there any features that tinterlace has and this new filter has not?
> If not, then I think it would be better to just replace tinterlace
> entirely.
The main difference between those those two filters is that reinterlace 
processes
planes of output frames in parallel.

Another difference is that reinterlace introduces two new modes, which 
are described at [1]
> I can answer myself: tinterlace has asm optimizations. This is not for
> me only to decide, but I am rather against having duplicated features
> just for licensing reasons.
> In this particular instance, I think you could use the GPL asm
> optimizations while having the filter itself LGPL.
Your arguments are valid, but what happens if I want to use a GPL 
tinterlace in an closed source application that will be distributed.

[1] http://ffmpeg.org/pipermail/ffmpeg-devel/2017-December/223066.html

Vasile Toncu (2017-12-28):
> >Are there any features that tinterlace has and this new filter has not?
> >If not, then I think it would be better to just replace tinterlace
> >entirely.
> The main difference between those those two filters is that reinterlace
> processes
> planes of output frames in parallel.
> 
> Another difference is that reinterlace introduces two new modes, which are
> described at [1]

You did not answer the question. I asked if there are features in
tinterlace that are not in reinterlace. You answered the opposite
question.

> >I can answer myself: tinterlace has asm optimizations. This is not for
> >me only to decide, but I am rather against having duplicated features
> >just for licensing reasons.
> >In this particular instance, I think you could use the GPL asm
> >optimizations while having the filter itself LGPL.
> Your arguments are valid, but what happens if I want to use a GPL tinterlace
> in an closed source application that will be distributed.

You cannot.

But making tinterlace LGPL would solve that for you just the same as
adding yet another filter that does the same thing.

As I said, I oppose duplicated features, except for very good reasons.
Licensing is not a good reason from my point of view.

Regards,

On 28.12.2017 15:17, Nicolas George wrote:
> Vasile Toncu (2017-12-28):
>>> Are there any features that tinterlace has and this new filter has not?
>>> If not, then I think it would be better to just replace tinterlace
>>> entirely.
> You did not answer the question. I asked if there are features in
> tinterlace that are not in reinterlace. You answered the opposite
> question.


There are some features that tinterlace has and reinterlace does not.

Tinterlace has some more flags, reinterlace has only simple low pass filter.
Also, in a newer version, tinterlace does support input formats on 16 bit.

Expecting these two, there are no other special features.


>>> I can answer myself: tinterlace has asm optimizations. This is not for
>>> me only to decide, but I am rather against having duplicated features
>>> just for licensing reasons.
>>> In this particular instance, I think you could use the GPL asm
>>> optimizations while having the filter itself LGPL.
>> Your arguments are valid, but what happens if I want to use a GPL tinterlace
>> in an closed source application that will be distributed.
> You cannot.
>
> But making tinterlace LGPL would solve that for you just the same as
> adding yet another filter that does the same thing.

Can one simply change the tinterlace from GPL to LGPL?

Vasile Toncu (2017-12-28):
> There are some features that tinterlace has and reinterlace does not.
> 
> Tinterlace has some more flags, reinterlace has only simple low pass filter.
> Also, in a newer version, tinterlace does support input formats on 16 bit.
> 
> Expecting these two, there are no other special features.

Thanks for that. I suspect that tinterlace is also faster. I suggest you
run some benchmarks.

> Can one simply change the tinterlace from GPL to LGPL?

Of course not. There are two non-simple courses of action to achieve it:

- Get the approval of all copyright holders. It has been done in the
  past for other filters.

- Remove the GPL tinterlace and at the same time add a new LGPL filter
  that does the same thing and is also called tinterlace.

The second is probably only acceptable if the new filter has all the
features and performance of the old one.

Regards,

On Thu, 28 Dec 2017 at 15:41, Nicolas George <george@nsup.org> wrote:

> Vasile Toncu (2017-12-28):
> > There are some features that tinterlace has and reinterlace does not.
> >
> > Tinterlace has some more flags, reinterlace has only simple low pass
> filter.
> > Also, in a newer version, tinterlace does support input formats on 16
> bit.
> >
> > Expecting these two, there are no other special features.
>
> Thanks for that. I suspect that tinterlace is also faster. I suggest you
> run some benchmarks.


I run  some benchmarks and reinterlace has basicaly the same performance as
tinterlace. For some modes, tinterlace is up to 5% faster. I run my tests
on an win64 machine and on an linux x64.



>
> > Can one simply change the tinterlace from GPL to LGPL?
>
> Of course not. There are two non-simple courses of action to achieve it:
>
> - Get the approval of all copyright holders. It has been done in the
>   past for other filters.
>
> - Remove the GPL tinterlace and at the same time add a new LGPL filter
>   that does the same thing and is also called tinterlace.
>
> The second is probably only acceptable if the new filter has all the
> features and performance of the old one.


Is there anything that I can do to the current version of reinterlace so
that the filter can be accepted?

Vasile Toncu (2017-12-28):
> Is there anything that I can do to the current version of reinterlace so
> that the filter can be accepted?

For me, yes: make it fully compatible with tinterlace and call it that.

Regards,

On Thu, 28 Dec 2017 at 17:14, Nicolas George <george@nsup.org> wrote:

> Vasile Toncu (2017-12-28):
> > Is there anything that I can do to the current version of reinterlace so
> > that the filter can be accepted?
>
> For me, yes: make it fully compatible with tinterlace and call it that.
>

The reinterlace is compatible with tinterlace on ffmpeg version n3.0.2

Will the same issues regarding the submission rise if I trie to submit the
filter to an older branch (3.0.2n)?  Is submision to an older version
possible?

Vasile Toncu (2017-12-28):
>				       Is submision to an older version
> possible?

No. Branches are only maintained for bugs. New development only happens
on Git head.

Regards,

2017-12-28 16:12 GMT+01:00 Vasile Toncu <vasile.toncu@tremend.com>:

> Is there anything that I can do to the current version of reinterlace
> so that the filter can be accepted?

Wasn't this already explained?
Add the existing (GPL'd) asm optimizations to your new filter (and
make sure the filter works both with and without --enable-gpl).

Or do I miss something?

Carl Eugen

Thank you for your guidance. I understand your points.

I was thinking about a simple solution:

Can it be submitted on a new branch, different than master? In the future I
may replace the tinterlace to be LGPL, until then can reinterlace exist on
a separate branch in git?

On Fri, Dec 29, 2017 at 12:14 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com>
wrote:

> 2017-12-28 16:12 GMT+01:00 Vasile Toncu <vasile.toncu@tremend.com>:
>
> > Is there anything that I can do to the current version of reinterlace
> > so that the filter can be accepted?
>
> Wasn't this already explained?
> Add the existing (GPL'd) asm optimizations to your new filter (and
> make sure the filter works both with and without --enable-gpl).
>
> Or do I miss something?
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

2018-01-03 16:08 GMT+01:00 Vasile Toncu <vasile.toncu@tremend.com>:

> Can it be submitted on a new branch, different than master?

You can create such a branch in your own github account.

> In the future I may replace the tinterlace to be LGPL, until
> then can reinterlace exist on a separate branch in git?

You cannot use the existing asm optimizations for your
filter?

Please do not top-post here, Carl Eugen

Hello,

 From the messages regarding the new reinterlace filter, based on your 
arguments and guidance, I came out with the following solution.

Modify tinterlace filter so that it would behave like that:

    /if GONGIG_GPL is defined:/

    /    use curent titnerlace code with asm opts/

    /else:/

    /    use the new reitnerlace filter, without asm opts/

After that I'll make sure that the new tinterlace gets compiled in 
ffmpeg without /--enable-gpl/ and /--enable-nonfree/. However, if one 
decides to use old tinterlace code with asm opts, he can use those 
configure flags.

That being said, can the new tinterlace be released under LGPL? It is 
important for me to confirm once again this, to avoid writing new code 
that will have no future life.



On 28.12.2017 17:30, Nicolas George wrote:
> Vasile Toncu (2017-12-28):
>> 				       Is submision to an older version
>> possible?
> No. Branches are only maintained for bugs. New development only happens
> on Git head.
>
> Regards,
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

On 1/10/18, Vasile Toncu <vasile.toncu@tremend.com> wrote:
> Hello,
>
>  From the messages regarding the new reinterlace filter, based on your
> arguments and guidance, I came out with the following solution.
>
> Modify tinterlace filter so that it would behave like that:
>
>     /if GONGIG_GPL is defined:/
>
>     /    use curent titnerlace code with asm opts/
>
>     /else:/
>
>     /    use the new reitnerlace filter, without asm opts/
>
> After that I'll make sure that the new tinterlace gets compiled in
> ffmpeg without /--enable-gpl/ and /--enable-nonfree/. However, if one
> decides to use old tinterlace code with asm opts, he can use those
> configure flags.
>
> That being said, can the new tinterlace be released under LGPL? It is
> important for me to confirm once again this, to avoid writing new code
> that will have no future life.

No such duplicatated funcionality is allowed in codebase.

As previously being said, just rewrite tinterlace filter in LGPL and add
shim for GPL asm parts.

2018-01-10 11:57 GMT+01:00 Vasile Toncu <vasile.toncu@tremend.com>:

> From the messages regarding the new reinterlace filter, based on your
> arguments and guidance, I came out with the following solution.
>
> Modify tinterlace filter so that it would behave like that:
>
>    /if GONGIG_GPL is defined:/
>
>    /    use curent titnerlace code with asm opts/
>
>    /else:/
>
>    /    use the new reitnerlace filter, without asm opts/

I believe the relevant question is if it is technically possible to
use the existing asm optimizations for the existing tinterlace
filter in your new reinterlace filter.
If it is technically possible, then the asm optimizations are
used if FFmpeg was compiled with --enable-gpl and are not
used if --enable-gpl was not passed to configure.
In both cases, an interlace filter exist.

Above solution seems worse than adding a new filter for
license reasons, something not all developers are opposed
to.

Carl Eugen