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

[Vitor Sessak]

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

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