On 8/22/17, Ronald S. Bultje <rsbul...@gmail.com> wrote: > Hi, > > On Mon, Aug 21, 2017 at 11:16 AM, Carl Eugen Hoyos <ceffm...@gmail.com> > wrote: > >> Hi! >> >> Attached patch tries to slightly simplify and clean up the usage of >> put_bits* in libavcodec: put_bits_le() functions exist for the >> little-endian G.726 encoder, so the define makes less sense now. >> >> Fate passes here, please review, Carl Eugen > > > I have to agree with Paul here, I can't say I'm a big fan of this...
People, As developers you are required not only to give ultimate final verdicts, but also give (nice technical) reasoning for them. Here is my list of pro and cons: - If it ain't broken, don't change it. + Bytesteam already uses explicit _le/be and it looks good. + Makes the code more clear. It is obvious that the given encoder writes BE stream. Something that could easily be missed with the single define. - The type of bitstream however is not really important for the codec working. Aka, there is no confusing, because no codec writes BE and LE at the same time.(yet) + Removes messy defines that obfuscate put_bits code, by separating the big and little ending code. - Duplicates put_bits.h code. It would probably make reworking harder, as changes have to be synced in 2 places. While the last negative point is most important to me, there is a workaround for it. The new bitstream functions could be created by a template. This however would make the code use more messy defines, that also negates the last positive point. Well, maybe other developers have better points. Let's see them. Best Regards _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
