[FFmpeg-devel] [PATCH 1/3] avutil/avassert: Add av_assert_unreachable()

[FFmpeg-devel] [PATCH 1/3] avutil/avassert: Add av_assert_unreachable()

[Marvin Scholz]

---
 libavutil/avassert.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/libavutil/avassert.h b/libavutil/avassert.h
index 1895fb7551..cdab912fe4 100644
--- a/libavutil/avassert.h
+++ b/libavutil/avassert.h
@@ -75,4 +75,16 @@
  */
 void av_assert0_fpu(void);
 
+/**
+ * Assert this can not be reached
+ */
+#if AV_HAS_BUILTIN(__builtin_unreachable)
+#define av_assert_unreachable() do {   \
+    av_assert2(0);                      \
+    __builtin_unreachable();            \
+} while (0)
+#else
+#define av_assert_unreachable() av_assert2(0)
+#endif
+
 #endif /* AVUTIL_AVASSERT_H */

base-commit: 85706f5136cf7c88f95843b2634dd3f7d7d2cb6d
-- 
2.39.3 (Apple Git-146)


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

[FFmpeg-devel] [PATCH 2/3] avfilter/af_afftdn: use av_assert_unreachable

[Marvin Scholz]

Fixes a compiler warning about mag possibly being uninitialised.
---
 libavfilter/af_afftdn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/af_afftdn.c b/libavfilter/af_afftdn.c
index a2e6ca6107..f26b620cfb 100644
--- a/libavfilter/af_afftdn.c
+++ b/libavfilter/af_afftdn.c
@@ -378,7 +378,7 @@ static void process_frame(AVFilterContext *ctx,
             noisy_data[i] = mag = hypot(fft_data_dbl[i].re, fft_data_dbl[i].im);
             break;
         default:
-            av_assert2(0);
+            av_assert_unreachable();
         }
 
         power = mag * mag;
-- 
2.39.3 (Apple Git-146)


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

[FFmpeg-devel] [PATCH 3/3] avcodec/pcm-dvdenc: use av_assert_unreachable

[Marvin Scholz]

Fixes a compiler warning about quant possibly being uninitialised.
---
 libavcodec/pcm-dvdenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/pcm-dvdenc.c b/libavcodec/pcm-dvdenc.c
index 71e9b6915a..f553d8076a 100644
--- a/libavcodec/pcm-dvdenc.c
+++ b/libavcodec/pcm-dvdenc.c
@@ -58,7 +58,7 @@ static av_cold int pcm_dvd_encode_init(AVCodecContext *avctx)
         quant = 2;
         break;
     default:
-        av_assert1(0);
+        av_assert_unreachable();
     }
 
     avctx->bits_per_coded_sample = 16 + quant * 4;
-- 
2.39.3 (Apple Git-146)
_______________________________________________
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] [PATCH 1/3] avutil/avassert: Add av_assert_unreachable()

[Andreas Rheinhardt]

Marvin Scholz:
> ---
>  libavutil/avassert.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/libavutil/avassert.h b/libavutil/avassert.h
> index 1895fb7551..cdab912fe4 100644
> --- a/libavutil/avassert.h
> +++ b/libavutil/avassert.h
> @@ -75,4 +75,16 @@
>   */
>  void av_assert0_fpu(void);
>  
> +/**
> + * Assert this can not be reached
> + */
> +#if AV_HAS_BUILTIN(__builtin_unreachable)
> +#define av_assert_unreachable() do {   \
> +    av_assert2(0);                      \
> +    __builtin_unreachable();            \
> +} while (0)
> +#else
> +#define av_assert_unreachable() av_assert2(0)
> +#endif
> +
>  #endif /* AVUTIL_AVASSERT_H */
> 
> base-commit: 85706f5136cf7c88f95843b2634dd3f7d7d2cb6d

You are not the first one with this idea:
https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328116.html
But Michael Niedermayer thinks that adding a new macro instead of
directly reusing av_assert is more complicated.

- Andreas

_______________________________________________
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] [PATCH 1/3] avutil/avassert: Add av_assert_unreachable()

[epirat07]

On 13 Jul 2024, at 2:47, Andreas Rheinhardt wrote:

> Marvin Scholz:
>> ---
>>  libavutil/avassert.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/libavutil/avassert.h b/libavutil/avassert.h
>> index 1895fb7551..cdab912fe4 100644
>> --- a/libavutil/avassert.h
>> +++ b/libavutil/avassert.h
>> @@ -75,4 +75,16 @@
>>   */
>>  void av_assert0_fpu(void);
>>
>> +/**
>> + * Assert this can not be reached
>> + */
>> +#if AV_HAS_BUILTIN(__builtin_unreachable)
>> +#define av_assert_unreachable() do {   \
>> +    av_assert2(0);                      \
>> +    __builtin_unreachable();            \
>> +} while (0)
>> +#else
>> +#define av_assert_unreachable() av_assert2(0)
>> +#endif
>> +
>>  #endif /* AVUTIL_AVASSERT_H */
>>
>> base-commit: 85706f5136cf7c88f95843b2634dd3f7d7d2cb6d
>
> You are not the first one with this idea:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328116.html
> But Michael Niedermayer thinks that adding a new macro instead of
> directly reusing av_assert is more complicated.
>
Oh I did not notice that one, thanks…

I definitely think an av_assert_unreachable() is clearer than
av_assert0(!"reached") too, obviously, else I would not propose it.

(Somewhat proven by these two cases fixed by the patch where we use
av_assert1/2(0) variants which then result in the compiler not knowing
these are unreachable cases when the assert is not enabled.)

If this approach is rejected, I can submit a version that uses
av_assert0(!"reached“).

> - Andreas
>
> _______________________________________________
> 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".
_______________________________________________
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] [PATCH 1/3] avutil/avassert: Add av_assert_unreachable()

[Michael Niedermayer]

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

On Sat, Jul 13, 2024 at 04:44:03PM +0200, epirat07@gmail.com wrote:
> On 13 Jul 2024, at 2:47, Andreas Rheinhardt wrote:
> 
> > Marvin Scholz:
> >> ---
> >>  libavutil/avassert.h | 12 ++++++++++++
> >>  1 file changed, 12 insertions(+)
> >>
> >> diff --git a/libavutil/avassert.h b/libavutil/avassert.h
> >> index 1895fb7551..cdab912fe4 100644
> >> --- a/libavutil/avassert.h
> >> +++ b/libavutil/avassert.h
> >> @@ -75,4 +75,16 @@
> >>   */
> >>  void av_assert0_fpu(void);
> >>
> >> +/**
> >> + * Assert this can not be reached
> >> + */
> >> +#if AV_HAS_BUILTIN(__builtin_unreachable)
> >> +#define av_assert_unreachable() do {   \
> >> +    av_assert2(0);                      \
> >> +    __builtin_unreachable();            \
> >> +} while (0)
> >> +#else
> >> +#define av_assert_unreachable() av_assert2(0)
> >> +#endif
> >> +
> >>  #endif /* AVUTIL_AVASSERT_H */
> >>
> >> base-commit: 85706f5136cf7c88f95843b2634dd3f7d7d2cb6d
> >
> > You are not the first one with this idea:
> > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/328116.html
> > But Michael Niedermayer thinks that adding a new macro instead of
> > directly reusing av_assert is more complicated.
> >
> Oh I did not notice that one, thanks…
> 
> I definitely think an av_assert_unreachable() is clearer than
> av_assert0(!"reached") too, obviously, else I would not propose it.
> 
> (Somewhat proven by these two cases fixed by the patch where we use
> av_assert1/2(0) variants which then result in the compiler not knowing
> these are unreachable cases when the assert is not enabled.)
> 
> If this approach is rejected, I can submit a version that uses
> av_assert0(!"reached“).

I think you look at this from the wrong side

We have 3 levels of assert()
and we have one threshold to decide how much we want to check at runtime

what we do NOT have is a threshold at what we want the compiler to assume
teh statment is true
And here this av_assert1(0) is not assumed true and not checked at runtime
so the compiler thinks this is reachable

I think the solution is to add this 2nd threshold so we can gloablly
use av_assert*() for optimization
This should remove the warning with no changes to the code

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates

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