[FFmpeg-devel,13/19] avfilter/af_aiir: Avoid temporary buffer when drawing image

When creating the output video, a temporary buffer is used whose elments
are only used three times: Once to store a value in it, once more to add
the value of the preceding element of the temporary buffer to the current
element and once more to add the current i. element to the i. element of
another bufffer; the two latter operations were performed in one loop,
whereas the first was performed in a loop of its own.

Yet these loops can be combined and the temporary buffer avoided. All
one needs to do is store the running total of the elements that
currently are in the temporary buffer and keep the preceding value of
the destination buffer (it is needed to calculate the next element of
the current temporary buffer, i.e. it is needed to know by how much to
increment the running total).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
One could combine various allocations here and in many other places.
But for now I'll leave this to a future patchset.

 libavfilter/af_aiir.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Andreas Rheinhardt (12020-08-25):
> When creating the output video, a temporary buffer is used whose elments
> are only used three times: Once to store a value in it, once more to add
> the value of the preceding element of the temporary buffer to the current
> element and once more to add the current i. element to the i. element of
> another bufffer; the two latter operations were performed in one loop,
> whereas the first was performed in a loop of its own.
> 
> Yet these loops can be combined and the temporary buffer avoided. All
> one needs to do is store the running total of the elements that
> currently are in the temporary buffer and keep the preceding value of
> the destination buffer (it is needed to calculate the next element of
> the current temporary buffer, i.e. it is needed to know by how much to
> increment the running total).

I think "i." is a germanism. I do not feel confident to judge the
contents this patch.

The other patches look good to me (except a long line in the commit
message of #17), but I only maintain amerge, probably wait a few days
for he others.

Thanks.

Regards,

On 8/25/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
> When creating the output video, a temporary buffer is used whose elments
> are only used three times: Once to store a value in it, once more to add
> the value of the preceding element of the temporary buffer to the current
> element and once more to add the current i. element to the i. element of
> another bufffer; the two latter operations were performed in one loop,
> whereas the first was performed in a loop of its own.
>
> Yet these loops can be combined and the temporary buffer avoided. All
> one needs to do is store the running total of the elements that
> currently are in the temporary buffer and keep the preceding value of
> the destination buffer (it is needed to calculate the next element of
> the current temporary buffer, i.e. it is needed to know by how much to
> increment the running total).
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> One could combine various allocations here and in many other places.
> But for now I'll leave this to a future patchset.
>
>  libavfilter/af_aiir.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/libavfilter/af_aiir.c b/libavfilter/af_aiir.c
> index 3df25b4d9b..75bb79eb06 100644
> --- a/libavfilter/af_aiir.c
> +++ b/libavfilter/af_aiir.c
> @@ -866,8 +866,9 @@ static void get_response(int channel, int format, double
> w,
>  static void draw_response(AVFilterContext *ctx, AVFrame *out, int
> sample_rate)
>  {
>      AudioIIRContext *s = ctx->priv;
> -    double *mag, *phase, *temp, *delay, min = DBL_MAX, max = -DBL_MAX;
> +    double *mag, *phase, *delay, min = DBL_MAX, max = -DBL_MAX;
>      double min_delay = DBL_MAX, max_delay = -DBL_MAX, min_phase, max_phase;
> +    double tmp, oldphase;
>      int prev_ymag = -1, prev_yphase = -1, prev_ydelay = -1;
>      char text[32];
>      int ch, i;
> @@ -875,10 +876,9 @@ static void draw_response(AVFilterContext *ctx, AVFrame
> *out, int sample_rate)
>      memset(out->data[0], 0, s->h * out->linesize[0]);
>
>      phase = av_malloc_array(s->w, sizeof(*phase));
> -    temp = av_malloc_array(s->w, sizeof(*temp));
>      mag = av_malloc_array(s->w, sizeof(*mag));
>      delay = av_malloc_array(s->w, sizeof(*delay));
> -    if (!mag || !phase || !delay || !temp)
> +    if (!mag || !phase || !delay)
>          goto end;
>
>      ch = av_clip(s->ir_channel, 0, s->channels - 1);
> @@ -898,17 +898,15 @@ static void draw_response(AVFilterContext *ctx,
> AVFrame *out, int sample_rate)
>          max = fmax(max, mag[i]);
>      }
>
> -    temp[0] = 0.;
> -    for (i = 0; i < s->w - 1; i++) {
> -        double d = phase[i] - phase[i + 1];
> -        temp[i + 1] = ceil(fabs(d) / (2. * M_PI)) * 2. * M_PI * ((d > M_PI)
> - (d < -M_PI));
> -    }
> -
> +    tmp = 0.;
> +    oldphase  = phase[0];
>      min_phase = phase[0];
>      max_phase = phase[0];
>      for (i = 1; i < s->w; i++) {
> -        temp[i] += temp[i - 1];
> -        phase[i] += temp[i];
> +        double d  = oldphase - phase[i];
> +        oldphase  = phase[i];
> +        tmp += ceil(fabs(d) / (2. * M_PI)) * 2. * M_PI * ((d > M_PI) - (d <
> -M_PI));
> +        phase[i] += tmp;
>          min_phase = fmin(min_phase, phase[i]);
>          max_phase = fmax(max_phase, phase[i]);
>      }
> @@ -975,7 +973,6 @@ static void draw_response(AVFilterContext *ctx, AVFrame
> *out, int sample_rate)
>
>  end:
>      av_free(delay);
> -    av_free(temp);
>      av_free(phase);
>      av_free(mag);
>  }
> --
> 2.20.1
>

Should be fine if output drawn does not change drastically.