[FFmpeg-devel] [PATCH] G.722 decoder

[Martin Storsjö]

On Sun, 5 Sep 2010, Michael Niedermayer wrote:

> On Fri, Aug 06, 2010 at 09:24:23PM +0300, Martin Storsj? wrote:
> > Hi,
> > 
> > The G.722 decoder patch that Kenan Gillet took a far way through the 
> > review process last year seemed to have stalled, I'm trying to pick it up 
> > and get it committed.
> > 
> > On Fri, 10 Apr 2009, Michael Niedermayer wrote:
> > 
> > > On Thu, Apr 09, 2009 at 08:14:58AM -0700, Kenan Gillet wrote:
> > > [...]
> > > > + * @note For the 56000bps and 48000bps bitrates, the respectively 7 and 6 bits
> > > > + *       codeword might be packed, so unpacking might be needed either
> > > > + *       internally or as a separate parser.
> > > 
> > > do non packed cases exist anywhere in the wild?
> > > do packed cases exist anywhere in the wild?
> > > 
> > > it seems to me that filling 30% of the bits with 0 is not particlarely
> > > likely in a real world environment
> > 
> > According to the spec, the point of the lower bitrate versions is to be 
> > able to use 1 or 2 bits per byte as an auxillary data channel. The encoder 
> > always outputs the 64 kbps mode, but you can choose to overwrite the 
> > lowest two bits if you want to.
> > 
> > The decoder is able to cope with these bits being overwritten with random 
> > bits, but you get slightly better results if you tell the decoder that 
> > this has happened and so it should disregard those bits.
> > 
> > With that in mind, as far as I can interpret the spec, it's the lowest 
> > bits that should be skipped for those modes, but the source for this 
> > decoder seems to think otherwise. The implementation in spandsp (that 
> > has some common heritage with this code base, iirc, does things this 
> > way, too.
> 
> this does not awnser my question
> 
> do non packed cases exist anywhere in the wild?

Not that I know of. The only place in the wild where I know G.722 is used 
at all is within RTP, and that uses the full bitrate version.

> do packed cases exist anywhere in the wild?

Not that I know of.

> this matters because the packed case is usefull due to lower bitrate but its
> not supported while the non packed case is supported in a way that you say
> doesnt comply to the spec and i cant see anyone using it except by mistake
> or "because some spec says so"

Yes, I agree. I changed the non-packed mode to comply with the spec, which 
to me seems to be the sanest thing to do until someone can point to a 
proper in the wild example of something else in some container.

> [...]
> > +static const int16_t low_inv_quant5[32] = {
> > +     -280,   -280, -23352, -17560, -14120, -11664, -9752, -8184,
> > +    -6864,  -5712,  -4696,  -3784,  -2960,  -2208, -1520,  -880,
> > +    23352,  17560,  14120,  11664,   9752,   8184,  6864,  5712,
> > +     4696,   3784,   2960,   2208,   1520,    880,   280,  -280
> > +};
> > +static const int16_t low_inv_quant6[64] = {
> > +      -136,   -136,   -136,   -136, -24808, -21904, -19008, -16704,
> > +    -14984, -13512, -12280, -11192, -10232,  -9360,  -8576,  -7856,
> > +     -7192,  -6576,  -6000,  -5456,  -4944,  -4464,  -4008,  -3576,
> > +     -3168,  -2776,  -2400,  -2032,  -1688,  -1360,  -1040,   -728,
> > +     24808,  21904,  19008,  16704,  14984,  13512,  12280,  11192,
> > +     10232,   9360,   8576,   7856,   7192,   6576,   6000,   5456,
> > +      4944,   4464,   4008,   3576,   3168,   2776,   2400,   2032,
> > +      1688,   1360,   1040,    728,    432,    136,   -432,   -136
> > +};
> > +
> > +static const int16_t *low_inv_quants[3] = { low_inv_quant6, low_inv_quant5,
> > +                                 low_inv_quant4 };
> 
> these tables possible can be divided by 2^C and some >> reduced

Yes, divided all inv_quant tables by 8 and adjusted the shifting.

New patches attached.

// Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-G.722-decoder-patch.patch
Type: text/x-diff
Size: 15206 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100906/36e4a790/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-G.722-muxer-demuxer.patch
Type: text/x-diff
Size: 2872 bytes
Desc: 
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100906/36e4a790/attachment-0001.patch>