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

[Vitor Sessak]

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.

-Vitor




-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_unpack_removal.diff
Type: text/x-patch
Size: 3616 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080513/4e86962d/attachment.bin>