[FFmpeg-devel] [PATCH]lavc/put_bits: Remove usage of BITSTREAM_WRITER_LE.

[Carl Eugen Hoyos]

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:
>> Hi,
>>
>> On Mon, Aug 21, 2017 at 11:16 AM, Carl Eugen Hoyos <ceffmpeg at 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