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

[Vitor Sessak]

Michael Niedermayer wrote:
> On Sun, May 25, 2008 at 01:06:27PM +0200, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Sat, May 24, 2008 at 06:48:03PM +0200, Vitor Sessak wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Sat, May 24, 2008 at 02:35:05PM +0200, Vitor Sessak wrote:
>>>>>> Michael Niedermayer wrote:
>>>>>>> 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
>>>>>> All commited.
>>>>>>
>>>>>> Now, ra144_handle_bytes_missing.diff checks if it has enough input (20 
>>>>>> bytes) before decoding. This avoid breaking randomly binary 
>>>>>> compatibility...
>>>>> fine
>>>> Commited. Now more ctx var redux.
>>> fine if it does the same thing as before (i didnt check ...)
>> Commited. Now some more (names should be pretty descriptive).
> 
> ok

One more...

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_remove_gbuf2_ctx.diff
Type: text/x-patch
Size: 3029 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080525/c0a2de8a/attachment.bin>