[FFmpeg-devel] [PATCH] wavdec: RIFX file format support

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Dec 16 08:36:30 CET 2014


On Mon, Dec 15, 2014 at 10:24:55AM +0000, Carl Eugen Hoyos wrote:
> Thomas Volkert <silvo <at> gmx.net> writes:
> 
> > +#include <netinet/in.h>
> 
> This will hopefully be unneeded.
> 
> >       codec->sample_rate = avio_rl32(pb);
> >       codec->bit_rate    = avio_rl32(pb) * 8;
> >       codec->block_align = avio_rl16(pb);
> > +    if (big_endian) {
> > +        id                 = ntohs(id);
> > +        codec->channels    = ntohs(codec->channels);
> > +        codec->sample_rate = ntohl(codec->sample_rate);
> > +        codec->bit_rate    = ntohl(codec->bit_rate / 8) * 8;
> > +        codec->block_align = ntohs(codec->block_align);
> > +    }
> 
> Instead please do:
> if (big_endian) {
>   id = avio_rb32(pb);
>   codec->channels = avio_rb32(pb);
>  ...
> } else {
> id = avio_rl32(pb);
> ...
> }

Not sure this is a good idea, as it duplicates the code.
It might be better to use the if as-is, just replacing ntoh*
by av_bswap*.
Also note that ntoh* is just incorrect here, since they are a NOP
on big-endian systems, i.e. the code wouldn't work on big-endian.
Which also means that adding a test-case for this format would
be advisable so we spot such errors, rare formats are likely
to break with future changes if they have no test coverage.


More information about the ffmpeg-devel mailing list