[FFmpeg-devel] [PATCH] G.729 initialization routine (skeleton)

[Vladimir Voroshilov]

2009/6/10 Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Jun 09, 2009 at 11:30:17PM +0700, Vladimir Voroshilov wrote:
>> 2009/6/9 Michael Niedermayer <michaelni at gmx.at>:
>> > On Sun, Jun 07, 2009 at 12:28:37AM +0700, Vladimir Voroshilov wrote:
>> >> 2009/6/7 Diego Biurrun <diego at biurrun.de>:
>> >> > On Sun, Jun 07, 2009 at 12:18:10AM +0700, Vladimir Voroshilov wrote:
>> >> >>
>> >> >> Here is patch without format.
>> >> >>
>> >> >> --- ffmpeg-r19125.orig/libavcodec/g729dec.c
>> >> >> +++ ffmpeg-r19125.mod/libavcodec/g729dec.c
>> >> >> @@ -97,11 +101,39 @@ static inline int get_parity(uint8_t value)
>> >> >>
>> >> >> +static int decoder_init(AVCodecContext * avctx)
>> >> >
>> >> > This function should likely be marked av_cold.
>> >> >
>> >> > Please doublecheck which other functions could receive this attribute.
>> >> >
>> >>
>> >> decoder_init does not call other internal routines, thus no additional
>> >> routines require this attribute, imho.
>> >> fixed.
>> > [...]
>> >
>> >> + ? ?if (avctx->sample_rate == 8000) {
>> >> + ? ? ? ?ctx->subframe_size = 40;
>> >> +#ifdef G729_SUPPORT_4400
>> >> + ? ?} else if (avctx->sample_rate == 4400) {
>> >> + ? ? ? ?ctx->subframe_size = 44;
>> >> +#endif
>> >> + ? ?} else {
>> >> + ? ? ? ?av_log(avctx, AV_LOG_ERROR, "Sample rate %d is not supported.\n", avctx->sample_rate);
>> >> + ? ? ? ?return AVERROR_NOFMT;
>> >> + ? ?}
>> >
>> > ? ?ctx->subframe_size = 40;
>> > #ifdef G729_SUPPORT_4400
>> > ? ?if (avctx->sample_rate == 4400) {
>> > ? ? ? ?ctx->subframe_size = 44;
>> > ? ?}
>> > #endif
>>
>> In this casse you'll get garbage sound instead of meanfull error
>> message for unsupported g.729 modes.
>
> which mode?

G.729 standard also describes 11.5k and 6.4k modes (and also several
non-standard modes exists, like above 4.4k).
So what will be happend if such mode will be passed to decoder ?


>
>
>>
>> >
>> >
>> >> +
>> >> ? ? ?if (avctx->channels != 1) {
>> >> ? ? ? ? ?av_log(avctx, AV_LOG_ERROR, "Only mono sound is supported (requested channels: %d).\n", avctx->channels);
>> >> ? ? ? ? ?return AVERROR_NOFMT;
>> >> ? ? ?}
>> >>
>> >
>> >> + ? ?avctx->frame_size = ctx->subframe_size << 1;
>> >
>> > if that is so, why is subframe_size needed at all? avctx->frame_size could be
>> > used as well
>> >
>>
>> subframe_size is used often, I am trying to avoid ">>1" in every routine.
>
> i cannot comment on code that i dont see
> you can introduce this variable once its existence simplifies code
> currently it does not
>
> [...]
>
> --
> Michael ? ? GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iD8DBQFKLpihYR7HhwQLD6sRAkfNAKCFOyzd7Rm55NOICXMjeViGYNbxWACeISGj
> rxMVfuOVWxqRT5vJG7Vpews=
> =+hTo
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>



-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719