[FFmpeg-devel] [PATCH] g722 decoder, no licensing fame

[Michael Niedermayer]

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.


> +    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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090404/1cdb61f8/attachment.pgp>