diff mbox series

[FFmpeg-devel] avcodec/avcodec: remove usage of __typeof__()

Message ID 20240908192201.9963-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel] avcodec/avcodec: remove usage of __typeof__() | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer Sept. 8, 2024, 7:22 p.m. UTC
It's non-standard C.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/avcodec.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Martin Storsjö Sept. 8, 2024, 7:24 p.m. UTC | #1
On Sun, 8 Sep 2024, James Almer wrote:

> It's non-standard C.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/avcodec.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)

LGTM

// Martin
Martin Storsjö Sept. 8, 2024, 7:43 p.m. UTC | #2
On Sun, 8 Sep 2024, James Almer wrote:

> It's non-standard C.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/avcodec.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index cb89236549..78153d12f1 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -708,9 +708,9 @@ int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
>     return ff_encode_receive_frame(avctx, frame);
> }
>
> -#define WRAP_CONFIG(allowed_type, field, terminator)                        \
> +#define WRAP_CONFIG(allowed_type, field, field_type, terminator)            \
>     do {                                                                    \
> -        static const __typeof__(*(field)) end = terminator;                 \
> +        static const field_type end = terminator;                           \
>         if (codec->type != (allowed_type))                                  \
>             return AVERROR(EINVAL);                                         \
>         *out_configs = (field);                                             \
> @@ -753,15 +753,15 @@ int ff_default_get_supported_config(const AVCodecContext *avctx,
>     switch (config) {
> FF_DISABLE_DEPRECATION_WARNINGS
>     case AV_CODEC_CONFIG_PIX_FORMAT:
> -        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, AV_PIX_FMT_NONE);
> +        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, enum AVPixelFormat, AV_PIX_FMT_NONE);
>     case AV_CODEC_CONFIG_FRAME_RATE:
> -        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, (AVRational){0});
> +        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, AVRational, (AVRational){0});
>     case AV_CODEC_CONFIG_SAMPLE_RATE:
> -        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, 0);
> +        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, int, 0);
>     case AV_CODEC_CONFIG_SAMPLE_FORMAT:
> -        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, AV_SAMPLE_FMT_NONE);
> +        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, enum AVSampleFormat, AV_SAMPLE_FMT_NONE);
>     case AV_CODEC_CONFIG_CHANNEL_LAYOUT:
> -        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, (AVChannelLayout){0});
> +        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, AVChannelLayout, (AVChannelLayout){0});
> FF_ENABLE_DEPRECATION_WARNINGS
>
>     case AV_CODEC_CONFIG_COLOR_RANGE:
> -- 
> 2.46.0

Actually, this isn't quite enough to fix compilation with all compilers:

src/libavcodec/avcodec.c: In function 'ff_default_get_supported_config':
src/libavcodec/avcodec.c:758:9: error: initializer element is not constant
          WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, 
AVRational, (AVRational){0});
          ^
src/libavcodec/avcodec.c:764:9: error: initializer element is not constant
          WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, 
AVChannelLayout, (AVChannelLayout){0});
          ^

Since we're not using typeof here, we can drop the casts here and just use 
plain {0}:

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 78153d12f1..8d1a280323 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -755,13 +755,13 @@ FF_DISABLE_DEPRECATION_WARNINGS
      case AV_CODEC_CONFIG_PIX_FORMAT:
          WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, enum AVPixelFormat, AV_PIX_FMT_NONE);
      case AV_CODEC_CONFIG_FRAME_RATE:
-        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, AVRational, (AVRational){0});
+        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, AVRational, {0});
      case AV_CODEC_CONFIG_SAMPLE_RATE:
          WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, int, 0);
      case AV_CODEC_CONFIG_SAMPLE_FORMAT:
          WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, enum AVSampleFormat, AV_SAMPLE_FMT_NONE);
      case AV_CODEC_CONFIG_CHANNEL_LAYOUT:
-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, AVChannelLayout, (AVChannelLayout){0});
+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, AVChannelLayout, {0});
  FF_ENABLE_DEPRECATION_WARNINGS

// Martin
Rémi Denis-Courmont Sept. 9, 2024, 7:41 a.m. UTC | #3
Le 8 septembre 2024 22:22:01 GMT+03:00, James Almer <jamrial@gmail.com> a écrit :
>It's non-standard C.

The description is a little bit misleading. `typeof` is standard C (as of last year). Sure, technically `__typeof__` is not standard but this is easily misinterpreted.

Also TBH, this change seems highly error prone.

>
>Signed-off-by: James Almer <jamrial@gmail.com>
>---
> libavcodec/avcodec.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
>index cb89236549..78153d12f1 100644
>--- a/libavcodec/avcodec.c
>+++ b/libavcodec/avcodec.c
>@@ -708,9 +708,9 @@ int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
>     return ff_encode_receive_frame(avctx, frame);
> }
> 
>-#define WRAP_CONFIG(allowed_type, field, terminator)                        \
>+#define WRAP_CONFIG(allowed_type, field, field_type, terminator)            \
>     do {                                                                    \
>-        static const __typeof__(*(field)) end = terminator;                 \
>+        static const field_type end = terminator;                           \
>         if (codec->type != (allowed_type))                                  \
>             return AVERROR(EINVAL);                                         \
>         *out_configs = (field);                                             \
>@@ -753,15 +753,15 @@ int ff_default_get_supported_config(const AVCodecContext *avctx,
>     switch (config) {
> FF_DISABLE_DEPRECATION_WARNINGS
>     case AV_CODEC_CONFIG_PIX_FORMAT:
>-        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, AV_PIX_FMT_NONE);
>+        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, enum AVPixelFormat, AV_PIX_FMT_NONE);
>     case AV_CODEC_CONFIG_FRAME_RATE:
>-        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, (AVRational){0});
>+        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, AVRational, (AVRational){0});
>     case AV_CODEC_CONFIG_SAMPLE_RATE:
>-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, 0);
>+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, int, 0);
>     case AV_CODEC_CONFIG_SAMPLE_FORMAT:
>-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, AV_SAMPLE_FMT_NONE);
>+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, enum AVSampleFormat, AV_SAMPLE_FMT_NONE);
>     case AV_CODEC_CONFIG_CHANNEL_LAYOUT:
>-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, (AVChannelLayout){0});
>+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, AVChannelLayout, (AVChannelLayout){0});
> FF_ENABLE_DEPRECATION_WARNINGS
> 
>     case AV_CODEC_CONFIG_COLOR_RANGE:
Anton Khirnov Sept. 9, 2024, 10:01 a.m. UTC | #4
Quoting Rémi Denis-Courmont (2024-09-09 09:41:59)
> 
> 
> Le 8 septembre 2024 22:22:01 GMT+03:00, James Almer <jamrial@gmail.com> a écrit :
> >It's non-standard C.
> 
> The description is a little bit misleading. `typeof` is standard C (as of last year). Sure, technically `__typeof__` is not standard but this is easily misinterpreted.

AFAIK C23 is still in draft stage. Not to mention we use C11, where
typeof is indeed not standard.
diff mbox series

Patch

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index cb89236549..78153d12f1 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -708,9 +708,9 @@  int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
     return ff_encode_receive_frame(avctx, frame);
 }
 
-#define WRAP_CONFIG(allowed_type, field, terminator)                        \
+#define WRAP_CONFIG(allowed_type, field, field_type, terminator)            \
     do {                                                                    \
-        static const __typeof__(*(field)) end = terminator;                 \
+        static const field_type end = terminator;                           \
         if (codec->type != (allowed_type))                                  \
             return AVERROR(EINVAL);                                         \
         *out_configs = (field);                                             \
@@ -753,15 +753,15 @@  int ff_default_get_supported_config(const AVCodecContext *avctx,
     switch (config) {
 FF_DISABLE_DEPRECATION_WARNINGS
     case AV_CODEC_CONFIG_PIX_FORMAT:
-        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, AV_PIX_FMT_NONE);
+        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->pix_fmts, enum AVPixelFormat, AV_PIX_FMT_NONE);
     case AV_CODEC_CONFIG_FRAME_RATE:
-        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, (AVRational){0});
+        WRAP_CONFIG(AVMEDIA_TYPE_VIDEO, codec->supported_framerates, AVRational, (AVRational){0});
     case AV_CODEC_CONFIG_SAMPLE_RATE:
-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, 0);
+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->supported_samplerates, int, 0);
     case AV_CODEC_CONFIG_SAMPLE_FORMAT:
-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, AV_SAMPLE_FMT_NONE);
+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->sample_fmts, enum AVSampleFormat, AV_SAMPLE_FMT_NONE);
     case AV_CODEC_CONFIG_CHANNEL_LAYOUT:
-        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, (AVChannelLayout){0});
+        WRAP_CONFIG(AVMEDIA_TYPE_AUDIO, codec->ch_layouts, AVChannelLayout, (AVChannelLayout){0});
 FF_ENABLE_DEPRECATION_WARNINGS
 
     case AV_CODEC_CONFIG_COLOR_RANGE: