[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