On Wed, Dec 17, 2014 at 11:55:17AM +0100, Thomas Volkert wrote: > On 12/16/2014 08:36 AM, Reimar Döffinger wrote: > >On Mon, Dec 15, 2014 at 10:24:55AM +0000, Carl Eugen Hoyos wrote: > >>> 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*. > > I would prefer this version: > if (!big_endian) { > avio_rl32() > } else { > avio_rb32() > } > In case of big endianess, your idea (and my former solution) would need two > instead of one updates per value
Why would that matter? Performance, especially for big-endian type is hardly relevant. Due to fewer branches it might actually give better performance for the common case (but as said I don't think it matters). It mostly avoids duplicating some actual code like the *8. There are also other options to reduce the code size, though I am sceptical if they are a good idea. 1) Macros like: #define GET16(pb) (big_endian ? avio_rb16(pb) : avio_rl16(pb)) 2) Functions that take an additional endian argument. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel