2011/12/3 Måns Rullgård <[email protected]>:
> Nathan Caldwell <[email protected]> writes:
>> +if (cond) { \
>> +    av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \
>> +    return AVERROR(EINVAL); \
>> +}
>
> Please consider making this a do { } while (0).

Why would this be needed in this case?

Update attached.

-- 
-Nathan Caldwell
From 23344f7a0f093dabdb2ae37c36058a9e3145979e Mon Sep 17 00:00:00 2001
From: Nathan Caldwell <[email protected]>
Date: Fri, 2 Dec 2011 15:08:55 -0700
Subject: [PATCH] aacenc: aac_encode_init() cleanup
To: libav development <[email protected]>

Macroify sanity checks and check return values of allocs and other functions.
---
 libavcodec/aacenc.c |   84 +++++++++++++++++++++++++++++----------------------
 1 files changed, 48 insertions(+), 36 deletions(-)

diff --git a/libavcodec/aacenc.c b/libavcodec/aacenc.c
index b112fa8..8c597c7 100644
--- a/libavcodec/aacenc.c
+++ b/libavcodec/aacenc.c
@@ -46,6 +46,12 @@
 
 #define AAC_MAX_CHANNELS 6
 
+#define ERROR_IF(cond, ...) \
+    if (cond) { \
+        av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \
+        return AVERROR(EINVAL); \
+    }
+
 static const uint8_t swb_size_1024_96[] = {
     4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 8, 8, 8, 8, 8,
     12, 12, 12, 12, 12, 16, 16, 24, 28, 36, 44,
@@ -160,10 +166,24 @@ static void put_audio_specific_config(AVCodecContext *avctx)
     flush_put_bits(&pb);
 }
 
+static av_cold int aac_encode_end(AVCodecContext *avctx)
+{
+    AACEncContext *s = avctx->priv_data;
+    
+    ff_mdct_end(&s->mdct1024);
+    ff_mdct_end(&s->mdct128);
+    ff_psy_end(&s->psy);
+    if (s->psypp)
+        ff_psy_preprocess_end(s->psypp);
+    av_freep(&s->samples);
+    av_freep(&s->cpe);
+    return 0;
+}
+
 static av_cold int aac_encode_init(AVCodecContext *avctx)
 {
     AACEncContext *s = avctx->priv_data;
-    int i;
+    int i, ret = 0;
     const uint8_t *sizes[2];
     uint8_t grouping[AAC_MAX_CHANNELS];
     int lengths[2];
@@ -173,27 +193,25 @@ static av_cold int aac_encode_init(AVCodecContext *avctx)
     for (i = 0; i < 16; i++)
         if (avctx->sample_rate == avpriv_mpeg4audio_sample_rates[i])
             break;
-    if (i == 16) {
-        av_log(avctx, AV_LOG_ERROR, "Unsupported sample rate %d\n", avctx->sample_rate);
-        return -1;
-    }
-    if (avctx->channels > AAC_MAX_CHANNELS) {
-        av_log(avctx, AV_LOG_ERROR, "Unsupported number of channels: %d\n", avctx->channels);
-        return -1;
-    }
-    if (avctx->profile != FF_PROFILE_UNKNOWN && avctx->profile != FF_PROFILE_AAC_LOW) {
-        av_log(avctx, AV_LOG_ERROR, "Unsupported profile %d\n", avctx->profile);
-        return -1;
-    }
-    if (1024.0 * avctx->bit_rate / avctx->sample_rate > 6144 * avctx->channels) {
-        av_log(avctx, AV_LOG_ERROR, "Too many bits per frame requested\n");
-        return -1;
-    }
+
+    ERROR_IF(i == 16,
+             "Unsupported sample rate %d\n", avctx->sample_rate);
+    ERROR_IF(avctx->channels > AAC_MAX_CHANNELS,
+             "Unsupported number of channels: %d\n", avctx->channels);
+    ERROR_IF(avctx->profile != FF_PROFILE_UNKNOWN && avctx->profile != FF_PROFILE_AAC_LOW,
+             "Unsupported profile %d\n", avctx->profile);
+    ERROR_IF(1024.0 * avctx->bit_rate / avctx->sample_rate > 6144 * avctx->channels,
+             "Too many bits per frame requested\n");
+
     s->samplerate_index = i;
 
     dsputil_init(&s->dsp, avctx);
-    ff_mdct_init(&s->mdct1024, 11, 0, 1.0);
-    ff_mdct_init(&s->mdct128,   8, 0, 1.0);
+
+    if (ret = ff_mdct_init(&s->mdct1024, 11, 0, 1.0))
+        goto fail;
+    if (ret = ff_mdct_init(&s->mdct128,   8, 0, 1.0))
+        goto fail;
+    
     // window init
     ff_kbd_window_init(ff_aac_kbd_long_1024, 4.0, 1024);
     ff_kbd_window_init(ff_aac_kbd_short_128, 6.0, 128);
@@ -201,9 +219,10 @@ static av_cold int aac_encode_init(AVCodecContext *avctx)
     ff_init_ff_sine_windows(7);
 
     s->chan_map           = aac_chan_configs[avctx->channels-1];
-    s->samples            = av_malloc(2 * 1024 * avctx->channels * sizeof(s->samples[0]));
-    s->cpe                = av_mallocz(sizeof(ChannelElement) * s->chan_map[0]);
-    avctx->extradata      = av_mallocz(5 + FF_INPUT_BUFFER_PADDING_SIZE);
+    FF_ALLOC_OR_GOTO (avctx, s->samples, 2 * 1024 * avctx->channels * sizeof(s->samples[0]), alloc_fail);
+    FF_ALLOCZ_OR_GOTO(avctx, s->cpe, sizeof(ChannelElement) * s->chan_map[0], alloc_fail);
+    FF_ALLOCZ_OR_GOTO(avctx, avctx->extradata, 5 + FF_INPUT_BUFFER_PADDING_SIZE, alloc_fail);
+
     avctx->extradata_size = 5;
     put_audio_specific_config(avctx);
 
@@ -213,7 +232,8 @@ static av_cold int aac_encode_init(AVCodecContext *avctx)
     lengths[1] = ff_aac_num_swb_128[i];
     for (i = 0; i < s->chan_map[0]; i++)
         grouping[i] = s->chan_map[i + 1] == TYPE_CPE;
-    ff_psy_init(&s->psy, avctx, 2, sizes, lengths, s->chan_map[0], grouping);
+    if (ret = ff_psy_init(&s->psy, avctx, 2, sizes, lengths, s->chan_map[0], grouping))
+        goto fail;
     s->psypp = ff_psy_preprocess_init(avctx);
     s->coder = &ff_aac_coders[2];
 
@@ -222,6 +242,11 @@ static av_cold int aac_encode_init(AVCodecContext *avctx)
     ff_aac_tableinit();
 
     return 0;
+alloc_fail:
+    ret = AVERROR(ENOMEM);
+fail:
+    aac_encode_end(avctx);
+    return ret;
 }
 
 static void apply_window_and_mdct(AVCodecContext *avctx, AACEncContext *s,
@@ -653,19 +678,6 @@ static int aac_encode_frame(AVCodecContext *avctx,
     return put_bits_count(&s->pb)>>3;
 }
 
-static av_cold int aac_encode_end(AVCodecContext *avctx)
-{
-    AACEncContext *s = avctx->priv_data;
-
-    ff_mdct_end(&s->mdct1024);
-    ff_mdct_end(&s->mdct128);
-    ff_psy_end(&s->psy);
-    ff_psy_preprocess_end(s->psypp);
-    av_freep(&s->samples);
-    av_freep(&s->cpe);
-    return 0;
-}
-
 #define AACENC_FLAGS AV_OPT_FLAG_ENCODING_PARAM | AV_OPT_FLAG_AUDIO_PARAM
 static const AVOption aacenc_options[] = {
     {"stereo_mode", "Stereo coding method", offsetof(AACEncContext, options.stereo_mode), AV_OPT_TYPE_INT, {.dbl = 0}, -1, 1, AACENC_FLAGS, "stereo_mode"},
-- 
1.7.5.4

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to