Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/adpcm: Disable dead code
       [not found] <20210722065241.EDF5A4116B4@natalya.videolan.org>
@ 2023-04-16 11:12 ` Michael Niedermayer
  2023-04-17 11:06   ` Anton Khirnov
  0 siblings, 1 reply; 2+ messages in thread
From: Michael Niedermayer @ 2023-04-16 11:12 UTC (permalink / raw)
  To: ffmpeg-devel


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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/adpcm: Disable dead code
  2023-04-16 11:12 ` [FFmpeg-devel] [FFmpeg-cvslog] avcodec/adpcm: Disable dead code Michael Niedermayer
@ 2023-04-17 11:06   ` Anton Khirnov
  0 siblings, 0 replies; 2+ messages in thread
From: Anton Khirnov @ 2023-04-17 11:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-04-17 11:06 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210722065241.EDF5A4116B4@natalya.videolan.org>
2023-04-16 11:12 ` [FFmpeg-devel] [FFmpeg-cvslog] avcodec/adpcm: Disable dead code Michael Niedermayer
2023-04-17 11:06   ` Anton Khirnov

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git