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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to