[FFmpeg-devel] [PATCH] Some ra144.c simplifications
Vitor Sessak
vitor1001
Thu May 15 20:55:35 CEST 2008
Hi
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_const.diff: Make const everything that can be declared so
>
> ok
>
>
>> ra144_useless_buffers.diff: Remove some useless buffers (and avoid a memcpy)
>
> ok
>
>
>> ra144_useless_buffers2.diff: More useless buffer removal
>
> see below
>
>
>> ra144_nopar_ctx.diff: Don't send the context for functions that don't use it
>
> ok
>
>
>> ra144_rotate_block.diff: Simplify rotate_block()
>
> ok
Applied everything ok'ed.
>
>
>> 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)
>
> ill look at that in a moment
New patch attached, only changing _everything_ form hex to decimal.
> [...]
>> --- ../ffmpeg.old2/libavcodec/ra144.c 2008-05-14 18:02:44.000000000 +0200
>> +++ libavcodec/ra144.c 2008-05-14 18:24:08.000000000 +0200
>> @@ -105,7 +105,7 @@
>> static void do_output_subblock(Real144_internal *glob, const unsigned short *gsp, unsigned int gval, signed short *output_buffer, GetBitContext *gb)
>> {
>> unsigned short int buffer_a[40];
>> - unsigned short int buffer_d[40];
>> + unsigned short int *block;
>> int e, f, g;
>> int a = get_bits(gb, 7);
>> int d = get_bits(gb, 8);
>> @@ -125,13 +125,13 @@
>> else
>> g = 0;
>>
>
>> + memmove(glob->buffer_2, glob->buffer_2 + BLOCKSIZE, (BUFFERSIZE - BLOCKSIZE) * 2);
>> +
>> + block = glob->buffer_2 + BUFFERSIZE - BLOCKSIZE;
>> - add_wav(glob, d, a, g, e, f, buffer_a, etable1 + b*BLOCKSIZE,
>> - etable2 + c*BLOCKSIZE, buffer_d);
>> + add_wav(glob, d, a, g, e, f, buffer_a, etable1 + b*BLOCKSIZE,
>> + etable2 + c*BLOCKSIZE, block);
> ^^^^^
>> -
>> - memmove(glob->buffer_2, glob->buffer_2 + BLOCKSIZE, (BUFFERSIZE - BLOCKSIZE) * 2);
>> - memcpy(glob->buffer_2 + BUFFERSIZE - BLOCKSIZE, buffer_d, BLOCKSIZE * 2);
>>
>> - final(glob, gsp, buffer_d, output_buffer, glob->buffer, BLOCKSIZE);
>> + final(glob, gsp, block, output_buffer, glob->buffer, BLOCKSIZE);
> ^^^^^
> cosmetics, split!
Attached.
Also,
ra144_data_size.diff: Simplify a little ra144_decode_frame().
-Vitor
-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_data_size.diff
Type: text/x-patch
Size: 1093 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/2983b51f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_useless_buffers2.diff
Type: text/x-patch
Size: 1083 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/2983b51f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ra144_tables_dec.diff.gz
Type: application/x-gzip
Size: 74804 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080515/2983b51f/attachment-0002.bin>
More information about the ffmpeg-devel
mailing list