2017-08-23 0:56 GMT+02:00 Ivan Kalvachev <ikalvac...@gmail.com>: > 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.
No objection here - on the contrary, I wish this argument would count here more often... > + 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) Not at the same time but in the same encoder file (G.726). > + 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. I don't think this is correct (and not what the diffstats show afair) but it doesn't matter: I was under the impression this patch was a requirement after a previous commit, I am not particularly keen on it, see above. Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
