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

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2902 bytes --]

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?

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[Anton Khirnov]

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