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

Vitor Sessak vitor1001
Sun May 25 18:05:15 CEST 2008


Vitor Sessak wrote:
> 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...

... and some more cleanup:

ra144_vector_add_wav.diff: Make add_wav() receive a vector instead of 
three integers

ra144_params_dec2.diff: Do not calculate anything based in l, it is 
unrolled in the loop anyway

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



More information about the ffmpeg-devel mailing list