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.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
