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

[Michael Niedermayer]

On Wed, May 14, 2008 at 09:12:07AM +0200, Vitor Sessak wrote:
> Hi
> 
> On Tue, May 13, 2008 at 2:13 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >
> > On Tue, May 13, 2008 at 11:05:45AM +0200, Vitor Sessak wrote:
> >  > Michael Niedermayer wrote:
> >  >> On Sun, May 11, 2008 at 05:45:29PM +0200, Vitor Sessak wrote:
> >  >>> Michael Niedermayer wrote:
> >  >>>> On Sat, May 10, 2008 at 04:50:03PM +0200, Vitor Sessak wrote:
> >  >>>>> Hi,
> >  >>>>>
> >  >>>>> libavcodec/ra144.c really needs some cleanup. I'll start with the
> >  >>>>> following:
> >  >>>>>
> >  >>>>> ra144_indent.diff: Reindent the whole file. Since there was no
> >  >>>>> substantial change in this file since it inclusion in 2003, I don't
> >  >>>>> think there is any problem in making its indentation inline with the
> >  >>>>> rest of FFmpeg.
> >  >>>>>
> >  >>>>> ra144_unpack_rewrite.diff: Rewrite completely unpack_input()
> >  >>>>>
> >  >>>>> ra144_simp{1,2}.diff: minor simplifications
> >  >>>> all ok if you have tested your code
> >  >>> Next round:
> >  >>>
> >  >>> ra144_simp{3,4}.diff: minor simplifications (FFSWAP, unused stuff
> >  >>> removal)
> >  >> ok
> >  >>> ra144_sqrt.diff: remove ff_sqrt reimplementation and its corresponding
> >  >>> big table (output is not anymore binary-identical but this function is
> >  >>> more accurate, not less). Nice thing we have now a fast ff_sqrt :-)
> >  >> by how much does the output differ?
> >  >
> >  > According to tiny_psnr for drummers.14.ra
> >  > stddev: 28.84 PSNR:18.92 bytes:5369856
> >  >
> >  > which looks a lot to me (even if playing the file I can't hear any
> >  > difference). I suggest to leave this for after the rest of the cleanup.
> >  >
> >  >>> ra144_split_val.diff: glob->val is unrelated with the rest of the
> >  >>> unpacked buffer. This patch split its table out of the others
> >  >> ok
> >  >>> and move
> >  >>> it parsing to a more reasonable place.
> >  >> not ok, the place does not look reasonable to me
> >  >
> >  > I don't know if it is this that you mean, but the unpack_input()
> >  > function is a bit senseless. It read three unrelated things (10 codebook
> >  > indexes, a codebook index for val and 4 4-byte blocks) from the
> >  > bitstream and put everything in the same buffer. The attached patch (to
> >  > be applied after ra144_rename_valtab.diff) simply remove this function
> >  > and move the bitstream reading to where the data is actually needed.
> >
> >  ok (I assume that every cleanup has been tested and has bit identical
> >  output ...)
> 
> I do test them.
> 
> More cleanup (to be applied after the previous ok'ed patches):
> 
> ra144_trim_context.diff : Remove a lot of vars from context
> ra144_trim_context2.diff: Modify add_wav() to remove another
> context var

all ok

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- 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/20080514/ad44897b/attachment.pgp>