Hello,
On Jul 20, 2013 12:26 AM, "Diego Biurrun" <di...@biurrun.de> wrote:
>
> ---
>
> I removed parts of the changes to twinvq_data.h, as adding 4 spaces of
> indentation to all those tables makes for a 1.5MB patch. I kid you not ..
If what you didn't like about twinvq_data.h is that the huge tables are not
indented in the beginning of the lines, this was done deliberately: the
file was already huge enough without that, and making it even bigger for an
arguably gain in readability looked a bad idea.
> /**
> @@ -363,8 +367,7 @@ static void dequant(TwinContext *tctx, GetBitContext
*gb, float *out,
> enum FrameType ftype,
> const int16_t *cb0, const int16_t *cb1, int cb_len)
> {
> - int pos = 0;
> - int i, j;
> + int i, j, pos = 0;
I personally don't like merging multiple declarations with initialization.
This goes a little beyond k&r.
>
> +static const uint8_t wtype_to_wsize[] = { 0, 0, 2, 2, 2, 1, 0, 1, 1 };
> +
Why?
> av_cold int twin_decode_init(AVCodecContext *avctx)
> return AVERROR_INVALIDDATA;
> }
> switch (isampf) {
> - case 44: avctx->sample_rate = 44100; break;
> - case 22: avctx->sample_rate = 22050; break;
> - case 11: avctx->sample_rate = 11025; break;
> - default: avctx->sample_rate = isampf * 1000; break;
> + case 44:
> + avctx->sample_rate = 44100;
> + break;
> + case 22:
> + avctx->sample_rate = 22050;
> + break;
> + case 11:
> + avctx->sample_rate = 11025;
> + break;
> + default:
> + avctx->sample_rate = isampf * 1000;
> + break;
> }
I think readability is more important here than consistency with the rest
of the project, and I find the original more readable.
>
> if (avctx->channels <= 0 || avctx->channels > CHANNELS_MAX) {
> @@ -1134,23 +1143,43 @@ static av_cold int
twin_decode_init(AVCodecContext *avctx)
> avctx->channels);
> return -1;
> }
> - avctx->channel_layout = avctx->channels == 1 ? AV_CH_LAYOUT_MONO :
> - AV_CH_LAYOUT_STEREO;
> + avctx->channel_layout = avctx->channels == 1 ? AV_CH_LAYOUT_MONO
> + : AV_CH_LAYOUT_STEREO;
>
> ibps = avctx->bit_rate / (1000 * avctx->channels);
>
> - switch ((isampf << 8) + ibps) {
> - case (8 <<8) + 8: tctx->mtab = &mode_08_08; break;
> - case (11<<8) + 8: tctx->mtab = &mode_11_08; break;
> - case (11<<8) + 10: tctx->mtab = &mode_11_10; break;
> - case (16<<8) + 16: tctx->mtab = &mode_16_16; break;
> - case (22<<8) + 20: tctx->mtab = &mode_22_20; break;
> - case (22<<8) + 24: tctx->mtab = &mode_22_24; break;
> - case (22<<8) + 32: tctx->mtab = &mode_22_32; break;
> - case (44<<8) + 40: tctx->mtab = &mode_44_40; break;
> - case (44<<8) + 48: tctx->mtab = &mode_44_48; break;
> + switch ((isampf << 8) + ibps) {
> + case (8 << 8) + 8:
> + tctx->mtab = &mode_08_08;
> + break;
> + case (11 << 8) + 8:
> + tctx->mtab = &mode_11_08;
> + break;
> + case (11 << 8) + 10:
> + tctx->mtab = &mode_11_10;
> + break;
> + case (16 << 8) + 16:
> + tctx->mtab = &mode_16_16;
> + break;
> + case (22 << 8) + 20:
> + tctx->mtab = &mode_22_20;
> + break;
> + case (22 << 8) + 24:
> + tctx->mtab = &mode_22_24;
> + break;
> + case (22 << 8) + 32:
> + tctx->mtab = &mode_22_32;
> + break;
> + case (44 << 8) + 40:
> + tctx->mtab = &mode_44_40;
> + break;
> + case (44 << 8) + 48:
> + tctx->mtab = &mode_44_48;
> + break;
Ditto.
> -
> static const uint8_t tab7[][35] = {
> -
{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,1,0,0,0},
> -
{0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0},
> -
{0,0,0,1,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0},
> -
{0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0},
> -
{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,1,0,0,0},
> -
{0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0},
> -
{0,0,0,1,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0},
> -
{0,0,0,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0,0},
> -
{0,0,1,0,1,0,0,1,0,1,0,0,1,0,1,0,0,1,0,1,0,0,1,0,1,0,0,1,0,1,0,0,1,0,1},
> -
{0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1,0,0,0,0,0,0,0,0},
> -
{0,0,0,0,0,0,1,0,0,1,0,0,0,0,0,0,0,0,0,0,0,1,0,0,1,0,0,0,0,0,0,0,0,0,0}
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1,
> + 0, 0, 0 },
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
> + 0, 0, 0 },
> + { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0 },
> + { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0 },
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1,
> + 0, 0, 0 },
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1,
> + 0, 0, 0 },
> + { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0 },
> + { 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0 },
> + { 0, 0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 0, 1, 0, 1, 0,
> + 0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 0, 1, 0, 1, 0, 0,
> + 1, 0, 1 },
> + { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0,
> + 0, 0, 0 },
> + { 0, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0,
> + 0, 0, 0 }
> };
Here and in a few similar ones that follows also I think it was more
readable before.
The rest looks pretty good.
-Vitor
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
