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

[Michael Niedermayer]

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?


> 
> >
> >
> >> +
> >> ? ? ?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
-------------- 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/20090609/d81531b0/attachment.pgp>