diff mbox series

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

Message ID GV1P250MB0737F6A03249BF948291E39F8F5F9@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,1/9] avcodec/opus_rc: Remove write-only waste from OpusRangeCoder | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Oct. 7, 2022, 8:20 p.m. UTC
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(-)

Comments

Lynne Oct. 8, 2022, 3:25 a.m. UTC | #1
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.
diff mbox series

Patch

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;
 
 /**