[FFmpeg-devel] [PATCH] G.722 decoder
Martin Storsjö
martin
Wed Sep 8 15:38:36 CEST 2010
On Wed, 8 Sep 2010, Michael Niedermayer wrote:
> On Wed, Sep 08, 2010 at 09:18:21AM +0300, Martin Storsj? wrote:
> > +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
Yes, that would perhaps be logical, however, in this codec, the updated
predictor value isn't used until decoding the next sample, due to the
scalable bitrate design (where different inv quant tables are used for
decoding the actual samples, but the one same lower precision one always
is used for updating the predictor internally, regardless which way it is
decoded). So making this return the predictor wouldn't really help
anything in this implementation.
> > +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
Yes, dropped this parameter.
> also the <<2 can be removed, scale_factor be scaled down by 4 thus and a
> few shifts readjusted in uses of it
Hmm, I'm not sure I'm following you here. Do you mean that I should
premultiply inv_log2_table by 4 and skip the final right shift? Or just
add/subtract 2 from the 'shift' variable? I tried that (the former, but
the latter would have the same effect), but it doesn't give bitexact
output, since the codec relies on truncating the values when doing the
right shift.
> > +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
Yes, this is a problem. Do you have any other suggestion for how to signal
the lower bitrate variants? Would AVCodecContext->bits_per_coded_sample
perhaps be suitable for this?
> > + 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
Yes, that's true. The codec itself is only specified for 16 kHz, but
there's nothing stopping the user from using it on any data, as you said.
This restriction was earlier used to be able to decode only the lower
subband if 8 kHz was chosen, but I'm removing that function for now - if
it is desired, I guess it should be signalled in some other way.
Updated patch attached.
// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-G.722-decoder-patch.patch
Type: text/x-diff
Size: 14526 bytes
Desc:
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100908/8e8c2aa6/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-G.722-muxer-demuxer.patch
Type: text/x-diff
Size: 2874 bytes
Desc:
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100908/8e8c2aa6/attachment-0001.patch>
More information about the ffmpeg-devel
mailing list