[FFmpeg-devel] [PATCH] Some ra144.c simplifications

Michael Niedermayer michaelni
Wed Jun 25 00:03:00 CEST 2008


On Tue, Jun 24, 2008 at 11:35:12PM +0200, Vitor Sessak wrote:
> Michael Niedermayer wrote:
> > On Sat, Jun 21, 2008 at 07:53:09AM +0200, Vitor Sessak wrote:
> >> Michael Niedermayer wrote:
> >>> On Wed, Jun 04, 2008 at 08:18:10PM +0200, Vitor Sessak wrote:
> >>>> Michael Niedermayer wrote:
> >>>>> On Wed, May 28, 2008 at 09:23:02PM +0200, Vitor Sessak wrote:
> >>>>>> Michael Niedermayer wrote:
> >>>>>>> On Wed, May 28, 2008 at 06:56:45PM +0200, Vitor Sessak wrote:
> >>>>>>>> Michael Niedermayer wrote:
> >>>>>>>>> On Tue, May 27, 2008 at 09:16:09PM +0200, Vitor Sessak wrote:
> >>>>>>>>>> Michael Niedermayer wrote:
> >>>>>>>>>>> On Sun, May 25, 2008 at 07:11:52PM +0200, Vitor Sessak wrote:
> >>>>>>>>>>>> Michael Niedermayer wrote:
> >>>>>>>>>>>>> On Sun, May 25, 2008 at 06:05:15PM +0200, Vitor Sessak wrote:
> >>>>>>>>>>>>> [...]
> >>>>>>>>>>>>>>>> ok
> >>>>>>>>>>>>>>> One more...
> >>>>>>>>>>>>>> ... and some more cleanup:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> ra144_vector_add_wav.diff: Make add_wav() receive a vector 
> >>>>>>>>>>>>>> instead of three integers
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> ra144_params_dec2.diff: Do not calculate anything based in l, 
> >>>>>>>>>>>>>> it is unrolled in the loop anyway
> >>>>>>>>>>>>> ok
> >>>>>>>>>>>> Now s/(unsigned) short/(u)int16_t.
> >>>>>>>>>>> ok
> >>>>>>>>>> Next one. dec2() interpolates the block coefficients from the 
> >>>>>>>>>> previous one and fall back to a block-dependent schema if the 
> >>>>>>>>>> interpolation results in an unstable filter...
> >>>>>>>>> [...]
> >>>>>>>>>> +    // Interpolate block coefficients from the this frame forth 
> >>>>>>>>>> block and
> >>>>>>>>>> +    // last frame forth block
> >>>>>>>>>>      for (x=0; x<30; x++)
> >>>>>>>>>> -        decsp[x] = (a * inp[x] + b * inp2[x]) >> 2;
> >>>>>>>>>> +        decsp[x] = (a * ractx->lpc_coef[x] + b * 
> >>>>>>>>>> ractx->lpc_coef_old[x])>> 2;
> >>>>>>>>> ff_acelp_weighted_vector_sum()
> >>>>>>>> Ok, but to do that I need to use int16_t. So I propose to apply my 
> >>>>>>>> original patch and then the attached one.
> >>>>>>> hmm, ok
> >>>>>> Done. Now remove the dec1() function (that was memcpy + 1 line of 
> >>>>>> code). As a side effect, it removes the need of a memcpy (the dec1() 
> >>>>>> call at decode_frame()).
> >>>>> ok
> >>>> Now the first patch (ra144_rescale_energy.diff) split the energy 
> >>>> rescaling out of the rms() function. The next patch remove *lpc_refl from 
> >>>> the context, since the only thing needed from the last frame is the non 
> >>>> rescaled output of rms().
> >>> ok
> >> Now, I'm almost finished with this. Two things remains:
> >>
> >> 1- When decoding a ra144 encoded file, ffmpeg produces lots of "Multiple 
> >> frames in a packet from stream 0" (see 
> >> http://fate.multimedia.cx/index.php?test_result=1911120 for an example). 
> >> This is because the decoder receives a 1000 byte sample and decode only 20 
> >> bytes. The attached patch fix this (it decode all the 50 blocks).
> > 
> > wrong solution, we need a AVParser that splits the 1000bytes in 20byte
> > packets. A generic one that works based on block_align might be usefull
> > for other cases as well ...
> 
> I'll see it later.
> 
> > 
> > 
> >> 2- There are lots of unused table entries. Ok to remove them or do you 
> >> thing they can useful for anything (another codec?)?
> > 
> > remove
> > 
> > 
> >> Finally, if there is anything else you don't like about ra144.{c,h}, now is 
> >> the time to say if you want me to have a look at it...
> > 
> > 1st pass review of ra144.c is below :)
> > (yes you regret now that you asked, i know ...)
> 
> It was my masochistic side that made that question =)

good, please remind me to do another review when you are finished
with that one :)

With your help ra144.c will soon be pretty nice and clean, next comes
ra288.c i assume :)


[...]
> > 
> >> static void lpc_filter(const int16_t *lpc_coefs, const int16_t *adapt_coef,
> >>                        void *out, int *statbuf, int len)
> >> {
> >>     int x, i;
> >>     uint16_t work[50];
> >>     int16_t *ptr = work;
> >>
> >>     memcpy(work, statbuf,20);
> > 
> > 10*sizeof(int16_t)
> > 
> > 
> >>     memcpy(work + 10, adapt_coef, len * 2);
> >>
> >>     for (i=0; i<len; i++) {
> >>         int sum = 0;
> >>         int new_val;
> >>
> >>         for(x=0; x<10; x++)
> >>             sum += lpc_coefs[9-x] * ptr[x];
> >>
> >>         sum >>= 12;
> >>
> >>         new_val = ptr[10] - sum;
> >>
> >>         if (new_val < -32768 || new_val > 32767) {
> >>             memset(out, 0, len * 2);
> >>             memset(statbuf, 0, 20);
> >>             return;
> >>         }
> >>
> >>         ptr[10] = new_val;
> >>         ptr++;
> >>     }
> >>
> >>     memcpy(out, work+10, len * 2);
> >>     memcpy(statbuf, work + 40, 20);
> >> }
> > 
> > duplicate of ff_acelp_lp_synthesis_filter)(
> 
> No, they are slightly different (ff_acelp_lp_synthesis_filter use the 
> last n input values to predict out[n] 

no it does not, it uses filter_length which you could set to 10

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

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20080625/516acb75/attachment.pgp>



More information about the ffmpeg-devel mailing list