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

[Vitor Sessak]

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_const.diff: Make const everything that can be declared so
ra144_useless_buffers.diff: Remove some useless buffers (and avoid a memcpy)
ra144_useless_buffers2.diff: More useless buffer removal
ra144_nopar_ctx.diff: Don't send the context for functions that don't use it
ra144_rotate_block.diff: Simplify rotate_block()
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)

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_const.diff
Type: text/x-diff
Size: 4226 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/64d9c397/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_useless_buffers.diff
Type: text/x-diff
Size: 1228 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/64d9c397/attachment-0001.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_useless_buffers2.diff
Type: text/x-diff
Size: 1162 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/64d9c397/attachment-0002.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_nopar_ctx.diff
Type: text/x-diff
Size: 2402 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/64d9c397/attachment-0003.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_rotate_block.diff
Type: text/x-diff
Size: 750 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/64d9c397/attachment-0004.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_uint8_tables.diff.gz
Type: application/x-gzip
Size: 35907 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/64d9c397/attachment.bin>