[FFmpeg-devel] [PATCH] G.722 decoder

[Michael Niedermayer]

On Wed, Sep 08, 2010 at 09:18:21AM +0300, Martin Storsj? wrote:
> On Wed, 8 Sep 2010, Stefano Sabatini wrote:
> 
> > On date Wednesday 2010-09-08 00:13:07 +0300, Martin Storsj? encoded:
> > [...]
> > > +AVCodec adpcm_g722_decoder = {
> > > +    .name           = "g722",
> > 
> > > +    .type           = CODEC_TYPE_AUDIO,
> > 
> > AVMEDIA_TYPE_AUDIO
> 
> Hmm, I thought I changed this already. Thanks for pointing it out. Updated 
> patch attached.
[...]
> +static void do_adaptive_prediction(struct G722Band *band, const int cur_diff)
> +{
> +    int sg[2], limit, i, cur_qtzd_reconst;
> +
> +    const int cur_part_reconst = band->s_zero + cur_diff < 0;
> +
> +    sg[0] = sign_lookup[cur_part_reconst != band->part_reconst_mem[0]];
> +    sg[1] = sign_lookup[cur_part_reconst == band->part_reconst_mem[1]];
> +    band->part_reconst_mem[1] = band->part_reconst_mem[0];
> +    band->part_reconst_mem[0] = cur_part_reconst;
> +
> +    band->pole_mem[1] = av_clip((sg[0] * av_clip(band->pole_mem[0], -8191, 8191) >> 5) +
> +                                (sg[1] << 7) + (band->pole_mem[1] * 127 >> 7), -12288, 12288);
> +
> +    limit = 15360 - band->pole_mem[1];
> +    band->pole_mem[0] = av_clip(-192 * sg[0] + (band->pole_mem[0] * 255 >> 8), -limit, limit);
> +
> +
> +    if (cur_diff) {
> +        for (i = 0; i < 6; i++)
> +            band->zero_mem[i] = ((band->zero_mem[i]*255) >> 8) +
> +                                ((band->diff_mem[i]^cur_diff) < 0 ? -128 : 128);
> +    } else

> +        for (i = 0; i < 6; i++)
> +            band->zero_mem[i] = (band->zero_mem[i]*255) >> 8;

> +
> +    for (i = 5; i > 0; i--)
> +        band->diff_mem[i] = band->diff_mem[i-1];
> +    band->diff_mem[0] = av_clip_int16(cur_diff << 1);
> +
> +    band->s_zero = 0;
> +    for (i = 5; i >= 0; i--)
> +        band->s_zero += (band->zero_mem[i]*band->diff_mem[i]) >> 15;
> +
> +
> +    cur_qtzd_reconst = av_clip_int16((band->s_predictor + cur_diff) << 1);
> +    band->s_predictor = av_clip_int16(band->s_zero +
> +                                      (band->pole_mem[0] * cur_qtzd_reconst >> 15) +
> +                                      (band->pole_mem[1] * band->prev_qtzd_reconst >> 15));
> +    band->prev_qtzd_reconst = cur_qtzd_reconst;
> +}

somehow iam feeling this function should return s_predictor and not void


> +

> +static int inline linear_scale_factor(const int log_factor, int shift)
> +{
> +    const int wd1 = inv_log2_table[(log_factor >> 6) & 31];
> +    shift -= log_factor >> 11;
> +    return (shift < 0 ? wd1 << -shift : wd1 >> shift) << 2;
> +}

the shift parameter can be droped and instead a constant added to the
log_factor param for each call

also the <<2 can be removed, scale_factor be scaled down by 4 thus and a
few shifts readjusted in uses of it


[...]
> +static av_cold int g722_init(AVCodecContext * avctx)
> +{
> +    G722Context *c = avctx->priv_data;
> +
> +    if (avctx->channels != 1) {
> +        av_log(avctx, AV_LOG_ERROR, "Only mono tracks are allowed.\n");
> +        return AVERROR_INVALIDDATA;
> +    }
> +    avctx->sample_fmt = SAMPLE_FMT_S16;
> +
> +    switch (avctx->bit_rate) {
> +    case 64000:
> +    case 56000:
> +    case 48000:
> +        c->bits_per_sample = avctx->bit_rate/8000;
> +        break;

theres a problem here
bitrate is the bitrate of the bitstream in libavcodec
thus a lower bitrate stream + trash bits has 64000 too you cant distinguish
them like this


> +    default:
> +        if (avctx->strict_std_compliance >= FF_COMPLIANCE_STRICT) {
> +            av_log(avctx, AV_LOG_ERROR, "Unsupported bitrate [%d] only "
> +                                        "48Kb, 56Kb and 64Kb are supported.\n",
> +                                        avctx->bit_rate);
> +            return AVERROR_INVALIDDATA;
> +        }
> +        av_log(avctx, AV_LOG_WARNING, "Unsupported bitrate [%d], trying with "
> +                                      "64Kb.\n", avctx->bit_rate);
> +        avctx->bit_rate    = 64000;
> +        c->bits_per_sample = 8;
> +    }
> +

> +    if (avctx->sample_rate != 8000 && avctx->sample_rate != 16000) {
> +        av_log(avctx, AV_LOG_ERROR, "Unsupported sample rate [%d] only 8KHz "
> +                                    "and 16KHz are supported.\n",
> +                                    avctx->sample_rate);
> +        return AVERROR_INVALIDDATA;
> +    }

this looks lame, the code surely works too with 22khz or 44khz or anything
else, its just a stream of integers that are being compressed or decompressed
the samplerate is not essential
    
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100908/1841ca09/attachment.pgp>