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

[Michael Niedermayer]

On Mon, May 19, 2008 at 09:36:03AM +0200, Vitor Sessak wrote:
> Michael Niedermayer wrote:
> > On Thu, May 15, 2008 at 09:44:51AM +0200, Vitor Sessak wrote:
> >> Hi
> >>
> >> Michael Niedermayer wrote:
> >>> On Wed, May 14, 2008 at 09:12:07AM +0200, Vitor Sessak wrote:
> >>>> 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
> >> Yet some more:
> >>
> > [...]
> >> ra144_uint8_tables.diff.gz: Declare as (u)int8_t tables that fit in one
> >> byte (yes, that makes the indentation of ra144.h inconsistent. I'll fix
> >> the rest of the file later)
> >
> > this needs to be split in 3-4 patches
> >
> > short -> (u)int8_t change
> > [] -> [][]
> > hex -> decimal
> 
> Now [] -> [][] for etable{1,2}

ok

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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- 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/20080519/5250da9b/attachment.pgp>