Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init()
@ 2024-05-22  8:57 Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting Andreas Rheinhardt
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-22  8:57 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Fixes Coverity issue #1516804.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/af_atempo.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
index 0c36eb4dd7..1aedb82060 100644
--- a/libavfilter/af_atempo.c
+++ b/libavfilter/af_atempo.c
@@ -269,6 +269,7 @@ static int yae_reset(ATempoContext *atempo,
     uint32_t nlevels  = 0;
     float scale = 1.f, iscale = 1.f;
     uint32_t pot;
+    int ret;
     int i;
 
     atempo->format   = format;
@@ -300,16 +301,18 @@ static int yae_reset(ATempoContext *atempo,
     av_tx_uninit(&atempo->real_to_complex);
     av_tx_uninit(&atempo->complex_to_real);
 
-    av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn, AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
-    if (!atempo->real_to_complex) {
+    ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
+                     AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
+    if (ret < 0) {
         yae_release_buffers(atempo);
-        return AVERROR(ENOMEM);
+        return ret;
     }
 
-    av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn, AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
-    if (!atempo->complex_to_real) {
+    ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
+                     AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
+    if (ret < 0) {
         yae_release_buffers(atempo);
-        return AVERROR(ENOMEM);
+        return ret;
     }
 
     RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1), sizeof(AVComplexFloat));
-- 
2.40.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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting
  2024-05-22  8:57 [FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init() Andreas Rheinhardt
@ 2024-05-22  8:59 ` Andreas Rheinhardt
  2024-05-23  0:56   ` Pavel Koshevoy
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 3/5] avfilter/af_atempo: Fix indentation Andreas Rheinhardt
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-22  8:59 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

The earlier code distinguished between a partial reset
(yae_clear()) and a complete reset (yae_release_buffers()
which also releases the buffers); this separation existed
to avoid allocations, as buffers were reallocated on reconfigs.

Yet it is pointless since a5704659e3e41b7698812b89f67d9a7467a74d20,
so simply use yae_release_buffers() everywhere.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/af_atempo.c | 69 +++++++++++++++++++++--------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
index 1aedb82060..110f792eec 100644
--- a/libavfilter/af_atempo.c
+++ b/libavfilter/af_atempo.c
@@ -244,18 +244,6 @@ static void yae_release_buffers(ATempoContext *atempo)
     av_tx_uninit(&atempo->complex_to_real);
 }
 
-/* av_realloc is not aligned enough; fortunately, the data does not need to
- * be preserved */
-#define RE_MALLOC_OR_FAIL(field, field_size, element_size)      \
-    do {                                                        \
-        av_freep(&field);                                       \
-        field = av_calloc(field_size, element_size);            \
-        if (!field) {                                           \
-            yae_release_buffers(atempo);                        \
-            return AVERROR(ENOMEM);                             \
-        }                                                       \
-    } while (0)
-
 /**
  * Prepare filter for processing audio data of given format,
  * sample rate and number of channels.
@@ -289,40 +277,51 @@ static int yae_reset(ATempoContext *atempo,
         nlevels++;
     }
 
+    /* av_realloc is not aligned enough, so simply discard all the old buffers
+     * (fortunately, their data does not need to be preserved) */
+    yae_release_buffers(atempo);
+
     // initialize audio fragment buffers:
-    RE_MALLOC_OR_FAIL(atempo->frag[0].data, atempo->window, atempo->stride);
-    RE_MALLOC_OR_FAIL(atempo->frag[1].data, atempo->window, atempo->stride);
-    RE_MALLOC_OR_FAIL(atempo->frag[0].xdat_in, (atempo->window + 1), sizeof(AVComplexFloat));
-    RE_MALLOC_OR_FAIL(atempo->frag[1].xdat_in, (atempo->window + 1), sizeof(AVComplexFloat));
-    RE_MALLOC_OR_FAIL(atempo->frag[0].xdat, (atempo->window + 1), sizeof(AVComplexFloat));
-    RE_MALLOC_OR_FAIL(atempo->frag[1].xdat, (atempo->window + 1), sizeof(AVComplexFloat));
+    if (!(atempo->frag[0].data    = av_calloc(atempo->window, atempo->stride)) ||
+        !(atempo->frag[1].data    = av_calloc(atempo->window, atempo->stride)) ||
+        !(atempo->frag[0].xdat_in = av_calloc(atempo->window + 1, sizeof(AVComplexFloat))) ||
+        !(atempo->frag[1].xdat_in = av_calloc(atempo->window + 1, sizeof(AVComplexFloat))) ||
+        !(atempo->frag[0].xdat    = av_calloc(atempo->window + 1, sizeof(AVComplexFloat))) ||
+        !(atempo->frag[1].xdat    = av_calloc(atempo->window + 1, sizeof(AVComplexFloat)))) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
 
     // initialize rDFT contexts:
-    av_tx_uninit(&atempo->real_to_complex);
-    av_tx_uninit(&atempo->complex_to_real);
-
     ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
                      AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
-    if (ret < 0) {
-        yae_release_buffers(atempo);
-        return ret;
-    }
+    if (ret < 0)
+        goto fail;
 
     ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
                      AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
-    if (ret < 0) {
-        yae_release_buffers(atempo);
-        return ret;
-    }
+    if (ret < 0)
+        goto fail;
 
-    RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1), sizeof(AVComplexFloat));
-    RE_MALLOC_OR_FAIL(atempo->correlation, atempo->window, sizeof(AVComplexFloat));
+    if (!(atempo->correlation_in = av_calloc(atempo->window + 1, sizeof(AVComplexFloat))) ||
+        !(atempo->correlation    = av_calloc(atempo->window,     sizeof(AVComplexFloat)))) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
 
     atempo->ring = atempo->window * 3;
-    RE_MALLOC_OR_FAIL(atempo->buffer, atempo->ring, atempo->stride);
+    atempo->buffer = av_calloc(atempo->ring, atempo->stride);
+    if (!atempo->buffer) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
 
     // initialize the Hann window function:
-    RE_MALLOC_OR_FAIL(atempo->hann, atempo->window, sizeof(float));
+    atempo->hann = av_malloc_array(atempo->window, sizeof(float));
+    if (!atempo->hann) {
+        ret = AVERROR(ENOMEM);
+        goto fail;
+    }
 
     for (i = 0; i < atempo->window; i++) {
         double t = (double)i / (double)(atempo->window - 1);
@@ -330,8 +329,10 @@ static int yae_reset(ATempoContext *atempo,
         atempo->hann[i] = (float)h;
     }
 
-    yae_clear(atempo);
     return 0;
+fail:
+    yae_release_buffers(atempo);
+    return ret;
 }
 
 static int yae_update(AVFilterContext *ctx)
-- 
2.40.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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 3/5] avfilter/af_atempo: Fix indentation
  2024-05-22  8:57 [FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init() Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting Andreas Rheinhardt
@ 2024-05-22  8:59 ` Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 5/5] avformat/riffenc: Fix outdated comment Andreas Rheinhardt
  3 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-22  8:59 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Forgotten after b8f74ee57a6e9e75a507adcddf30f84dd4a39d39.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavfilter/af_atempo.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
index 110f792eec..3658348c45 100644
--- a/libavfilter/af_atempo.c
+++ b/libavfilter/af_atempo.c
@@ -998,20 +998,20 @@ static av_cold void uninit(AVFilterContext *ctx)
     yae_release_buffers(atempo);
 }
 
-    // WSOLA necessitates an internal sliding window ring buffer
-    // for incoming audio stream.
-    //
-    // Planar sample formats are too cumbersome to store in a ring buffer,
-    // therefore planar sample formats are not supported.
-    //
-    static const enum AVSampleFormat sample_fmts[] = {
-        AV_SAMPLE_FMT_U8,
-        AV_SAMPLE_FMT_S16,
-        AV_SAMPLE_FMT_S32,
-        AV_SAMPLE_FMT_FLT,
-        AV_SAMPLE_FMT_DBL,
-        AV_SAMPLE_FMT_NONE
-    };
+// WSOLA necessitates an internal sliding window ring buffer
+// for incoming audio stream.
+//
+// Planar sample formats are too cumbersome to store in a ring buffer,
+// therefore planar sample formats are not supported.
+//
+static const enum AVSampleFormat sample_fmts[] = {
+    AV_SAMPLE_FMT_U8,
+    AV_SAMPLE_FMT_S16,
+    AV_SAMPLE_FMT_S32,
+    AV_SAMPLE_FMT_FLT,
+    AV_SAMPLE_FMT_DBL,
+    AV_SAMPLE_FMT_NONE
+};
 
 static int config_props(AVFilterLink *inlink)
 {
-- 
2.40.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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure
  2024-05-22  8:57 [FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init() Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 3/5] avfilter/af_atempo: Fix indentation Andreas Rheinhardt
@ 2024-05-22  8:59 ` Andreas Rheinhardt
  2024-05-24  6:28   ` Andreas Rheinhardt
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 5/5] avformat/riffenc: Fix outdated comment Andreas Rheinhardt
  3 siblings, 1 reply; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-22  8:59 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Fixes Coverity issue #1506706.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/matroskaenc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 63bae9fe1c..76c542d50b 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -1250,7 +1250,9 @@ static int mkv_assemble_codecprivate(AVFormatContext *s, AVIOContext *dyn_cp,
             par->codec_tag = tag;
 
         /* Same comment as for ff_put_bmp_header applies here. */
-        ff_put_wav_header(s, dyn_cp, par, FF_PUT_WAV_HEADER_FORCE_WAVEFORMATEX);
+        ret = ff_put_wav_header(s, dyn_cp, par, FF_PUT_WAV_HEADER_FORCE_WAVEFORMATEX);
+        if (ret < 0)
+            return ret;
 #endif
     }
 
-- 
2.40.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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [FFmpeg-devel] [PATCH 5/5] avformat/riffenc: Fix outdated comment
  2024-05-22  8:57 [FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init() Andreas Rheinhardt
                   ` (2 preceding siblings ...)
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure Andreas Rheinhardt
@ 2024-05-22  8:59 ` Andreas Rheinhardt
  3 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-22  8:59 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/riffenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/riffenc.c b/libavformat/riffenc.c
index 8accb69541..59c9932c36 100644
--- a/libavformat/riffenc.c
+++ b/libavformat/riffenc.c
@@ -72,7 +72,7 @@ int ff_put_wav_header(AVFormatContext *s, AVIOContext *pb,
     }
 
     /* We use the known constant frame size for the codec if known, otherwise
-     * fall back on using AVCodecContext.frame_size, which is not as reliable
+     * fall back on using AVCodecParameters.frame_size, which is not as reliable
      * for indicating packet duration. */
     frame_size = av_get_audio_frame_duration2(par, par->block_align);
 
-- 
2.40.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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting Andreas Rheinhardt
@ 2024-05-23  0:56   ` Pavel Koshevoy
  2024-05-23  8:41     ` Andreas Rheinhardt
  0 siblings, 1 reply; 8+ messages in thread
From: Pavel Koshevoy @ 2024-05-23  0:56 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Wed, May 22, 2024, 02:59 Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> The earlier code distinguished between a partial reset
> (yae_clear()) and a complete reset (yae_release_buffers()
> which also releases the buffers); this separation existed
> to avoid allocations, as buffers were reallocated on reconfigs.
>
> Yet it is pointless since a5704659e3e41b7698812b89f67d9a7467a74d20,
> so simply use yae_release_buffers() everywhere.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavfilter/af_atempo.c | 69 +++++++++++++++++++++--------------------
>  1 file changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
> index 1aedb82060..110f792eec 100644
> --- a/libavfilter/af_atempo.c
> +++ b/libavfilter/af_atempo.c
> @@ -244,18 +244,6 @@ static void yae_release_buffers(ATempoContext *atempo)
>      av_tx_uninit(&atempo->complex_to_real);
>  }
>
> -/* av_realloc is not aligned enough; fortunately, the data does not need
> to
> - * be preserved */
> -#define RE_MALLOC_OR_FAIL(field, field_size, element_size)      \
> -    do {                                                        \
> -        av_freep(&field);                                       \
> -        field = av_calloc(field_size, element_size);            \
> -        if (!field) {                                           \
> -            yae_release_buffers(atempo);                        \
> -            return AVERROR(ENOMEM);                             \
> -        }                                                       \
> -    } while (0)
> -
>  /**
>   * Prepare filter for processing audio data of given format,
>   * sample rate and number of channels.
> @@ -289,40 +277,51 @@ static int yae_reset(ATempoContext *atempo,
>          nlevels++;
>      }
>
> +    /* av_realloc is not aligned enough, so simply discard all the old
> buffers
> +     * (fortunately, their data does not need to be preserved) */
> +    yae_release_buffers(atempo);
> +
>      // initialize audio fragment buffers:
> -    RE_MALLOC_OR_FAIL(atempo->frag[0].data, atempo->window,
> atempo->stride);
> -    RE_MALLOC_OR_FAIL(atempo->frag[1].data, atempo->window,
> atempo->stride);
> -    RE_MALLOC_OR_FAIL(atempo->frag[0].xdat_in, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -    RE_MALLOC_OR_FAIL(atempo->frag[1].xdat_in, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -    RE_MALLOC_OR_FAIL(atempo->frag[0].xdat, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -    RE_MALLOC_OR_FAIL(atempo->frag[1].xdat, (atempo->window + 1),
> sizeof(AVComplexFloat));
> +    if (!(atempo->frag[0].data    = av_calloc(atempo->window,
> atempo->stride)) ||
> +        !(atempo->frag[1].data    = av_calloc(atempo->window,
> atempo->stride)) ||
> +        !(atempo->frag[0].xdat_in = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +        !(atempo->frag[1].xdat_in = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +        !(atempo->frag[0].xdat    = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +        !(atempo->frag[1].xdat    = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat)))) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
>
>      // initialize rDFT contexts:
> -    av_tx_uninit(&atempo->real_to_complex);
> -    av_tx_uninit(&atempo->complex_to_real);
> -
>      ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
>                       AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
> -    if (ret < 0) {
> -        yae_release_buffers(atempo);
> -        return ret;
> -    }
> +    if (ret < 0)
> +        goto fail;
>
>      ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
>                       AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
> -    if (ret < 0) {
> -        yae_release_buffers(atempo);
> -        return ret;
> -    }
> +    if (ret < 0)
> +        goto fail;
>
> -    RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1),
> sizeof(AVComplexFloat));
> -    RE_MALLOC_OR_FAIL(atempo->correlation, atempo->window,
> sizeof(AVComplexFloat));
> +    if (!(atempo->correlation_in = av_calloc(atempo->window + 1,
> sizeof(AVComplexFloat))) ||
> +        !(atempo->correlation    = av_calloc(atempo->window,
>  sizeof(AVComplexFloat)))) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
>
>      atempo->ring = atempo->window * 3;
> -    RE_MALLOC_OR_FAIL(atempo->buffer, atempo->ring, atempo->stride);
> +    atempo->buffer = av_calloc(atempo->ring, atempo->stride);
> +    if (!atempo->buffer) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
>
>      // initialize the Hann window function:
> -    RE_MALLOC_OR_FAIL(atempo->hann, atempo->window, sizeof(float));
> +    atempo->hann = av_malloc_array(atempo->window, sizeof(float));
> +    if (!atempo->hann) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
>
>      for (i = 0; i < atempo->window; i++) {
>          double t = (double)i / (double)(atempo->window - 1);
> @@ -330,8 +329,10 @@ static int yae_reset(ATempoContext *atempo,
>          atempo->hann[i] = (float)h;
>      }
>
> -    yae_clear(atempo);
>      return 0;
> +fail:
> +    yae_release_buffers(atempo);
> +    return ret;
>  }
>
>  static int yae_update(AVFilterContext *ctx)
> --
> 2.40.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".
>


This doesn't feel like a necessary change (how frequently is the affected
code actually traversed, realistically?)... but since you've already spent
the effort to make this change I have no objection.

Pavel

>
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting
  2024-05-23  0:56   ` Pavel Koshevoy
@ 2024-05-23  8:41     ` Andreas Rheinhardt
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-23  8:41 UTC (permalink / raw)
  To: ffmpeg-devel

Pavel Koshevoy:
> On Wed, May 22, 2024, 02:59 Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
> 
>> The earlier code distinguished between a partial reset
>> (yae_clear()) and a complete reset (yae_release_buffers()
>> which also releases the buffers); this separation existed
>> to avoid allocations, as buffers were reallocated on reconfigs.
>>
>> Yet it is pointless since a5704659e3e41b7698812b89f67d9a7467a74d20,
>> so simply use yae_release_buffers() everywhere.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>  libavfilter/af_atempo.c | 69 +++++++++++++++++++++--------------------
>>  1 file changed, 35 insertions(+), 34 deletions(-)
>>
>> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c
>> index 1aedb82060..110f792eec 100644
>> --- a/libavfilter/af_atempo.c
>> +++ b/libavfilter/af_atempo.c
>> @@ -244,18 +244,6 @@ static void yae_release_buffers(ATempoContext *atempo)
>>      av_tx_uninit(&atempo->complex_to_real);
>>  }
>>
>> -/* av_realloc is not aligned enough; fortunately, the data does not need
>> to
>> - * be preserved */
>> -#define RE_MALLOC_OR_FAIL(field, field_size, element_size)      \
>> -    do {                                                        \
>> -        av_freep(&field);                                       \
>> -        field = av_calloc(field_size, element_size);            \
>> -        if (!field) {                                           \
>> -            yae_release_buffers(atempo);                        \
>> -            return AVERROR(ENOMEM);                             \
>> -        }                                                       \
>> -    } while (0)
>> -
>>  /**
>>   * Prepare filter for processing audio data of given format,
>>   * sample rate and number of channels.
>> @@ -289,40 +277,51 @@ static int yae_reset(ATempoContext *atempo,
>>          nlevels++;
>>      }
>>
>> +    /* av_realloc is not aligned enough, so simply discard all the old
>> buffers
>> +     * (fortunately, their data does not need to be preserved) */
>> +    yae_release_buffers(atempo);
>> +
>>      // initialize audio fragment buffers:
>> -    RE_MALLOC_OR_FAIL(atempo->frag[0].data, atempo->window,
>> atempo->stride);
>> -    RE_MALLOC_OR_FAIL(atempo->frag[1].data, atempo->window,
>> atempo->stride);
>> -    RE_MALLOC_OR_FAIL(atempo->frag[0].xdat_in, (atempo->window + 1),
>> sizeof(AVComplexFloat));
>> -    RE_MALLOC_OR_FAIL(atempo->frag[1].xdat_in, (atempo->window + 1),
>> sizeof(AVComplexFloat));
>> -    RE_MALLOC_OR_FAIL(atempo->frag[0].xdat, (atempo->window + 1),
>> sizeof(AVComplexFloat));
>> -    RE_MALLOC_OR_FAIL(atempo->frag[1].xdat, (atempo->window + 1),
>> sizeof(AVComplexFloat));
>> +    if (!(atempo->frag[0].data    = av_calloc(atempo->window,
>> atempo->stride)) ||
>> +        !(atempo->frag[1].data    = av_calloc(atempo->window,
>> atempo->stride)) ||
>> +        !(atempo->frag[0].xdat_in = av_calloc(atempo->window + 1,
>> sizeof(AVComplexFloat))) ||
>> +        !(atempo->frag[1].xdat_in = av_calloc(atempo->window + 1,
>> sizeof(AVComplexFloat))) ||
>> +        !(atempo->frag[0].xdat    = av_calloc(atempo->window + 1,
>> sizeof(AVComplexFloat))) ||
>> +        !(atempo->frag[1].xdat    = av_calloc(atempo->window + 1,
>> sizeof(AVComplexFloat)))) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>>
>>      // initialize rDFT contexts:
>> -    av_tx_uninit(&atempo->real_to_complex);
>> -    av_tx_uninit(&atempo->complex_to_real);
>> -
>>      ret = av_tx_init(&atempo->real_to_complex, &atempo->r2c_fn,
>>                       AV_TX_FLOAT_RDFT, 0, 1 << (nlevels + 1), &scale, 0);
>> -    if (ret < 0) {
>> -        yae_release_buffers(atempo);
>> -        return ret;
>> -    }
>> +    if (ret < 0)
>> +        goto fail;
>>
>>      ret = av_tx_init(&atempo->complex_to_real, &atempo->c2r_fn,
>>                       AV_TX_FLOAT_RDFT, 1, 1 << (nlevels + 1), &iscale, 0);
>> -    if (ret < 0) {
>> -        yae_release_buffers(atempo);
>> -        return ret;
>> -    }
>> +    if (ret < 0)
>> +        goto fail;
>>
>> -    RE_MALLOC_OR_FAIL(atempo->correlation_in, (atempo->window + 1),
>> sizeof(AVComplexFloat));
>> -    RE_MALLOC_OR_FAIL(atempo->correlation, atempo->window,
>> sizeof(AVComplexFloat));
>> +    if (!(atempo->correlation_in = av_calloc(atempo->window + 1,
>> sizeof(AVComplexFloat))) ||
>> +        !(atempo->correlation    = av_calloc(atempo->window,
>>  sizeof(AVComplexFloat)))) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>>
>>      atempo->ring = atempo->window * 3;
>> -    RE_MALLOC_OR_FAIL(atempo->buffer, atempo->ring, atempo->stride);
>> +    atempo->buffer = av_calloc(atempo->ring, atempo->stride);
>> +    if (!atempo->buffer) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>>
>>      // initialize the Hann window function:
>> -    RE_MALLOC_OR_FAIL(atempo->hann, atempo->window, sizeof(float));
>> +    atempo->hann = av_malloc_array(atempo->window, sizeof(float));
>> +    if (!atempo->hann) {
>> +        ret = AVERROR(ENOMEM);
>> +        goto fail;
>> +    }
>>
>>      for (i = 0; i < atempo->window; i++) {
>>          double t = (double)i / (double)(atempo->window - 1);
>> @@ -330,8 +329,10 @@ static int yae_reset(ATempoContext *atempo,
>>          atempo->hann[i] = (float)h;
>>      }
>>
>> -    yae_clear(atempo);
>>      return 0;
>> +fail:
>> +    yae_release_buffers(atempo);
>> +    return ret;
>>  }
>>
>>  static int yae_update(AVFilterContext *ctx)
>> --
>> 2.40.1
>>
> 
> 
> This doesn't feel like a necessary change (how frequently is the affected
> code actually traversed, realistically?)... but since you've already spent
> the effort to make this change I have no objection.

1. Looking at ff_filter_config_links(), it seems like the answer is at
most once; yet IIRC it is intended for filtergraph reconfigurations to
become a thing one day and given that the old code already supported it,
I kept supporting it.
2. I disagree with your point that code that is executed only
infrequently need not be optimized. Optimizations are good if they
enhance clarity or reduce codesize; IMO this patch does both.

- Andreas

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure
  2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure Andreas Rheinhardt
@ 2024-05-24  6:28   ` Andreas Rheinhardt
  0 siblings, 0 replies; 8+ messages in thread
From: Andreas Rheinhardt @ 2024-05-24  6:28 UTC (permalink / raw)
  To: ffmpeg-devel

Andreas Rheinhardt:
> Fixes Coverity issue #1506706.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavformat/matroskaenc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 63bae9fe1c..76c542d50b 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1250,7 +1250,9 @@ static int mkv_assemble_codecprivate(AVFormatContext *s, AVIOContext *dyn_cp,
>              par->codec_tag = tag;
>  
>          /* Same comment as for ff_put_bmp_header applies here. */
> -        ff_put_wav_header(s, dyn_cp, par, FF_PUT_WAV_HEADER_FORCE_WAVEFORMATEX);
> +        ret = ff_put_wav_header(s, dyn_cp, par, FF_PUT_WAV_HEADER_FORCE_WAVEFORMATEX);
> +        if (ret < 0)
> +            return ret;
>  #endif
>      }
>  

Will apply the rest of this patchset tomorrow unless there are objections.

- Andreas

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-05-24  6:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-22  8:57 [FFmpeg-devel] [PATCH 1/5] avfilter/af_atempo: Properly check av_tx_init() Andreas Rheinhardt
2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 2/5] avfilter/af_atempo: Simplify resetting Andreas Rheinhardt
2024-05-23  0:56   ` Pavel Koshevoy
2024-05-23  8:41     ` Andreas Rheinhardt
2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 3/5] avfilter/af_atempo: Fix indentation Andreas Rheinhardt
2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: Check ff_put_wav_header() failure Andreas Rheinhardt
2024-05-24  6:28   ` Andreas Rheinhardt
2024-05-22  8:59 ` [FFmpeg-devel] [PATCH 5/5] avformat/riffenc: Fix outdated comment Andreas Rheinhardt

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git