On Wed, Jan 18, 2012 at 8:09 AM, Diego Biurrun <[email protected]> wrote: > On Wed, Jan 18, 2012 at 01:39:48AM -0700, Nathan Caldwell wrote: >> Macroify sanity checks and check return values of allocs and other functions. >> >> --- a/libavcodec/aacenc.c >> +++ b/libavcodec/aacenc.c >> @@ -46,6 +46,12 @@ >> >> +#define ERROR_IF(cond, ...) \ >> + if (cond) { \ >> + av_log(avctx, AV_LOG_ERROR, __VA_ARGS__); \ >> + return AVERROR(EINVAL); \ >> + } > > I don't like this much - why do you want to obscure basic C syntax? > >> @@ -173,37 +224,26 @@ 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"); > > We have code like the above in tons of places all over libav. > I'd just keep it. > > Note, however, that I don't want to insist too much on this. If you or > Alex like this sooooo much better, so be it. I just don't think it > helps readability, on the contrary.
I prefer this way, but it's not a sticking point. I don't see how it's much worse than FF_ALLOC_OR_GOTO(). -- -Nathan Caldwell _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
