* [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