[FFmpeg-devel] [PATCH 3/3] avcodec/mpeg4videodec: Replace always true check by assert

[Andreas Rheinhardt]

Michael Niedermayer:
> Maybe helps coverity
> Helps: CID1433771
> 
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/mpeg4videodec.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> index e2bde73639..715cb606c9 100644
> --- a/libavcodec/mpeg4videodec.c
> +++ b/libavcodec/mpeg4videodec.c
> @@ -1981,7 +1981,8 @@ static int mpeg4_decode_studio_block(MpegEncContext *s, int32_t block[64], int n
>                  return AVERROR_INVALIDDATA;
>              j = scantable[idx++];
>              block[j] = get_xbits(&s->gb, additional_code_len);
> -        } else if (group == 21) {
> +        } else {
> +            av_assert2(group == 21);
>              /* Escape */
>              if (idx > 63)
>                  return AVERROR_INVALIDDATA;

This also reminds me of an old attempt of mine to add an AV_UNREACHABLE
macro for such scenarios:
https://github.com/mkver/FFmpeg/commits/unreachable. There are two
reasons why I never sent it to the ML:
a) It uses ASSERT_LEVEL (i.e. with a high ASSERT_LEVEL it degenarates
into an actual assert). But this is only defined internally, so useless
to an API user. Therefore I wonder whether this should be in a public
header (the same issue exists for av_assert1 and av_assert2).
b) Both Clang and MSVC have something more, namely a
__builtin_assume(cond)  resp. __assume(cond). I was unsure whether this
should not be added, too. It could be translated to "if (!(cond))
__builtin_unreachable()" to GCC, but would be more natural in general.
(Of course, cond must not have any side effects.)

- Andreas