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

[Michael Niedermayer]

On Wed, Jun 10, 2009 at 01:55:08AM +0700, Vladimir Voroshilov wrote:
> 2009/6/10 Michael Niedermayer <michaelni at gmx.at>:
> > On Wed, Jun 10, 2009 at 12:27:35AM +0700, Vladimir Voroshilov wrote:
> >> 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 ?
> >
> > as the values you quote are bitrates and the check is testing the
> > samplerate ...
> >
> 
> OMG! I've mixed bit_rate and sample_rate.
> 
> Well...
> First look give me (at least for 8k and 6k4. i'll finally drop out 4k4
> in next patch since i've no method to test it):
> avctx->sample_rate=8000
> avctx->bit_rate = 8000 and 6400 respectively
> avctx->frame_size = avctx->sample_rate /100 (1 packet = 10ms)
> 
> ctx->subframe_size=avctx->frame_size/2
> ctx->packed_frame_size = avctx-bit_rate / 800 (1 packet = 10ms and bits->bytes)
> ctx->unpacked_frame_size = avctx->frame_size * sizeof(int16_t)
> 
> No format tables at this stage are required (they will due to
> different bits allocation in packet)!
> I'll test and fix all these calculations in next patch, but i have
> several question now:
> 
> 1. Who should initialize sample_rate and frame_size (decoder or demuxer) ?

whoever is able to
the demuxer will set it if it can, the decoder should set it if there is a
sample_rate stored in the bitstream


> 2. Can i be sure that output buffer passed to decode_frame will be at
> least frame_size*2 long or i have to check this explicitly ?

if you set frame_size during init then you should not need an explicit
check, that said, this is one of the things i dont mind if done redundantly


> 
> 3. What should i pass instead of X, Y and Z to av_set_pts(st, X, Y, Z)
> in demuxer to ensure that all packets will be
> packets_frame_size long (10 and 8 bytes respectively) ? av_set_pts(st,
> 10, bit_rate, sample_rate) if all packets are 10ms ?

which demuxer ?


> Perhaps i should set up packet size in different way ?


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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/7045e057/attachment.pgp>