Re: [FFmpeg-devel] [PATCH 1/1] avcodec/pcm: reduce code size

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

chenyu202107@gmail.com:
> From: chenyu <chenyu202107@gmail.com>
> 
> add depends to pcm.c for reducing size when ALAW/MULAW/VIDC not defined
> 
> Signed-off-by: chenyu <chenyu202107@gmail.com>
> ---
>  libavcodec/pcm.c          | 36 +++++++++++++++++++++++++++++++-----
>  libavcodec/pcm_tablegen.h | 22 ++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/pcm.c b/libavcodec/pcm.c
> index bff61f2195..60a2c544a8 100644
> --- a/libavcodec/pcm.c
> +++ b/libavcodec/pcm.c
> @@ -44,15 +44,20 @@ static av_cold int pcm_encode_init(AVCodecContext *avctx)
>  #if !CONFIG_HARDCODED_TABLES
>      switch (avctx->codec->id) {
>  #define INIT_ONCE(id, name)                                                 \
> -    case AV_CODEC_ID_PCM_ ## id:                                            \
> -        if (CONFIG_PCM_ ## id ## _ENCODER) {                                \
> +    case AV_CODEC_ID_PCM_ ## id: {                                          \
>              static AVOnce init_static_once = AV_ONCE_INIT;                  \
>              ff_thread_once(&init_static_once, pcm_ ## name ## _tableinit);  \
> -        }                                                                   \
> -        break
> +            break;                                                          \
> +        }
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>          INIT_ONCE(ALAW,  alaw);
> +#endif
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>          INIT_ONCE(MULAW, ulaw);
> +#endif
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>          INIT_ONCE(VIDC,  vidc);
> +#endif

The macro already checks whether the relevant encoder is enabled; your
code meanwhile enables the code when the encoder or the decoder is
enabled, although only the encoders will ever use this.

>      default:
>          break;
>      }
> @@ -216,24 +221,30 @@ static int pcm_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>              bytestream_put_buffer(&dst, src, n * sample_size);
>          }
>          break;
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>      case AV_CODEC_ID_PCM_ALAW:
>          for (; n > 0; n--) {
>              v      = *samples++;
>              *dst++ = linear_to_alaw[(v + 32768) >> 2];
>          }
>          break;
> +#endif
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>      case AV_CODEC_ID_PCM_MULAW:
>          for (; n > 0; n--) {
>              v      = *samples++;
>              *dst++ = linear_to_ulaw[(v + 32768) >> 2];
>          }
>          break;
> +#endif
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>      case AV_CODEC_ID_PCM_VIDC:
>          for (; n > 0; n--) {
>              v      = *samples++;
>              *dst++ = linear_to_vidc[(v + 32768) >> 2];
>          }
>          break;
> +#endif
>      default:
>          return -1;
>      }
> @@ -327,20 +338,25 @@ static av_cold av_unused int pcm_lut_decode_init(AVCodecContext *avctx)
>      PCMLUTDecode *s = avctx->priv_data;
>  
>      switch (avctx->codec_id) {
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>      case AV_CODEC_ID_PCM_ALAW:
>          for (int i = 0; i < 256; i++)
>              s->table[i] = alaw2linear(i);
>          break;
> +#endif
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>      case AV_CODEC_ID_PCM_MULAW:
>          for (int i = 0; i < 256; i++)
>              s->table[i] = ulaw2linear(i);
>          break;
> +#endif
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>      case AV_CODEC_ID_PCM_VIDC:
>          for (int i = 0; i < 256; i++)
>              s->table[i] = vidc2linear(i);
>          break;
>      }
> -
> +#endif

The encoder checks here are wrong; and the placement of the last endif
is wrong, too.

>      avctx->sample_fmt = AV_SAMPLE_FMT_S16;
>      s->base.sample_size = 1;
>  
> @@ -545,6 +561,9 @@ static int pcm_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>              bytestream_get_buffer(&src, samples, n * sample_size);
>          }
>          break;
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER || \
> +    CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER || \
> +    CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER

This should not check for the encoders at all, because this is in the
decoder.

>      case AV_CODEC_ID_PCM_ALAW:
>      case AV_CODEC_ID_PCM_MULAW:
>      case AV_CODEC_ID_PCM_VIDC: {
> @@ -555,6 +574,7 @@ static int pcm_decode_frame(AVCodecContext *avctx, AVFrame *frame,
>              *samples_16++ = lut[*src++];
>          break;
>      }
> +#endif
>      case AV_CODEC_ID_PCM_LXF:
>      {
>          int i;
> @@ -655,7 +675,9 @@ const FFCodec ff_ ## name_ ## _decoder = {                                  \
>   *       to the table in pcm_decode_init() as well. */
>  //            AV_CODEC_ID_*      pcm_* name
>  //                          AV_SAMPLE_FMT_*    long name                                DecodeContext   decode init func
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>  PCM_CODEC_EXT(ALAW,         S16, alaw,         "PCM A-law / G.711 A-law",               PCMLUTDecode,   pcm_lut_decode_init);
> +#endif

These macros already contain implicit checks to ensure that only the
FFCodecs for actually enabled codecs are included in the binary.

>  PCM_DEC_EXT  (F16LE,        FLT, f16le,        "PCM 16.8 floating point little-endian", PCMScaleDecode, pcm_scale_decode_init);
>  PCM_DEC_EXT  (F24LE,        FLT, f24le,        "PCM 24.0 floating point little-endian", PCMScaleDecode, pcm_scale_decode_init);
>  PCM_CODEC    (F32BE,        FLT, f32be,        "PCM 32-bit floating point big-endian");
> @@ -663,7 +685,9 @@ PCM_CODEC    (F32LE,        FLT, f32le,        "PCM 32-bit floating point little
>  PCM_CODEC    (F64BE,        DBL, f64be,        "PCM 64-bit floating point big-endian");
>  PCM_CODEC    (F64LE,        DBL, f64le,        "PCM 64-bit floating point little-endian");
>  PCM_DECODER  (LXF,          S32P,lxf,          "PCM signed 20-bit little-endian planar");
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>  PCM_CODEC_EXT(MULAW,        S16, mulaw,        "PCM mu-law / G.711 mu-law",             PCMLUTDecode,   pcm_lut_decode_init);
> +#endif
>  PCM_CODEC    (S8,           U8,  s8,           "PCM signed 8-bit");
>  PCM_CODEC    (S8_PLANAR,    U8P, s8_planar,    "PCM signed 8-bit planar");
>  PCM_CODEC    (S16BE,        S16, s16be,        "PCM signed 16-bit big-endian");
> @@ -686,5 +710,7 @@ PCM_CODEC    (U32BE,        S32, u32be,        "PCM unsigned 32-bit big-endian")
>  PCM_CODEC    (U32LE,        S32, u32le,        "PCM unsigned 32-bit little-endian");
>  PCM_CODEC    (S64BE,        S64, s64be,        "PCM signed 64-bit big-endian");
>  PCM_CODEC    (S64LE,        S64, s64le,        "PCM signed 64-bit little-endian");
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>  PCM_CODEC_EXT(VIDC,         S16, vidc,         "PCM Archimedes VIDC",                   PCMLUTDecode,   pcm_lut_decode_init);
> +#endif
>  PCM_DECODER  (SGA,          U8,  sga,          "PCM SGA");
> diff --git a/libavcodec/pcm_tablegen.h b/libavcodec/pcm_tablegen.h
> index 7274c3cd17..590ba59814 100644
> --- a/libavcodec/pcm_tablegen.h
> +++ b/libavcodec/pcm_tablegen.h
> @@ -42,6 +42,7 @@
>  #define         VIDC_SEG_SHIFT   (5)
>  #define         VIDC_SEG_MASK    (0xE0)
>  
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>  /* alaw2linear() - Convert an A-law value to 16-bit linear PCM */
>  static av_cold int alaw2linear(unsigned char a_val)
>  {
> @@ -57,7 +58,9 @@ static av_cold int alaw2linear(unsigned char a_val)
>  
>      return (a_val & SIGN_BIT) ? t : -t;
>  }
> +#endif
>  
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>  static av_cold int ulaw2linear(unsigned char u_val)
>  {
>      int t;
> @@ -74,7 +77,9 @@ static av_cold int ulaw2linear(unsigned char u_val)
>  
>      return (u_val & SIGN_BIT) ? (BIAS - t) : (t - BIAS);
>  }
> +#endif
>  
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>  static av_cold int vidc2linear(unsigned char u_val)
>  {
>      int t;
> @@ -88,6 +93,7 @@ static av_cold int vidc2linear(unsigned char u_val)
>  
>      return (u_val & VIDC_SIGN_BIT) ? (BIAS - t) : (t - BIAS);
>  }
> +#endif
>  
>  #if CONFIG_HARDCODED_TABLES
>  #define pcm_alaw_tableinit()
> @@ -96,10 +102,19 @@ static av_cold int vidc2linear(unsigned char u_val)
>  #include "libavcodec/pcm_tables.h"
>  #else
>  /* 16384 entries per table */
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>  static uint8_t linear_to_alaw[16384];
> +#endif
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>  static uint8_t linear_to_ulaw[16384];
> +#endif
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>  static uint8_t linear_to_vidc[16384];
> +#endif
>  
> +#if CONFIG_PCM_ALAW_DECODER  || CONFIG_PCM_ALAW_ENCODER  || \
> +    CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER || \
> +    CONFIG_PCM_VIDC_DECODER  || CONFIG_PCM_VIDC_ENCODER
>  static av_cold void build_xlaw_table(uint8_t *linear_to_xlaw,
>                               int (*xlaw2linear)(unsigned char),
>                               int mask)
> @@ -123,21 +138,28 @@ static av_cold void build_xlaw_table(uint8_t *linear_to_xlaw,
>      }
>      linear_to_xlaw[0] = linear_to_xlaw[1];
>  }
> +#endif
>  
> +#if CONFIG_PCM_ALAW_DECODER || CONFIG_PCM_ALAW_ENCODER
>  static void pcm_alaw_tableinit(void)
>  {
>      build_xlaw_table(linear_to_alaw, alaw2linear, 0xd5);
>  }
> +#endif
>  
> +#if CONFIG_PCM_MULAW_DECODER || CONFIG_PCM_MULAW_ENCODER
>  static void pcm_ulaw_tableinit(void)
>  {
>      build_xlaw_table(linear_to_ulaw, ulaw2linear, 0xff);
>  }
> +#endif
>  
> +#if CONFIG_PCM_VIDC_DECODER || CONFIG_PCM_VIDC_ENCODER
>  static void pcm_vidc_tableinit(void)
>  {
>      build_xlaw_table(linear_to_vidc, vidc2linear, 0xff);
>  }
> +#endif
>  #endif /* CONFIG_HARDCODED_TABLES */

This is static stuff; the compiler can already optimize it away when it
sees that it is not referenced by anything that is kept.

>  
>  #endif /* AVCODEC_PCM_TABLEGEN_H */

_______________________________________________
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".