[FFmpeg-devel] [PATCH] g722 decoder, no licensing fame
Kenan Gillet
kenan.gillet
Sat Apr 4 20:46:31 CEST 2009
On Apr 3, 2009, at 9:42 PM, Michael Niedermayer wrote:
> On Tue, Mar 31, 2009 at 11:34:34PM -0700, Kenan Gillet wrote:
>> Hi
>>
>> On Thu, Mar 26, 2009 at 11:24 AM, Michael Niedermayer <michaelni at gmx.at
>> > wrote:
>>> On Wed, Mar 25, 2009 at 09:37:58PM -0700, Kenan Gillet wrote:
>>>> On Wed, Mar 25, 2009 at 2:39 PM, Michael Niedermayer <michaelni at gmx.at
>>>> > wrote:
>>>>> On Wed, Mar 25, 2009 at 09:57:51AM -0700, Kenan Gillet wrote:
>>>>>> 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:
>>>>> [...]
>>>>>>>
>>>>>>>
>>>>>>>> @@ -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" ?
>>>>>
>>>>> AVFormatContext.bit_rate maybe but i am not sure if this might
>>>>> cause some
>>>>> issues
>>>>>
>>>>
>>>> actually a much simpler solution would be to change the -ab
>>>> options to
>>>> set in the
>>>> AVCodecContext.bitrate for decoder too like in the patch attached
>>>>
>>>> but it would change the behavior of ffmpeg
>>>>
>>>> but that's exactly what is needed, to be able to give the g722
>>>> decoder
>>>> a bit_rate
>>>>
>>>> would that be acceptable?
>>>
>>> have you checked that this doesnt break transcoding with multiple
>>> inputs and
>>> outputs, i mean the -ab must not mistakely override the wrong one
>>
>> I tried with different samples and so far it works, but I am still
>> working on making
>> sure no regression happens by inspecting the audio decoder.
>>
>> In the meantime, the init function overwrites the
>> AVCodecContext.bit_rate to the
>> default g722 value of 64000 and issue a warning if the standard
>> compliance is not
>> set to strict.
>>
>> new patches attached
> [...]
>> +/**
>> + * adaptive predictor
>> + *
>> + * @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]];
>
> i dont see why a LUT should be used here, its not really more readable
> and i doubt its faster.
it is faster
on Core 2 Duo 2Ghz , gcc 4.2.1
LUT based:
testing for 64kb 16KHz: [OK][ 1574 dezicycles in
do_adaptive_prediction, 262130 runs, 14 skips ]
testing for 56kb 16KHz: [OK][ 1668 dezicycles in
do_adaptive_prediction, 262110 runs, 34 skips ]
testing for 48kb 16KHz: [OK][ 1607 dezicycles in
do_adaptive_prediction, 262112 runs, 32 skips ]
testing for 64Kb 8KHz: [OK][ 1584 dezicycles in
do_adaptive_prediction, 131066 runs, 6 skips ]
testing encoding for 64Kb 16KHz: [OK][ 1558 dezicycles in
do_adaptive_prediction, 262108 runs, 36 skips ]
non-LUT:
testing for 64kb 16KHz: [OK][ 1686 dezicycles in
do_adaptive_prediction, 262126 runs, 18 skips ]
testing for 56kb 16KHz: [OK][ 1719 dezicycles in
do_adaptive_prediction, 262107 runs, 37 skips ]
testing for 48kb 16KHz: [OK][ 1689 dezicycles in
do_adaptive_prediction, 262126 runs, 18 skips ]
testing for 64Kb 8KHz: [OK][ 1676 dezicycles in
do_adaptive_prediction, 131065 runs, 7 skips ]
testing encoding for 64Kb 16KHz: [OK][ 1673 dezicycles in
do_adaptive_prediction, 262116 runs, 28 skips ]
i can revert to non-LUT if you prefer
>> + 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);
>> +
>> +
>> + if(cur_diff) {
>> + for (i = 0; i < 6; i++)
>> + band->zero_mem[i] = ((band->zero_mem[i]*255) >> 8) +
>
>> + (band->diff_mem[i] >> 15 ==
>> cur_diff >> 15 ? 128 : -128);
>
> if this is testing the equality of the sign bit then
> (x^y) < 0
> might be faster
yes it is faster :)
before
testing for 64kb 16KHz: [OK][ 1651 dezicycles in
do_adaptive_prediction, 262104 runs, 40 skips ]
testing for 56kb 16KHz: [OK][ 1770 dezicycles in
do_adaptive_prediction, 262117 runs, 27 skips ]
testing for 48kb 16KHz: [OK][ 1683 dezicycles in
do_adaptive_prediction, 262118 runs, 26 skips ]
testing for 64Kb 8KHz: [OK][ 1660 dezicycles in
do_adaptive_prediction, 131064 runs, 8 skips ]
testing encoding for 64Kb 16KHz: [OK][ 1637 dezicycles in
do_adaptive_prediction, 262110 runs, 34 skips ]
after:
testing for 64kb 16KHz: [OK][ 1575 dezicycles in
do_adaptive_prediction, 262114 runs, 30 skips ]
testing for 56kb 16KHz: [OK][ 1668 dezicycles in
do_adaptive_prediction, 262116 runs, 28 skips ]
testing for 48kb 16KHz: [OK][ 1607 dezicycles in
do_adaptive_prediction, 262123 runs, 21 skips ]
testing for 64Kb 8KHz: [OK][ 1585 dezicycles in
do_adaptive_prediction, 131061 runs, 11 skips ]
testing encoding for 64Kb 16KHz: [OK][ 1558 dezicycles in
do_adaptive_prediction, 262124 runs, 20 skips ]
I also forgot to mention that the encoder and decoder passes the
official ITU test sequences
at http://www.itu.int/rec/T-REC-G.722-198703-I!AppII/en
thanks for the review
Kenan
More information about the ffmpeg-devel
mailing list