Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/adpcm: Disable dead code

From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/adpcm: Disable dead code
Date: Mon, 17 Apr 2023 13:06:19 +0200
Message-ID: <168172957987.3843.3613514529454952820@lain.khirnov.net> (raw)
In-Reply-To: <20230416111246.GB275136@pb2>

Quoting Michael Niedermayer (2023-04-16 13:12:46)
> On Thu, Jul 22, 2021 at 06:52:40AM +0000, Andreas Rheinhardt wrote:
> > ffmpeg | branch: master | Andreas Rheinhardt <andreas.rheinhardt@outlook.com> | Tue Jun  8 18:26:52 2021 +0200| [0f168344f1034c84fe85c4a8c3b5638bd4ba9931] | committer: Andreas Rheinhardt
> > 
> > avcodec/adpcm: Disable dead code
> > 
> > This change ensures that the linker can drop adpcm_data.o if no decoder
> > that actually uses anything from there is enabled.
> > 
> > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=0f168344f1034c84fe85c4a8c3b5638bd4ba9931
> > ---
> > 
> >  libavcodec/adpcm.c | 212 +++++++++++++++++++++++++++--------------------------
> >  1 file changed, 108 insertions(+), 104 deletions(-)
> > 
> > diff --git a/libavcodec/adpcm.c b/libavcodec/adpcm.c
> > index 79581c863c..886fce9209 100644
> > --- a/libavcodec/adpcm.c
> > +++ b/libavcodec/adpcm.c
> > @@ -61,6 +61,18 @@
> >   * readstr http://www.geocities.co.jp/Playtown/2004/
> >   */
> >  
> > +#define CASE_0(codec_id, ...)
> > +#define CASE_1(codec_id, ...) \
> > +    case codec_id:            \
> > +    { __VA_ARGS__ }           \
> > +    break;
> > +#define CASE_2(enabled, codec_id, ...) \
> > +        CASE_ ## enabled(codec_id, __VA_ARGS__)
> > +#define CASE_3(config, codec_id, ...) \
> > +        CASE_2(config, codec_id, __VA_ARGS__)
> > +#define CASE(codec, ...) \
> > +        CASE_3(CONFIG_ ## codec ## _DECODER, AV_CODEC_ID_ ## codec, __VA_ARGS__)
> > +
> >  /* These are for CD-ROM XA ADPCM */
> >  static const int8_t xa_adpcm_table[5][2] = {
> >      {   0,   0 },
> > @@ -821,8 +833,7 @@ static int get_nb_samples(AVCodecContext *avctx, GetByteContext *gb,
> >              buf_size = FFMIN(buf_size, avctx->block_align);
> >          nb_samples = (buf_size - 4 * ch) * 2 / ch;
> >          break;
> > -    case AV_CODEC_ID_ADPCM_IMA_WAV:
> > -    {
> > +    CASE(ADPCM_IMA_WAV,
> >          int bsize = ff_adpcm_ima_block_sizes[avctx->bits_per_coded_sample - 2];
> >          int bsamples = ff_adpcm_ima_block_samples[avctx->bits_per_coded_sample - 2];
> >          if (avctx->block_align > 0)
> 
> This makes the code much harder to debug because the line number shown
> by an error or warning is now the line of the macro and not the line number
> of the argument of the macro.
> The argument is just a multiline blob it from the point of view of a compiler
> seems not to be assigned to a problem occuring later after macro expasion.
> 
> I think use of super smart macros is a bad idea. Other big projects have gone
> this direction long ago and i cant name one where that made them better, can you?

I agree, complex macros make the code much harder to read AND debug and
should be avoided whenever possible.

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