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

[Michael Niedermayer]

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 ...)

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

Democracy is the form of government in which you can choose your dictator
-------------- 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/20080524/1f8aad47/attachment.pgp>