[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