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

[Michael Niedermayer]

On Wed, May 21, 2008 at 09:25:40PM +0200, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> 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
>
> Same thing for wavtable{1,2}: ra144_wavtables.diff
>
> Also
>
> ra144_more_decode_frame.diff: More simplifications to ra144_decode_frame()
> ra144_{eq,rms,final}.diff: Simplify {eq,rms,final}()

all ok

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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20080521/03977909/attachment.pgp>