Re: [FFmpeg-devel] [PATCH 1/9] avcodec/opus_rc: Remove write-only waste from OpusRangeCoder

[Lynne <dev@lynne.ee>]

Oct 7, 2022, 22:20 by andreas.rheinhardt@outlook.com:

> Write-only since e7d977b446194649aa30f2aacc6c17bce7aeb90b
> (and local to ff_opus_rc_enc_end() before that).
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/opus_rc.c | 2 --
>  libavcodec/opus_rc.h | 3 ---
>  2 files changed, 5 deletions(-)
>
> diff --git a/libavcodec/opus_rc.c b/libavcodec/opus_rc.c
> index c432eb90c9..2061418e52 100644
> --- a/libavcodec/opus_rc.c
> +++ b/libavcodec/opus_rc.c
> @@ -383,8 +383,6 @@ void ff_opus_rc_enc_end(OpusRangeCoder *rc, uint8_t *dst, int size)
>  rng_bytes = rc->rng_cur - rc->buf;
>  memcpy(dst, rc->buf, rng_bytes);
>  
> -    rc->waste = size*8 - (rc->rb.bytes*8 + rc->rb.cachelen) - rng_bytes*8;
> -
>  /* Put the rawbits part, if any */
>  if (rc->rb.bytes || rc->rb.cachelen) {
>  int i, lap;
> diff --git a/libavcodec/opus_rc.h b/libavcodec/opus_rc.h
> index 627f83229e..1b3cb93a15 100644
> --- a/libavcodec/opus_rc.h
> +++ b/libavcodec/opus_rc.h
> @@ -49,9 +49,6 @@ typedef struct OpusRangeCoder {
>  uint8_t *rng_cur;                      /* Current range coded byte */
>  int ext;                               /* Awaiting propagation */
>  int rem;                               /* Carryout flag */
> -
> -    /* Encoding stats */
> -    int waste;
>  } OpusRangeCoder;
>

Not really to patch 1. The purpose of the waste field was to know how many
bits were spare and usable to put stuff like encoder settings into. It's only a
small field, compilers probably eliminate it anyway, and it's useful for debugging,
just leave it there for now. It's not something I'd argue about, so if you'd really like
to get rid of it, do tell.

LGTM to patches 2-6. Whatever for the opusenc_psy file, I have a rewrite
I need to finish at some point.

Definite NAK to patch 7. The RC system is very easy to read and well-organized.
I'd like to keep it that way than hunt for spare bytes that any LTO trivially removes.
I'd accept a patch that changes the encoder array to heap allocated one (better yet
a framepool) if you'd still like to save on the struct size.

LGTM to patches 8-9.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".