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

[Michael Niedermayer]

On Sat, Jun 06, 2009 at 11:27:52PM +0700, Vladimir Voroshilov wrote:
> 2009/6/6 Michael Niedermayer <michaelni at gmx.at>:
> > On Sat, Jun 06, 2009 at 08:06:40PM +0700, Vladimir Voroshilov wrote:
> >> 2009/6/6 Michael Niedermayer <michaelni at gmx.at>:
> >> > On Sat, Jun 06, 2009 at 12:56:01PM +0700, Vladimir Voroshilov wrote:
> >> >> 2009/6/6 Diego Biurrun <diego at biurrun.de>:
> >> >> > On Sat, Jun 06, 2009 at 01:01:02AM +0200, Stefano Sabatini wrote:
> >> >> >> On date Friday 2009-06-05 14:24:39 +0700, Vladimir Voroshilov encoded:
> 
> [...]
> 
> >> >> diff --git ffmpeg-r19120.orig/libavcodec/g729dec.c ffmpeg-r19120.mod/libavcodec/g729dec.c
> >> >> index f224a37..dd55895 100644
> >> >> --- ffmpeg-r19120.orig/libavcodec/g729dec.c
> >> >> +++ ffmpeg-r19120.mod/libavcodec/g729dec.c
> >> >> @@ -81,6 +81,16 @@ typedef struct {
> >> >> ? ? ?int mr_energy;
> >> >> ?} G729FormatDescription;
> >> >>
> >> >
> >> >> +typedef enum {
> >> >> + ? ?FORMAT_G729_8K = 0,
> >> >> + ? ?FORMAT_G729_4K4,
> >> >> +} G729Formats;
> >> >
> >> > this can be replaced by direct sample_rate checks in the code
> >>
> >> You suggest using:
> >>
> >> ========================
> >> typedef G729Context{
> >> ? int mr_energy
> >> }
> >>
> >> decode()
> >> {
> >> ?some_routine(ctx->mr_energy);
> >> }
> >> init ()
> >> {
> >> ?if (sample_rate== 1)
> >> ? ctx->mr_energy = 1;
> >> ?else if (sample_rate == 2)
> >> ? ctx->mr_energy = 3;
> >> }
> >> =======================
> >
> > no 1 and 2 are no sample rates
> > your code did check AVCodecContext.sample_rate and then sets format
> > based on it and then checks format everywhere, you can check sample_rate
> > directly
> > please remove the intermediate step not rename it
> >
> 
> In other words i should pass AVCodecContext instead of G729Context
> when format check was required and check for
> avctx->sample_rate instead of ctx->sample_rate or ctx->format. Also i
> should use ctx->mr_energy (initialized in init routine once)
> instead of formats[ctx->format].mr_energy (the same for other
> G729FormatDescription members).
> 
> Am i right ?

i cant comment on how you should handle things in future patches, just that
in the current patch format is redundant.

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

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
-------------- 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/20090606/ec47190a/attachment.pgp>