[FFmpeg-devel] [PATCH v3 2/2] libavcodec/flacenc: Implement encoding of 32 bit-per-sample PCM

[Martijn van Beurden]

First of all, thanks for reviewing.

Op vr 2 sep. 2022 om 17:11 schreef Andreas Rheinhardt <
andreas.rheinhardt at outlook.com>:

> > +static inline void put_sbits64(PutBitContext *pb, int n, int64_t value)
> > +{
> > +    av_assert2(n >= 0 && n <= 64);
> > +
> > +    put_bits64(pb, n, (uint64_t)(value) & (~(UINT64_MAX << n)));
>
> Shifting by 64 bits here is UB, so better modify the assert to disallow
> it. And rename the function to put_sbits63().
>
>
I could also add specific handling for the 64-bit case. Perhaps something
like

put_bits64(pb, n, (uint64_t)(value) & (~((n < 64)?(UINT64_MAX << n):0)));

or should I leave that to whoever needs that functionality?


> > -static inline void set_sr_golomb_flac(PutBitContext *pb, int i, int k,
> > -                                      int limit, int esc_len)
> > +static inline void set_sr_golomb_flac(PutBitContext *pb, int i, int k)
>
> This seems to be only used by flacenc.c, so IMO it would be better to
> move it there.
>
>
Yes, I was wondering what to do with this, similarly with the functions in
get_bits, mathops and put_bits. I suppose the additions to get_bits,
mathops and put_bits might be useful to others in the future. The golomb
code is probably not useful for other codecs indeed. Is that reasoning
valid?