[FFmpeg-devel] [RFC] move wmv2.c to its own file

[Diego Biurrun]

On Sat, Jul 28, 2007 at 08:15:38PM +0200, Diego Biurrun wrote:
> On Sat, Jul 21, 2007 at 10:50:03PM +0200, Aurelien Jacobs wrote:
> > On Fri, 20 Jul 2007 19:18:54 +0200
> > Diego Biurrun <diego at biurrun.de> wrote:
> > 
> > > On Thu, Jul 19, 2007 at 11:10:21PM +0200, Diego Biurrun wrote:
> > > > 
> > > > On Sat, May 19, 2007 at 01:15:57AM +0200, Aurelien Jacobs wrote:
> > > > > On Fri, 18 May 2007 11:46:40 +0200
> > > > > Diego Biurrun <diego at biurrun.de> wrote:
> > > > > 
> > > > > > On Fri, May 11, 2007 at 11:35:29PM +0200, Diego Biurrun wrote:
> > > > > > > This is a rough draft for moving wmv2.c to its own file, i.e. make it
> > > > > > > not be just an include from msmpeg4.c[1].
> > > > > > 
> > > > > > Here is the second try.  Unfortunately the patch is starting to become
> > > > > > large and the resulting libavcodec.a is about 100k bigger than the
> > > > > > original.  I'm sure you guys can tell me how to avoid this...
> > > > > 
> > > > > You include msmpeg4tab.h in msmpeg4.h which is then included both
> > > > > in msmpeg4.c and wmv2.c. This duplicates the static tables in the
> > > > > 2 objects.
> > > > > msmpeg4tab.h *must* only be included from a single .c file.
> > > > > Tables which needs to be shared between the 2 .c files *must*
> > > > > be moved to msmpeg4data.c.
> > > > > This should solve the libavcodec.a size increase.
> > > > 
> > > > Is there a (good) reason to keep any of those tables in msmpeg4tab.h?
> > > > What about moving them all to msmpeg4data.c?
> > > 
> > > Here is a rough draft to accomplish this.  I left out the removal of
> > > msmpeg4tab.h and the pasting of its content into mspeg4data.c, otherwise
> > > the patch would have ballooned to 170kB.  It compiles fine here and
> > > there is no size increase for libavcodec.a.  Do I need to give some of
> > > the table names ff_ prefixes and/or make them non-static?
> > > 
> > > --- libavcodec/msmpeg4data.h	(revision 9768)
> > > +++ libavcodec/msmpeg4data.h	(working copy)
> > > @@ -32,7 +32,19 @@
> > >  
> > > [...]
> > > +
> > > +static const uint8_t *wmv1_scantable[WMV1_SCANTABLE_COUNT+1];
> > > +
> > > +#define NB_RL_TABLES  6
> > > +
> > > +static RLTable rl_table[NB_RL_TABLES];
> > > +
> > > +static const uint8_t wmv1_y_dc_scale_table[32];
> > > +static const uint8_t wmv1_c_dc_scale_table[32];
> > > +static const uint8_t old_ff_y_dc_scale_table[32];
> > > +static const uint8_t old_ff_c_dc_scale_table[32];
> > > +
> > > +static MVTable mv_tables[2];
> > > +
> > > +static const uint8_t v2_mb_type[8][2];
> > > +static const uint8_t v2_intra_cbpc[4][2];
> > > +
> > > +static const uint32_t table_mb_non_intra[128][2];
> > > +static const uint8_t  table_inter_intra[4][2];
> > > +
> > > +static const uint32_t ff_table0_dc_lum[120][2];
> > > +static const uint32_t ff_table1_dc_lum[120][2];
> > > +static const uint32_t ff_table0_dc_chroma[120][2];
> > > +static const uint32_t ff_table1_dc_chroma[120][2];
> > > +
> > > +#define WMV2_INTER_CBP_TABLE_COUNT 4
> > > +static const uint32_t (*wmv2_inter_table[WMV2_INTER_CBP_TABLE_COUNT])[2];
> > > +
> > > +static const uint8_t wmv2_scantableA[64];
> > > +static const uint8_t wmv2_scantableB[64];
> > 
> > Declaring some static tables in a .h file seems dubious...
> > Either don't declare them or declare them extern.
> 
> I cannot declare them both static and extern.  So what's it going to be?

Comments?

Diego