Am Samstag, den 22.10.2011, 14:19 +0200 schrieb Diego Biurrun:
First: Thanks for your review!
> > --- a/libavformat/oma.c
> > +++ b/libavformat/oma.c
> > @@ -65,6 +68,10 @@ static const AVCodecTag codec_oma_tags[] = {
> >
> > +static const uint16_t srate_tab[6] = {320,441,480,882,960,0};
> > +
> > +#if CONFIG_OMA_DEMUXER
>
> The table is only used in the muxer, so it should be below the
> appropriate #ifdef.
You seem to have missed something. It is used in the demuxer (in
oma_read_header, directly after the "case OMA_CODECID_ATRAC3:" line, as
well in the loop you quoted below in the muxer. I wonder whether the
optimization of using 100Hz units to save space by storing as 16 bit
values really makes sense, though, as it makes the code bigger. Scaling
by 2 should be enough to fit all the rates into 16 bits, and "*2" should
be smaller and faster than "*100".
> I wonder if you could not (easily) put the muxer in a separate file and
> skip the ifdeffery.
This is indeed possible, except for the sample rate table mentioned
above, which is shared between encoder and decoder. I expect making
three files out of it (one shared, containing only the sample rate
table, one for the muxer, one for the demuxer) is overkill, the other
two options are duplicating the table or using the ifdefs as it is now.
I don't think there is an option to have the compiler merge the tables
if they are identically defined in two source files (which gcc does for
strings)
Regards,
Michael Karcher
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
