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

[Aurelien Jacobs]

On Sat, 11 Aug 2007 15:58:34 +0200
Diego Biurrun <diego at biurrun.de> wrote:

> On Thu, Aug 09, 2007 at 10:15:53PM +0200, Aurelien Jacobs wrote:
> > On Fri, 3 Aug 2007 18:53:06 +0200
> > Diego Biurrun <diego at biurrun.de> wrote:
> > 
> > > 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:
> > > > > > > 
> > > > > > > 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?
> > 
> > Sorry for the delay, I was in holidays.
>  
> Thanks for taking the time to enlighten me.
> 
> > Well, I will try to be a bit more explicit.
> > Tables which are declared static in a .c file don't need to be declared
> > in a .h file as they can't be used in another compilation unit.
> > Tables which need to be used in another compilation unit can't be static.
> > 
> > In this situation, I guess what you want is to make those tables extern.
> 
> New patch attached.  OK to commit?

This one looks ok to me.

Aurel