[Ffmpeg-devel] [PATCH] remove ac3 tables from parser.c
Justin Ruggles
justinruggles
Sat Mar 10 05:18:42 CET 2007
Michael Niedermayer wrote:
> Hi
>
> On Fri, Mar 09, 2007 at 09:43:02AM -0500, Justin Ruggles wrote:
>
>>Justin Ruggles wrote:
>>
>>>Limin Wang wrote:
>>>
>>>>Hi,
>>>>
>>>>* Justin Ruggles <justinruggles at bellsouth.net> [2007-03-08 18:21:09 -0500]:
>>>>
>>>>
>>>>
>>>>
>>>>>This removes duplication in the AC-3 tables between encoder and parser.
>>>>>
>>>>>For now, there is not a place for common AC-3 code, so with this patch,
>>>>>parser.c includes ac3tab.h. Once there is an ac3.c, parser.c will be
>>>>>changed to include ac3.h instead.
>>>>>
>>>>>-Justin
>>>>
>>>>
>>>>I think table data shouldn't put into header file. we can put it into c file
>>>>and extern the array in the header file or define a function to get it.
>>>>So just remove the static and extern them in ac3tab.h.
>>>
>>>
>>>Okay, so to avoid the arguments here, maybe I should just wait until
>>>there is a c file for common ac3 data and functions. Right now the only
>>>way I can see to avoid the duplication and do what you're describing is
>>>to make the ac3 parser dependent on the ac3 encoder, which I don't think
>>>would be a good thing.
>>>
>>>Right now I'm just trying to reduce the ac3 decoder patch one step at a
>>>time. I guess I'll start somewhere else and do this step later.
>>
>>Now is a better time for a more complete patch to do this. The attached
>>patch does the same as the last one, but generates the frame size table
>>at runtime. It also does not duplicate the tables in the object files.
>
> [...]
>
>>Index: libavcodec/ac3.c
>>===================================================================
>>--- libavcodec/ac3.c (revision 8305)
>>+++ libavcodec/ac3.c (working copy)
>>@@ -194,4 +194,12 @@
>> l += v;
>> }
>> bndtab[50] = l;
>>+
>>+ /* generate ff_ac3_frame_sizes table */
>>+ for(i=0; i<38; i++) {
>>+ for(j=0; j<2; j++) {
>
>
> j<3 !
>
>
>
>>+ int br = ff_ac3_bitratetab[i >> 1] * 1000;
>>+ ff_ac3_frame_sizes[i][j] = (br * 96 / ff_ac3_freqs[j]) + (i & 1);
>>+ }
>>+ }
>
>
> this looks wrong, it doesnt generate the same table
> the followig MIGHT be correct (didnt really check)
>
> for(i=0; i<38; i++) {
> int br = ff_ac3_bitratetab[i >> 1];
> ff_ac3_frame_sizes[i][0] = ( 2*br );
> ff_ac3_frame_sizes[i][1] = (320*br / 147) + (i & 1);
> ff_ac3_frame_sizes[i][2] = ( 3*br );
> }
>
> [...]
You're right. I did forget about the fact that the odd numbers only
differ for 44.1kHz, and that was all I tested with.
ff_ac3_frame_sizes[i][j] = (br * 96 / ff_ac3_freqs[j]) + ((i & 1) &&
(j == 1));
The above also does the trick, but I like your version better. New
patch attached.
-Justin
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ac3_parser_remove_tables.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070309/015e4ced/attachment.asc>
More information about the ffmpeg-devel
mailing list