[FFmpeg-devel] [PATCH] mpegaudiodec: share identical tables between float and fixed decoder.

[Reimar Döffinger]

On Mon, Sep 01, 2014 at 11:50:18AM +0200, Michael Niedermayer wrote:
> On Mon, Sep 01, 2014 at 09:30:04AM +0200, Reimar Döffinger wrote:
> > On 01.09.2014, at 02:01, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > On Sun, Aug 31, 2014 at 10:47:09PM +0200, Reimar Döffinger wrote:
> > >> ff_mpa_table_4_3_value alone saves over 128kB.
> > > 
> > > i wonder if that table shouldnt be something different in the float
> > > implementation.
> > > Also i think anyone caring about file size would likely not build
> > > the float and fixed point mp3 decoder
> > 
> > There are other issues, having them twice IMHO makes a mess and it is unclear which tables actually differ between the documentation.
> > Which in turn makes it very difficult to add support for hardcoding them (which is what I started out wanting to do).
> > I also think some of these might only be used in one version of the decoder.
> 
> what i was thinking is that the decoders should be reviewed first to
> make sure each uses the optimal data types and layouts, that is
> possibly float based tables in the float decoder an int tables in the
> fixed one. But also possibly more differences where it makes sense
> (comparing to other open source mp3 decoders could also make sense)
> 
> and after the layout and types of the tables are optimal they would
> be split into shared and non shared and hardcoded support be added
> 
> if they are shared now, it would require to undo that for tables
> that end up only being used on one side later. So it seems this order
> of doing things would end up being more work overall, but then some
> of that is already done so dunno ...

I disagree with that strategy, because it seems to me to assume
that the biggest effort is in actually moving the tables around.
The issue right now is that it's a huge pain to figure out which
tables are even used by which decoder, which are identical between
fixed and float and which differ.
>From my point of view, my patch gives a somewhat shorter list
of structs to review if they could/should be specialized.
Or to put it differently: If I hadn't posted the patch, had you even
been aware that ff_mpa_table_4_3_value might be an optimization
opportunity? Though it's kind of a bad example, that one can be
hardcoded already, but through stupid includes it ended up twice in the
binary.
Also not that it's not particularly hard to make them different after
hardcoding them: ff_mpa_expval_table_fixed/ff_mpa_expval_table_float
is already an example of that.
All that said, I have to agree with you that what l3_unscale does seem
fairly stupid in case of the float decoder, though it might be a bit
tricky to optimize...