[FFmpeg-devel] [PATCH]lavc/put_bits: Remove usage of BITSTREAM_WRITER_LE.
Carl Eugen Hoyos
ceffmpeg at gmail.com
Thu Aug 24 14:19:01 EEST 2017
2017-08-23 0:56 GMT+02:00 Ivan Kalvachev <ikalvachev at gmail.com>:
> On 8/22/17, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> On Mon, Aug 21, 2017 at 11:16 AM, Carl Eugen Hoyos <ceffmpeg at gmail.com>
>>> 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...
> 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.
More information about the ffmpeg-devel