diff mbox series

[FFmpeg-devel,v4] avfilter/vf_zoompan: fix shaking when zooming

Message ID 20200129095451.14747-1-deibel.robert@googlemail.com
State New
Headers show
Series [FFmpeg-devel,v4] avfilter/vf_zoompan: fix shaking when zooming | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Robert Deibel Jan. 29, 2020, 9:54 a.m. UTC
Fix shaking of image when zoom is applied by the zoompan filter.
Resolves ticket https://trac.ffmpeg.org/ticket/4298
---
Fixed a typo/bug where dy was dx. Removed request for oversized frame from filter, instead allocate 
internal frame and copy contents to out frame once dimensions are correct.

 libavfilter/vf_zoompan.c | 109 +++++++++++++++++++++++++++++----------
 1 file changed, 82 insertions(+), 27 deletions(-)

Comments

Paul B Mahol Jan. 29, 2020, 10:49 a.m. UTC | #1
On 1/29/20, Robert Deibel <deibel.robert@googlemail.com> wrote:
> Fix shaking of image when zoom is applied by the zoompan filter.
> Resolves ticket https://trac.ffmpeg.org/ticket/4298
> ---
> Fixed a typo/bug where dy was dx. Removed request for oversized frame from
> filter, instead allocate
> internal frame and copy contents to out frame once dimensions are correct.
>
>  libavfilter/vf_zoompan.c | 109 +++++++++++++++++++++++++++++----------
>  1 file changed, 82 insertions(+), 27 deletions(-)
>
> diff --git a/libavfilter/vf_zoompan.c b/libavfilter/vf_zoompan.c
> index 59c9b19ec8..e18f9c8d1b 100644
> --- a/libavfilter/vf_zoompan.c
> +++ b/libavfilter/vf_zoompan.c
> @@ -150,16 +150,30 @@ static int config_output(AVFilterLink *outlink)
>      return 0;
>  }
>
> +/**
> + * Scales n to the next number divisible by 2 * 2^log_grid_size but
> minimally n + 2 * 2^log_grid_size.
> + *
> + * Used to scale the width and height of a frame to fit with the
> subsampling grid.
> + * @param n The number to be scaled.
> + * @param log_grid_size log 2 of the gridsize.
> + * @return The next number divisible by 2 * 2^log_grid_size but minimally n
> + 2 * 2^log_grid_size
> + */
> +static int scale_to_grid(int n, uint8_t log2_grid_size)
> +{
> +    return (((n + (1 << log2_grid_size) * 2) & ~((1 << log2_grid_size) -
> 1)) + 1) & ~1;;
> +}
> +
>  static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double
> *var_values, int i,
>                                 double *zoom, double *dx, double *dy)
>  {
>      ZPContext *s = ctx->priv;
>      AVFilterLink *outlink = ctx->outputs[0];
>      int64_t pts = s->frame_count;
> -    int k, x, y, w, h, ret = 0;
> +    int k, x, y, crop_x, crop_y, w, h, crop_w, crop_h, overscaled_w,
> overscaled_h, ret = 0;
>      uint8_t *input[4];
>      int px[4], py[4];
> -    AVFrame *out;
> +    AVFrame *out, *overscaled_frame;
> +    double dw, dh, dx_scaled, dy_scaled;
>
>      var_values[VAR_PX]    = s->x;
>      var_values[VAR_PY]    = s->y;
> @@ -173,32 +187,44 @@ static int output_single_frame(AVFilterContext *ctx,
> AVFrame *in, double *var_va
>
>      *zoom = av_clipd(*zoom, 1, 10);
>      var_values[VAR_ZOOM] = *zoom;
> -    w = in->width * (1.0 / *zoom);
> -    h = in->height * (1.0 / *zoom);
>
> -    *dx = av_expr_eval(s->x_expr, var_values, NULL);
> +    w = dw = (double) in->width / *zoom;
> +    h = dh = (double) in->height / *zoom;
> +    crop_w = scale_to_grid(w, s->desc->log2_chroma_w);
> +    crop_h = scale_to_grid(h, s->desc->log2_chroma_h);
> +    overscaled_w = outlink->w + (crop_w - dw) * *zoom;
> +    overscaled_h = outlink->h + (crop_h - dh) * *zoom;
>
> -    x = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
> -    var_values[VAR_X] = *dx;
> -    x &= ~((1 << s->desc->log2_chroma_w) - 1);
> +    *dx = av_expr_eval(s->x_expr, var_values, NULL);
> +    crop_x = ceil(av_clipd(*dx - (((double) crop_w - w) / 2.0), 0,
> FFMAX(in->width - crop_w, 0)));
> +    var_values[VAR_X] = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
> +    crop_x &= ~((1 << s->desc->log2_chroma_w) - 1);
>
>      *dy = av_expr_eval(s->y_expr, var_values, NULL);
> +    crop_y = ceil(av_clipd(*dy - (((double) crop_h - h)/ 2.0), 0,
> FFMAX(in->height - crop_h, 0)));
> +    var_values[VAR_Y] = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
> +    crop_y &= ~((1 << s->desc->log2_chroma_h) - 1);
>
> -    y = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
> -    var_values[VAR_Y] = *dy;
> -    y &= ~((1 << s->desc->log2_chroma_h) - 1);
> -
> -    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> -    if (!out) {
> +    overscaled_frame = av_frame_alloc();
> +    if (!overscaled_frame) {
>          ret = AVERROR(ENOMEM);
>          return ret;
>      }
>
> -    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
> -    px[0] = px[3] = x;
> +    overscaled_frame->height = overscaled_h;
> +    overscaled_frame->width = overscaled_w;
> +    overscaled_frame->format = outlink->format;
> +    if ((ret = av_frame_get_buffer(overscaled_frame, 0)) < 0)
> +        goto error;
>
> -    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
> -    py[0] = py[3] = y;
> +    px[1] = px[2] = AV_CEIL_RSHIFT(crop_x, s->desc->log2_chroma_w);
> +    px[0] = px[3] = crop_x;
> +
> +    py[1] = py[2] = AV_CEIL_RSHIFT(crop_y, s->desc->log2_chroma_h);
> +    py[0] = py[3] = crop_y;
> +
> +    for (k = 0; k < 4; k++)
> +        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
>
>      s->sws = sws_alloc_context();
>      if (!s->sws) {
> @@ -206,21 +232,48 @@ static int output_single_frame(AVFilterContext *ctx,
> AVFrame *in, double *var_va
>          goto error;
>      }
>
> -    for (k = 0; in->data[k]; k++)
> -        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
> -
> -    av_opt_set_int(s->sws, "srcw", w, 0);
> -    av_opt_set_int(s->sws, "srch", h, 0);
> +    av_opt_set_int(s->sws, "srcw", crop_w, 0);
> +    av_opt_set_int(s->sws, "srch", crop_h, 0);

Is this really needed?

>      av_opt_set_int(s->sws, "src_format", in->format, 0);
> -    av_opt_set_int(s->sws, "dstw", outlink->w, 0);
> -    av_opt_set_int(s->sws, "dsth", outlink->h, 0);
> +    av_opt_set_int(s->sws, "dstw", overscaled_w, 0);
> +    av_opt_set_int(s->sws, "dsth", overscaled_h, 0);

ditto

>      av_opt_set_int(s->sws, "dst_format", outlink->format, 0);
>      av_opt_set_int(s->sws, "sws_flags", SWS_BICUBIC, 0);
>
>      if ((ret = sws_init_context(s->sws, NULL, NULL)) < 0)
>          goto error;
>
> -    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0, h,
> out->data, out->linesize);
> +    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0,
> crop_h, overscaled_frame->data, overscaled_frame->linesize);
> +
> +    dx_scaled = ((crop_w - dw) * *zoom) / (((double) crop_w - dw) / (*dx -
> (double) crop_x));
> +    x = ceil(av_clipd(dx_scaled, 0, FFMAX(overscaled_w - outlink->w, 0)));
> +    x &= ~((1 << s->desc->log2_chroma_w) - 1);
> +
> +    dy_scaled = ((crop_h - dh) * *zoom) / (((double) crop_h - dh) / (*dy -
> (double) crop_y));
> +    y = ceil(av_clipd(dy_scaled, 0, FFMAX(overscaled_h - outlink->h, 0)));
> +    y &= ~((1 << s->desc->log2_chroma_h) - 1);
> +
> +    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
> +    px[0] = px[3] = x;
> +
> +    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
> +    py[0] = py[3] = y;
> +
> +    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> +    if (!out) {
> +        ret = AVERROR(ENOMEM);
> +        goto error;
> +    }
> +
> +    overscaled_frame->crop_left = x;
> +    overscaled_frame->crop_right = overscaled_w - outlink->w - x;
> +    overscaled_frame->crop_top = y;
> +    overscaled_frame->crop_bottom = overscaled_h - outlink->h - y;
> +    if ((ret = av_frame_apply_cropping(overscaled_frame,
> AV_FRAME_CROP_UNALIGNED)) < 0)
> +        goto error;
> +
> +    if ((ret = av_frame_copy(out, overscaled_frame)) < 0)
> +        goto error;
>
>      out->pts = pts;
>      s->frame_count++;
> @@ -229,6 +282,7 @@ static int output_single_frame(AVFilterContext *ctx,
> AVFrame *in, double *var_va
>      sws_freeContext(s->sws);
>      s->sws = NULL;
>      s->current_frame++;
> +    av_frame_free(&overscaled_frame);
>
>      if (s->current_frame >= s->nb_frames) {
>          if (*dx != -1)
> @@ -245,9 +299,10 @@ static int output_single_frame(AVFilterContext *ctx,
> AVFrame *in, double *var_va
>      }
>      return ret;
>  error:
> +    av_frame_free(&out);
>      sws_freeContext(s->sws);
>      s->sws = NULL;
> -    av_frame_free(&out);
> +    av_frame_free(&overscaled_frame);
>      return ret;
>  }
>
> --
> 2.25.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Robert Deibel Jan. 29, 2020, 11:37 a.m. UTC | #2
On 29.01.20 11:49, Paul B Mahol wrote:
> On 1/29/20, Robert Deibel <deibel.robert@googlemail.com> wrote:
>> -
>> -    av_opt_set_int(s->sws, "srcw", w, 0);
>> -    av_opt_set_int(s->sws, "srch", h, 0);
>> +    av_opt_set_int(s->sws, "srcw", crop_w, 0);
>> +    av_opt_set_int(s->sws, "srch", crop_h, 0);
> Is this really needed?
>
>>       av_opt_set_int(s->sws, "src_format", in->format, 0);
>> -    av_opt_set_int(s->sws, "dstw", outlink->w, 0);
>> -    av_opt_set_int(s->sws, "dsth", outlink->h, 0);
>> +    av_opt_set_int(s->sws, "dstw", overscaled_w, 0);
>> +    av_opt_set_int(s->sws, "dsth", overscaled_h, 0);
> ditto
Yes, I'm afraid so. Omitting these lines will result in faulty output, 
so is any combination of the original and the modified parameters. In 
fact the only combination to produce any image (apart from the one in 
the patch) is crop_w/crop_/h and outlink->w/outlink->h but even that's 
faulty.
Robert Deibel Feb. 6, 2020, 11:37 a.m. UTC | #3
Would appreciate further review or info.

On 29.01.20 10:54, Robert Deibel wrote:
> Fix shaking of image when zoom is applied by the zoompan filter.
> Resolves ticket https://trac.ffmpeg.org/ticket/4298
> ---
> Fixed a typo/bug where dy was dx. Removed request for oversized frame from filter, instead allocate
> internal frame and copy contents to out frame once dimensions are correct.
>
>   libavfilter/vf_zoompan.c | 109 +++++++++++++++++++++++++++++----------
>   1 file changed, 82 insertions(+), 27 deletions(-)
>
> diff --git a/libavfilter/vf_zoompan.c b/libavfilter/vf_zoompan.c
> index 59c9b19ec8..e18f9c8d1b 100644
> --- a/libavfilter/vf_zoompan.c
> +++ b/libavfilter/vf_zoompan.c
> @@ -150,16 +150,30 @@ static int config_output(AVFilterLink *outlink)
>       return 0;
>   }
>   
> +/**
> + * Scales n to the next number divisible by 2 * 2^log_grid_size but minimally n + 2 * 2^log_grid_size.
> + *
> + * Used to scale the width and height of a frame to fit with the subsampling grid.
> + * @param n The number to be scaled.
> + * @param log_grid_size log 2 of the gridsize.
> + * @return The next number divisible by 2 * 2^log_grid_size but minimally n + 2 * 2^log_grid_size
> + */
> +static int scale_to_grid(int n, uint8_t log2_grid_size)
> +{
> +    return (((n + (1 << log2_grid_size) * 2) & ~((1 << log2_grid_size) - 1)) + 1) & ~1;;
> +}
> +
>   static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_values, int i,
>                                  double *zoom, double *dx, double *dy)
>   {
>       ZPContext *s = ctx->priv;
>       AVFilterLink *outlink = ctx->outputs[0];
>       int64_t pts = s->frame_count;
> -    int k, x, y, w, h, ret = 0;
> +    int k, x, y, crop_x, crop_y, w, h, crop_w, crop_h, overscaled_w, overscaled_h, ret = 0;
>       uint8_t *input[4];
>       int px[4], py[4];
> -    AVFrame *out;
> +    AVFrame *out, *overscaled_frame;
> +    double dw, dh, dx_scaled, dy_scaled;
>   
>       var_values[VAR_PX]    = s->x;
>       var_values[VAR_PY]    = s->y;
> @@ -173,32 +187,44 @@ static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
>   
>       *zoom = av_clipd(*zoom, 1, 10);
>       var_values[VAR_ZOOM] = *zoom;
> -    w = in->width * (1.0 / *zoom);
> -    h = in->height * (1.0 / *zoom);
>   
> -    *dx = av_expr_eval(s->x_expr, var_values, NULL);
> +    w = dw = (double) in->width / *zoom;
> +    h = dh = (double) in->height / *zoom;
> +    crop_w = scale_to_grid(w, s->desc->log2_chroma_w);
> +    crop_h = scale_to_grid(h, s->desc->log2_chroma_h);
> +    overscaled_w = outlink->w + (crop_w - dw) * *zoom;
> +    overscaled_h = outlink->h + (crop_h - dh) * *zoom;
>   
> -    x = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
> -    var_values[VAR_X] = *dx;
> -    x &= ~((1 << s->desc->log2_chroma_w) - 1);
> +    *dx = av_expr_eval(s->x_expr, var_values, NULL);
> +    crop_x = ceil(av_clipd(*dx - (((double) crop_w - w) / 2.0), 0, FFMAX(in->width - crop_w, 0)));
> +    var_values[VAR_X] = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
> +    crop_x &= ~((1 << s->desc->log2_chroma_w) - 1);
>   
>       *dy = av_expr_eval(s->y_expr, var_values, NULL);
> +    crop_y = ceil(av_clipd(*dy - (((double) crop_h - h)/ 2.0), 0, FFMAX(in->height - crop_h, 0)));
> +    var_values[VAR_Y] = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
> +    crop_y &= ~((1 << s->desc->log2_chroma_h) - 1);
>   
> -    y = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
> -    var_values[VAR_Y] = *dy;
> -    y &= ~((1 << s->desc->log2_chroma_h) - 1);
> -
> -    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> -    if (!out) {
> +    overscaled_frame = av_frame_alloc();
> +    if (!overscaled_frame) {
>           ret = AVERROR(ENOMEM);
>           return ret;
>       }
>   
> -    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
> -    px[0] = px[3] = x;
> +    overscaled_frame->height = overscaled_h;
> +    overscaled_frame->width = overscaled_w;
> +    overscaled_frame->format = outlink->format;
> +    if ((ret = av_frame_get_buffer(overscaled_frame, 0)) < 0)
> +        goto error;
>   
> -    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
> -    py[0] = py[3] = y;
> +    px[1] = px[2] = AV_CEIL_RSHIFT(crop_x, s->desc->log2_chroma_w);
> +    px[0] = px[3] = crop_x;
> +
> +    py[1] = py[2] = AV_CEIL_RSHIFT(crop_y, s->desc->log2_chroma_h);
> +    py[0] = py[3] = crop_y;
> +
> +    for (k = 0; k < 4; k++)
> +        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
>   
>       s->sws = sws_alloc_context();
>       if (!s->sws) {
> @@ -206,21 +232,48 @@ static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
>           goto error;
>       }
>   
> -    for (k = 0; in->data[k]; k++)
> -        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
> -
> -    av_opt_set_int(s->sws, "srcw", w, 0);
> -    av_opt_set_int(s->sws, "srch", h, 0);
> +    av_opt_set_int(s->sws, "srcw", crop_w, 0);
> +    av_opt_set_int(s->sws, "srch", crop_h, 0);
>       av_opt_set_int(s->sws, "src_format", in->format, 0);
> -    av_opt_set_int(s->sws, "dstw", outlink->w, 0);
> -    av_opt_set_int(s->sws, "dsth", outlink->h, 0);
> +    av_opt_set_int(s->sws, "dstw", overscaled_w, 0);
> +    av_opt_set_int(s->sws, "dsth", overscaled_h, 0);
>       av_opt_set_int(s->sws, "dst_format", outlink->format, 0);
>       av_opt_set_int(s->sws, "sws_flags", SWS_BICUBIC, 0);
>   
>       if ((ret = sws_init_context(s->sws, NULL, NULL)) < 0)
>           goto error;
>   
> -    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0, h, out->data, out->linesize);
> +    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0, crop_h, overscaled_frame->data, overscaled_frame->linesize);
> +
> +    dx_scaled = ((crop_w - dw) * *zoom) / (((double) crop_w - dw) / (*dx - (double) crop_x));
> +    x = ceil(av_clipd(dx_scaled, 0, FFMAX(overscaled_w - outlink->w, 0)));
> +    x &= ~((1 << s->desc->log2_chroma_w) - 1);
> +
> +    dy_scaled = ((crop_h - dh) * *zoom) / (((double) crop_h - dh) / (*dy - (double) crop_y));
> +    y = ceil(av_clipd(dy_scaled, 0, FFMAX(overscaled_h - outlink->h, 0)));
> +    y &= ~((1 << s->desc->log2_chroma_h) - 1);
> +
> +    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
> +    px[0] = px[3] = x;
> +
> +    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
> +    py[0] = py[3] = y;
> +
> +    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> +    if (!out) {
> +        ret = AVERROR(ENOMEM);
> +        goto error;
> +    }
> +
> +    overscaled_frame->crop_left = x;
> +    overscaled_frame->crop_right = overscaled_w - outlink->w - x;
> +    overscaled_frame->crop_top = y;
> +    overscaled_frame->crop_bottom = overscaled_h - outlink->h - y;
> +    if ((ret = av_frame_apply_cropping(overscaled_frame, AV_FRAME_CROP_UNALIGNED)) < 0)
> +        goto error;
> +
> +    if ((ret = av_frame_copy(out, overscaled_frame)) < 0)
> +        goto error;
>   
>       out->pts = pts;
>       s->frame_count++;
> @@ -229,6 +282,7 @@ static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
>       sws_freeContext(s->sws);
>       s->sws = NULL;
>       s->current_frame++;
> +    av_frame_free(&overscaled_frame);
>   
>       if (s->current_frame >= s->nb_frames) {
>           if (*dx != -1)
> @@ -245,9 +299,10 @@ static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
>       }
>       return ret;
>   error:
> +    av_frame_free(&out);
>       sws_freeContext(s->sws);
>       s->sws = NULL;
> -    av_frame_free(&out);
> +    av_frame_free(&overscaled_frame);
>       return ret;
>   }
>
Paul B Mahol Feb. 7, 2020, 8:23 p.m. UTC | #4
On 2/6/20, Robert Deibel <deibel.robert@googlemail.com> wrote:
> Would appreciate further review or info.

Are you sure this change does not break older scripts?

>
> On 29.01.20 10:54, Robert Deibel wrote:
>> Fix shaking of image when zoom is applied by the zoompan filter.
>> Resolves ticket https://trac.ffmpeg.org/ticket/4298
>> ---
>> Fixed a typo/bug where dy was dx. Removed request for oversized frame from
>> filter, instead allocate
>> internal frame and copy contents to out frame once dimensions are correct.
>>
>>   libavfilter/vf_zoompan.c | 109 +++++++++++++++++++++++++++++----------
>>   1 file changed, 82 insertions(+), 27 deletions(-)
>>
>> diff --git a/libavfilter/vf_zoompan.c b/libavfilter/vf_zoompan.c
>> index 59c9b19ec8..e18f9c8d1b 100644
>> --- a/libavfilter/vf_zoompan.c
>> +++ b/libavfilter/vf_zoompan.c
>> @@ -150,16 +150,30 @@ static int config_output(AVFilterLink *outlink)
>>       return 0;
>>   }
>>
>> +/**
>> + * Scales n to the next number divisible by 2 * 2^log_grid_size but
>> minimally n + 2 * 2^log_grid_size.
>> + *
>> + * Used to scale the width and height of a frame to fit with the
>> subsampling grid.
>> + * @param n The number to be scaled.
>> + * @param log_grid_size log 2 of the gridsize.
>> + * @return The next number divisible by 2 * 2^log_grid_size but minimally
>> n + 2 * 2^log_grid_size
>> + */
>> +static int scale_to_grid(int n, uint8_t log2_grid_size)
>> +{
>> +    return (((n + (1 << log2_grid_size) * 2) & ~((1 << log2_grid_size) -
>> 1)) + 1) & ~1;;
>> +}
>> +
>>   static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double
>> *var_values, int i,
>>                                  double *zoom, double *dx, double *dy)
>>   {
>>       ZPContext *s = ctx->priv;
>>       AVFilterLink *outlink = ctx->outputs[0];
>>       int64_t pts = s->frame_count;
>> -    int k, x, y, w, h, ret = 0;
>> +    int k, x, y, crop_x, crop_y, w, h, crop_w, crop_h, overscaled_w,
>> overscaled_h, ret = 0;
>>       uint8_t *input[4];
>>       int px[4], py[4];
>> -    AVFrame *out;
>> +    AVFrame *out, *overscaled_frame;
>> +    double dw, dh, dx_scaled, dy_scaled;
>>
>>       var_values[VAR_PX]    = s->x;
>>       var_values[VAR_PY]    = s->y;
>> @@ -173,32 +187,44 @@ static int output_single_frame(AVFilterContext *ctx,
>> AVFrame *in, double *var_va
>>
>>       *zoom = av_clipd(*zoom, 1, 10);
>>       var_values[VAR_ZOOM] = *zoom;
>> -    w = in->width * (1.0 / *zoom);
>> -    h = in->height * (1.0 / *zoom);
>>
>> -    *dx = av_expr_eval(s->x_expr, var_values, NULL);
>> +    w = dw = (double) in->width / *zoom;
>> +    h = dh = (double) in->height / *zoom;
>> +    crop_w = scale_to_grid(w, s->desc->log2_chroma_w);
>> +    crop_h = scale_to_grid(h, s->desc->log2_chroma_h);
>> +    overscaled_w = outlink->w + (crop_w - dw) * *zoom;
>> +    overscaled_h = outlink->h + (crop_h - dh) * *zoom;
>>
>> -    x = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
>> -    var_values[VAR_X] = *dx;
>> -    x &= ~((1 << s->desc->log2_chroma_w) - 1);
>> +    *dx = av_expr_eval(s->x_expr, var_values, NULL);
>> +    crop_x = ceil(av_clipd(*dx - (((double) crop_w - w) / 2.0), 0,
>> FFMAX(in->width - crop_w, 0)));
>> +    var_values[VAR_X] = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
>> +    crop_x &= ~((1 << s->desc->log2_chroma_w) - 1);
>>
>>       *dy = av_expr_eval(s->y_expr, var_values, NULL);
>> +    crop_y = ceil(av_clipd(*dy - (((double) crop_h - h)/ 2.0), 0,
>> FFMAX(in->height - crop_h, 0)));
>> +    var_values[VAR_Y] = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
>> +    crop_y &= ~((1 << s->desc->log2_chroma_h) - 1);
>>
>> -    y = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
>> -    var_values[VAR_Y] = *dy;
>> -    y &= ~((1 << s->desc->log2_chroma_h) - 1);
>> -
>> -    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> -    if (!out) {
>> +    overscaled_frame = av_frame_alloc();
>> +    if (!overscaled_frame) {
>>           ret = AVERROR(ENOMEM);
>>           return ret;
>>       }
>>
>> -    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
>> -    px[0] = px[3] = x;
>> +    overscaled_frame->height = overscaled_h;
>> +    overscaled_frame->width = overscaled_w;
>> +    overscaled_frame->format = outlink->format;
>> +    if ((ret = av_frame_get_buffer(overscaled_frame, 0)) < 0)
>> +        goto error;
>>
>> -    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
>> -    py[0] = py[3] = y;
>> +    px[1] = px[2] = AV_CEIL_RSHIFT(crop_x, s->desc->log2_chroma_w);
>> +    px[0] = px[3] = crop_x;
>> +
>> +    py[1] = py[2] = AV_CEIL_RSHIFT(crop_y, s->desc->log2_chroma_h);
>> +    py[0] = py[3] = crop_y;
>> +
>> +    for (k = 0; k < 4; k++)
>> +        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
>>
>>       s->sws = sws_alloc_context();
>>       if (!s->sws) {
>> @@ -206,21 +232,48 @@ static int output_single_frame(AVFilterContext *ctx,
>> AVFrame *in, double *var_va
>>           goto error;
>>       }
>>
>> -    for (k = 0; in->data[k]; k++)
>> -        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
>> -
>> -    av_opt_set_int(s->sws, "srcw", w, 0);
>> -    av_opt_set_int(s->sws, "srch", h, 0);
>> +    av_opt_set_int(s->sws, "srcw", crop_w, 0);
>> +    av_opt_set_int(s->sws, "srch", crop_h, 0);
>>       av_opt_set_int(s->sws, "src_format", in->format, 0);
>> -    av_opt_set_int(s->sws, "dstw", outlink->w, 0);
>> -    av_opt_set_int(s->sws, "dsth", outlink->h, 0);
>> +    av_opt_set_int(s->sws, "dstw", overscaled_w, 0);
>> +    av_opt_set_int(s->sws, "dsth", overscaled_h, 0);
>>       av_opt_set_int(s->sws, "dst_format", outlink->format, 0);
>>       av_opt_set_int(s->sws, "sws_flags", SWS_BICUBIC, 0);
>>
>>       if ((ret = sws_init_context(s->sws, NULL, NULL)) < 0)
>>           goto error;
>>
>> -    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0, h,
>> out->data, out->linesize);
>> +    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0,
>> crop_h, overscaled_frame->data, overscaled_frame->linesize);
>> +
>> +    dx_scaled = ((crop_w - dw) * *zoom) / (((double) crop_w - dw) / (*dx
>> - (double) crop_x));
>> +    x = ceil(av_clipd(dx_scaled, 0, FFMAX(overscaled_w - outlink->w,
>> 0)));
>> +    x &= ~((1 << s->desc->log2_chroma_w) - 1);
>> +
>> +    dy_scaled = ((crop_h - dh) * *zoom) / (((double) crop_h - dh) / (*dy
>> - (double) crop_y));
>> +    y = ceil(av_clipd(dy_scaled, 0, FFMAX(overscaled_h - outlink->h,
>> 0)));
>> +    y &= ~((1 << s->desc->log2_chroma_h) - 1);
>> +
>> +    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
>> +    px[0] = px[3] = x;
>> +
>> +    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
>> +    py[0] = py[3] = y;
>> +
>> +    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> +    if (!out) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto error;
>> +    }
>> +
>> +    overscaled_frame->crop_left = x;
>> +    overscaled_frame->crop_right = overscaled_w - outlink->w - x;
>> +    overscaled_frame->crop_top = y;
>> +    overscaled_frame->crop_bottom = overscaled_h - outlink->h - y;
>> +    if ((ret = av_frame_apply_cropping(overscaled_frame,
>> AV_FRAME_CROP_UNALIGNED)) < 0)
>> +        goto error;
>> +
>> +    if ((ret = av_frame_copy(out, overscaled_frame)) < 0)
>> +        goto error;
>>
>>       out->pts = pts;
>>       s->frame_count++;
>> @@ -229,6 +282,7 @@ static int output_single_frame(AVFilterContext *ctx,
>> AVFrame *in, double *var_va
>>       sws_freeContext(s->sws);
>>       s->sws = NULL;
>>       s->current_frame++;
>> +    av_frame_free(&overscaled_frame);
>>
>>       if (s->current_frame >= s->nb_frames) {
>>           if (*dx != -1)
>> @@ -245,9 +299,10 @@ static int output_single_frame(AVFilterContext *ctx,
>> AVFrame *in, double *var_va
>>       }
>>       return ret;
>>   error:
>> +    av_frame_free(&out);
>>       sws_freeContext(s->sws);
>>       s->sws = NULL;
>> -    av_frame_free(&out);
>> +    av_frame_free(&overscaled_frame);
>>       return ret;
>>   }
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Robert Deibel Feb. 13, 2020, 1:40 p.m. UTC | #5
On 07.02.20 21:23, Paul B Mahol wrote:
> On 2/6/20, Robert Deibel <deibel.robert@googlemail.com> wrote:
>> Would appreciate further review or info.
> Are you sure this change does not break older scripts?

You were right, it did break older scripts. Clips that were upscaled in 
a previous filter would be transformed in a worse way than before.

Fixed it in a new patch. Will test some more and send it again.
diff mbox series

Patch

diff --git a/libavfilter/vf_zoompan.c b/libavfilter/vf_zoompan.c
index 59c9b19ec8..e18f9c8d1b 100644
--- a/libavfilter/vf_zoompan.c
+++ b/libavfilter/vf_zoompan.c
@@ -150,16 +150,30 @@  static int config_output(AVFilterLink *outlink)
     return 0;
 }
 
+/**
+ * Scales n to the next number divisible by 2 * 2^log_grid_size but minimally n + 2 * 2^log_grid_size.
+ *
+ * Used to scale the width and height of a frame to fit with the subsampling grid.
+ * @param n The number to be scaled.
+ * @param log_grid_size log 2 of the gridsize.
+ * @return The next number divisible by 2 * 2^log_grid_size but minimally n + 2 * 2^log_grid_size
+ */
+static int scale_to_grid(int n, uint8_t log2_grid_size)
+{
+    return (((n + (1 << log2_grid_size) * 2) & ~((1 << log2_grid_size) - 1)) + 1) & ~1;;
+}
+
 static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_values, int i,
                                double *zoom, double *dx, double *dy)
 {
     ZPContext *s = ctx->priv;
     AVFilterLink *outlink = ctx->outputs[0];
     int64_t pts = s->frame_count;
-    int k, x, y, w, h, ret = 0;
+    int k, x, y, crop_x, crop_y, w, h, crop_w, crop_h, overscaled_w, overscaled_h, ret = 0;
     uint8_t *input[4];
     int px[4], py[4];
-    AVFrame *out;
+    AVFrame *out, *overscaled_frame;
+    double dw, dh, dx_scaled, dy_scaled;
 
     var_values[VAR_PX]    = s->x;
     var_values[VAR_PY]    = s->y;
@@ -173,32 +187,44 @@  static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
 
     *zoom = av_clipd(*zoom, 1, 10);
     var_values[VAR_ZOOM] = *zoom;
-    w = in->width * (1.0 / *zoom);
-    h = in->height * (1.0 / *zoom);
 
-    *dx = av_expr_eval(s->x_expr, var_values, NULL);
+    w = dw = (double) in->width / *zoom;
+    h = dh = (double) in->height / *zoom;
+    crop_w = scale_to_grid(w, s->desc->log2_chroma_w);
+    crop_h = scale_to_grid(h, s->desc->log2_chroma_h);
+    overscaled_w = outlink->w + (crop_w - dw) * *zoom;
+    overscaled_h = outlink->h + (crop_h - dh) * *zoom;
 
-    x = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
-    var_values[VAR_X] = *dx;
-    x &= ~((1 << s->desc->log2_chroma_w) - 1);
+    *dx = av_expr_eval(s->x_expr, var_values, NULL);
+    crop_x = ceil(av_clipd(*dx - (((double) crop_w - w) / 2.0), 0, FFMAX(in->width - crop_w, 0)));
+    var_values[VAR_X] = *dx = av_clipd(*dx, 0, FFMAX(in->width - w, 0));
+    crop_x &= ~((1 << s->desc->log2_chroma_w) - 1);
 
     *dy = av_expr_eval(s->y_expr, var_values, NULL);
+    crop_y = ceil(av_clipd(*dy - (((double) crop_h - h)/ 2.0), 0, FFMAX(in->height - crop_h, 0)));
+    var_values[VAR_Y] = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
+    crop_y &= ~((1 << s->desc->log2_chroma_h) - 1);
 
-    y = *dy = av_clipd(*dy, 0, FFMAX(in->height - h, 0));
-    var_values[VAR_Y] = *dy;
-    y &= ~((1 << s->desc->log2_chroma_h) - 1);
-
-    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
-    if (!out) {
+    overscaled_frame = av_frame_alloc();
+    if (!overscaled_frame) {
         ret = AVERROR(ENOMEM);
         return ret;
     }
 
-    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
-    px[0] = px[3] = x;
+    overscaled_frame->height = overscaled_h;
+    overscaled_frame->width = overscaled_w;
+    overscaled_frame->format = outlink->format;
+    if ((ret = av_frame_get_buffer(overscaled_frame, 0)) < 0)
+        goto error;
 
-    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
-    py[0] = py[3] = y;
+    px[1] = px[2] = AV_CEIL_RSHIFT(crop_x, s->desc->log2_chroma_w);
+    px[0] = px[3] = crop_x;
+
+    py[1] = py[2] = AV_CEIL_RSHIFT(crop_y, s->desc->log2_chroma_h);
+    py[0] = py[3] = crop_y;
+
+    for (k = 0; k < 4; k++)
+        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
 
     s->sws = sws_alloc_context();
     if (!s->sws) {
@@ -206,21 +232,48 @@  static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
         goto error;
     }
 
-    for (k = 0; in->data[k]; k++)
-        input[k] = in->data[k] + py[k] * in->linesize[k] + px[k];
-
-    av_opt_set_int(s->sws, "srcw", w, 0);
-    av_opt_set_int(s->sws, "srch", h, 0);
+    av_opt_set_int(s->sws, "srcw", crop_w, 0);
+    av_opt_set_int(s->sws, "srch", crop_h, 0);
     av_opt_set_int(s->sws, "src_format", in->format, 0);
-    av_opt_set_int(s->sws, "dstw", outlink->w, 0);
-    av_opt_set_int(s->sws, "dsth", outlink->h, 0);
+    av_opt_set_int(s->sws, "dstw", overscaled_w, 0);
+    av_opt_set_int(s->sws, "dsth", overscaled_h, 0);
     av_opt_set_int(s->sws, "dst_format", outlink->format, 0);
     av_opt_set_int(s->sws, "sws_flags", SWS_BICUBIC, 0);
 
     if ((ret = sws_init_context(s->sws, NULL, NULL)) < 0)
         goto error;
 
-    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0, h, out->data, out->linesize);
+    sws_scale(s->sws, (const uint8_t *const *)&input, in->linesize, 0, crop_h, overscaled_frame->data, overscaled_frame->linesize);
+
+    dx_scaled = ((crop_w - dw) * *zoom) / (((double) crop_w - dw) / (*dx - (double) crop_x));
+    x = ceil(av_clipd(dx_scaled, 0, FFMAX(overscaled_w - outlink->w, 0)));
+    x &= ~((1 << s->desc->log2_chroma_w) - 1);
+
+    dy_scaled = ((crop_h - dh) * *zoom) / (((double) crop_h - dh) / (*dy - (double) crop_y));
+    y = ceil(av_clipd(dy_scaled, 0, FFMAX(overscaled_h - outlink->h, 0)));
+    y &= ~((1 << s->desc->log2_chroma_h) - 1);
+
+    px[1] = px[2] = AV_CEIL_RSHIFT(x, s->desc->log2_chroma_w);
+    px[0] = px[3] = x;
+
+    py[1] = py[2] = AV_CEIL_RSHIFT(y, s->desc->log2_chroma_h);
+    py[0] = py[3] = y;
+
+    out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
+    if (!out) {
+        ret = AVERROR(ENOMEM);
+        goto error;
+    }
+
+    overscaled_frame->crop_left = x;
+    overscaled_frame->crop_right = overscaled_w - outlink->w - x;
+    overscaled_frame->crop_top = y;
+    overscaled_frame->crop_bottom = overscaled_h - outlink->h - y;
+    if ((ret = av_frame_apply_cropping(overscaled_frame, AV_FRAME_CROP_UNALIGNED)) < 0)
+        goto error;
+
+    if ((ret = av_frame_copy(out, overscaled_frame)) < 0)
+        goto error;
 
     out->pts = pts;
     s->frame_count++;
@@ -229,6 +282,7 @@  static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
     sws_freeContext(s->sws);
     s->sws = NULL;
     s->current_frame++;
+    av_frame_free(&overscaled_frame);
 
     if (s->current_frame >= s->nb_frames) {
         if (*dx != -1)
@@ -245,9 +299,10 @@  static int output_single_frame(AVFilterContext *ctx, AVFrame *in, double *var_va
     }
     return ret;
 error:
+    av_frame_free(&out);
     sws_freeContext(s->sws);
     s->sws = NULL;
-    av_frame_free(&out);
+    av_frame_free(&overscaled_frame);
     return ret;
 }