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); ... } If you do not reindent the little endian block, the patch gets much easier to read. > + if(!big_endian) > + codec->bits_per_coded_sample = avio_rl16(pb); > + else > + codec->bits_per_coded_sample = ntohs(avio_rl16(pb)); avio_rb32(pb) and please add braces around if - else blocks. > if (size >= 18) { /* We're obviously dealing with WAVEFORMATEX */ > + if (big_endian) > + avpriv_report_missing_feature(codec, > "WAVEFORMATEX support for RIFX files\n"); Is this sufficient, no further error handling needed? > + if (!big_endian) > + return avio_rl32(pb); > + else > + return ntohl(avio_rl32(pb)); avio_rb32() and braces. > - if (!memcmp(p->buf, "RIFF", 4)) > + if ((!memcmp(p->buf, "RIFF", 4)) || (!memcmp(p->buf, "RIFX", 4))) Maybe I misread but this looks like too many parentheses. > - int rf64; > + int rf64 = 0; Should be unneeded. > - /* check RIFF header */ > - tag = avio_rl32(pb); nit: You could not remove the variable and do a switch (tag) below to make the patch smaller. (Smaller patch == easier review, both now and in the future) > + wav->rifx = 0; Should be unneeded. One line looked to me as if it contained a tab, use tools/patcheck to check your patch before submitting. Thank you! Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel