Re: [FFmpeg-devel] [PATCH] avfilter/asrc_sine: Generate full amplitude sine wave.

From: Paul B Mahol <onemda@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Santanu Biswas <shbisws@gmail.com>
Subject: Re: [FFmpeg-devel] [PATCH] avfilter/asrc_sine: Generate full amplitude sine wave.
Date: Sat, 28 May 2022 10:48:36 +0200
Message-ID: <CAPYw7P5PQ2HXCwwm24Xg8GFbeV2qvba1o8Bbe2d7OApFjZ4qOQ@mail.gmail.com> (raw)
In-Reply-To: <20220528083921.16538-1-shbisws@gmail.com>

On Sat, May 28, 2022 at 10:39 AM Santanu Biswas <shbisws@gmail.com> wrote:

> Previously, generating a full amplitude sine wave require to either
> use the 'volume' filter in combination with this filter or manually
> generate sine wave using 'aevalsrc' (e.g "aevalsrc=sin(2*PI*t*440)").
> This method is not bit-exact; therefore it is much coherent to not
> scale the amplitude by 1/8 and keep it as it is.
>
>
I think you are breaking fate tests with this change.

Also why this can not be set at start instead of being hardcoded?

> Signed-off-by: Santanu Biswas <shbisws@gmail.com>
> ---
>  doc/filters.texi        | 2 +-
>  libavfilter/asrc_sine.c | 8 ++------
>  2 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 0e10946cca..a42e27b9b5 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -7452,7 +7452,7 @@ Set number of taps for low-pass filter.
>
>  @section sine
>
> -Generate an audio signal made of a sine wave with amplitude 1/8.
> +Generate an audio signal made of a sine wave.
>
>  The audio signal is bit-exact.
>
> diff --git a/libavfilter/asrc_sine.c b/libavfilter/asrc_sine.c
> index c0d8d2265b..6c2f9d40f3 100644
> --- a/libavfilter/asrc_sine.c
> +++ b/libavfilter/asrc_sine.c
> @@ -83,13 +83,11 @@ static const AVOption sine_options[] = {
>  AVFILTER_DEFINE_CLASS(sine);
>
>  #define LOG_PERIOD 15
> -#define AMPLITUDE 4095
> -#define AMPLITUDE_SHIFT 3
>
>  static void make_sin_table(int16_t *sin)
>  {
>      unsigned half_pi = 1 << (LOG_PERIOD - 2);
> -    unsigned ampls = AMPLITUDE << AMPLITUDE_SHIFT;
> +    unsigned ampls = 32767;
>      uint64_t unit2 = (uint64_t)(ampls * ampls) << 32;
>      unsigned step, i, c, s, k, new_k, n2;
>
> @@ -116,9 +114,7 @@ static void make_sin_table(int16_t *sin)
>              sin[half_pi - i - step / 2] = (k * c + 0x8000) >> 16;
>          }
>      }
> -    /* Unshift amplitude */
> -    for (i = 0; i <= half_pi; i++)
> -        sin[i] = (sin[i] + (1 << (AMPLITUDE_SHIFT - 1))) >>
> AMPLITUDE_SHIFT;
> +
>      /* Use symmetries to fill the other three quarters */
>      for (i = 0; i < half_pi; i++)
>          sin[half_pi * 2 - i] = sin[i];
> --
> 2.35.1
>
> _______________________________________________
> 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".
>
_______________________________________________
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".