[FFmpeg-devel] [PATCH] G722 decoder

[Kenan Gillet]

Hi,

On Mar 21, 2009, at 1:34 PM, Michael Niedermayer wrote:

> On Sat, Mar 21, 2009 at 12:43:35AM -0700, Kenan Gillet wrote:
>> Hi,
>>
>> Here is an implementation of a bitexact G.722 decoder.
>> It is based upon the patch G722_decoder_by_Chas_Williams at [1]
>> and has been tested against the reference implementation found in  
>> ITU-G.711 .
>>
>> A sample can be found at /MPlayer/incoming/g722decoder
>> In order to play the file, we need to be able force the bitrate from
>> the comment line  (patch #1)
>> and to use a specific demuxer (patch #2)
>>
>> So far I have only found 64kbps / 16Khz raw samples, but i tested the
>> other bitrate (48kbps, 56 kbps)
>> against the reference implementation
>>
>> you can test the uploaded sample with the command:
>> ffmpeg -ac 1 -ab 64000 -ar 16000 -i conf-adminmenu-162.g722  
>> output.wav
>>
>> Thanks,
>>
>> Kenan
>>
>> [1] http://wiki.multimedia.cx/index.php?title=Interesting_Patches#G722_decoder_by_Chas_Williams 
>> :
>
>> Index: ffmpeg.c
>> ===================================================================
>> --- ffmpeg.c	(revision 18096)
>> +++ ffmpeg.c	(working copy)
>> @@ -150,6 +150,7 @@
>>
>> static int intra_only = 0;
>> static int audio_sample_rate = 44100;
>> +static int audio_bit_rate;
>> static int64_t channel_layout = 0;
>> #define QSCALE_NONE -99999
>> static float audio_qscale = QSCALE_NONE;
>> @@ -2345,12 +2346,14 @@
>> static int opt_bitrate(const char *opt, const char *arg)
>> {
>>     int codec_type = opt[0]=='a' ? CODEC_TYPE_AUDIO :  
>> CODEC_TYPE_VIDEO;
>> +    int bit_rate;
>>
>>     opt_default(opt, arg);
>>
>> -    if (av_get_int(avcodec_opts[codec_type], "b", NULL) < 1000)
>> +    if ((bit_rate = av_get_int(avcodec_opts[codec_type], "b",  
>> NULL)) < 1000)
>>         fprintf(stderr, "WARNING: The bitrate parameter is set too  
>> low. It takes bits/s as argument, not kbits/s\n");
>> -
>> +    if (codec_type == CODEC_TYPE_AUDIO)
>> +        audio_bit_rate = bit_rate;
>>     return 0;
>> }
>>
>
> this looks wrong, bit_rate should be available from the AVCodecContext
> without this

it is only available to the output AVCodecContext, where it is needed  
for
the input AVCodecContext.
I need it for the input file since g722 samples are raw format with no
information about which sample rate are used.

>
>
>> @@ -2819,6 +2822,7 @@
>>     memset(ap, 0, sizeof(*ap));
>>     ap->prealloced_context = 1;
>>     ap->sample_rate = audio_sample_rate;
>> +    ap->bit_rate = audio_bit_rate;
>>     ap->channels = audio_channels;
>>     ap->time_base.den = frame_rate.num;
>>     ap->time_base.num = frame_rate.den;
>> @@ -2892,6 +2896,7 @@
>>             channel_layout = enc->channel_layout;
>>             audio_channels = enc->channels;
>>             audio_sample_rate = enc->sample_rate;
>> +            audio_bit_rate = enc->bit_rate;
>>             audio_sample_fmt = enc->sample_fmt;
>>             input_codecs[nb_icodecs++] =  
>> avcodec_find_decoder_by_name(audio_codec_name);
>>             if(audio_disable)
>> @@ -3211,6 +3216,7 @@
>>     }
>>     nb_ocodecs++;
>>     audio_enc->sample_rate = audio_sample_rate;
>> +    audio_enc->bit_rate = audio_bit_rate;
>>     audio_enc->time_base= (AVRational){1, audio_sample_rate};
>>     if (audio_language) {
>>         av_metadata_set(&st->metadata, "language", audio_language);
>
>> Index: libavformat/avformat.h
>> ===================================================================
>> --- libavformat/avformat.h	(revision 18096)
>> +++ libavformat/avformat.h	(working copy)
>> @@ -261,6 +261,7 @@
>>     enum CodecID video_codec_id;
>>     enum CodecID audio_codec_id;
>> #endif
>> +    int bit_rate;
>> } AVFormatParameters;
>>
>> //! Demuxer will use url_fopen, no opened file should be provided  
>> by the caller.
>
> the whole struct is pretty much deprecated and iam not happy if people
> add new things to it.

any hint on to "do it the right way" ?

>
> [...]
>> +/**
>> + * adpative preditor
>> + *
>> + * @note On x86 using the MULL macro in a loop is slower than not  
>> using the macro.
>> + */
>> +static void do_adaptive_prediction(struct G722Band *band, const  
>> int cur_diff)
>> +{
>> +    int sg[2], limit, i, cur_part_reconst;
>> +
>> +    band->qtzd_reconst_mem[1] = band->qtzd_reconst_mem[0];
>> +    band->qtzd_reconst_mem[0] = av_clip_int16((band->s_predictor +  
>> cur_diff) << 1);
>> +
>> +    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) + MULL(band- 
>> >pole_mem[1], 127, 7), -12288, 12288);
>> +
>> +    limit = 15360 - band->pole_mem[1];
>> +    band->pole_mem[0] = av_clip(-192 * sg[0] + MULL(band- 
>> >pole_mem[0], 255, 8), -limit, limit);
>
> x+= (-x)>>8
> coul be used instead of
> x= (x*255)>>8

do_adaptive_prediction is around 5% slower with x+= (-x)>>8 on Core 2  
Duo 2Ghz, gcc 4.2.1

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

shift - 2 can be negative


Kenan