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

[Vitor Sessak]

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}()

-Vitor

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_more_decode_frame.diff
Type: text/x-patch
Size: 1035 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080521/c0a2fda0/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_eq.diff
Type: text/x-patch
Size: 1582 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080521/c0a2fda0/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_rms.diff
Type: text/x-patch
Size: 1038 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080521/c0a2fda0/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_final.diff
Type: text/x-patch
Size: 1397 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080521/c0a2fda0/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_wavtables.diff
Type: text/x-patch
Size: 59273 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080521/c0a2fda0/attachment-0004.bin>