diff mbox series

[FFmpeg-devel] avcodec/hevc_sei: Fix check for SEI end

Message ID DB6PR0101MB2214AB938D29E67A59B9D1258FB29@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com
State Accepted
Commit 70dc5fc658567f2d1d3f090f21ece364054a999c
Headers show
Series [FFmpeg-devel] avcodec/hevc_sei: Fix check for SEI end | expand

Checks

Context Check Description
yinshiyou/configure_loongarch64 warning Failed to apply patch
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_armv7_RPi4 success Make finished
andriy/make_fate_armv7_RPi4 success Make fate finished

Commit Message

Andreas Rheinhardt June 22, 2022, 11:26 a.m. UTC 
The intention behind the current check seems to be to check for
the rbsp_trailing_bits() syntax structure which is always 0x80
for valid SEI messages. Yet this is wrong: These trailing bits
are not part of the GetBitContext -- they have already been
stripped in ff_h2645_packet_split(). And it is harmful, as
0x80 is a legal SEI message payload type (namely for
Structure of pictures information SEI messages). We ignore this
type of SEI, but because of this bug we also ignored every
SEI message in the same NALU following it.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/hevc_sei.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Comments

Andreas Rheinhardt June 24, 2022, 9:53 a.m. UTC | #1 
Andreas Rheinhardt:
> The intention behind the current check seems to be to check for
> the rbsp_trailing_bits() syntax structure which is always 0x80
> for valid SEI messages. Yet this is wrong: These trailing bits
> are not part of the GetBitContext -- they have already been
> stripped in ff_h2645_packet_split(). And it is harmful, as
> 0x80 is a legal SEI message payload type (namely for
> Structure of pictures information SEI messages). We ignore this
> type of SEI, but because of this bug we also ignored every
> SEI message in the same NALU following it.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/hevc_sei.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
> index a5c7df34b0..953633f4bd 100644
> --- a/libavcodec/hevc_sei.c
> +++ b/libavcodec/hevc_sei.c
> @@ -549,12 +549,6 @@ static int decode_nal_sei_message(GetByteContext *gb, void *logctx, HEVCSEI *s,
>      }
>  }
>  
> -static int more_rbsp_data(GetByteContext *gb)
> -{
> -    return bytestream2_get_bytes_left(gb) > 0 &&
> -           bytestream2_peek_byteu(gb) != 0x80;
> -}
> -
>  int ff_hevc_decode_nal_sei(GetBitContext *gb, void *logctx, HEVCSEI *s,
>                             const HEVCParamSets *ps, int type)
>  {
> @@ -569,7 +563,7 @@ int ff_hevc_decode_nal_sei(GetBitContext *gb, void *logctx, HEVCSEI *s,
>          ret = decode_nal_sei_message(&gbyte, logctx, s, ps, type);
>          if (ret < 0)
>              return ret;
> -    } while (more_rbsp_data(&gbyte));
> +    } while (bytestream2_get_bytes_left(&gbyte) > 0);
>      return 1;
>  }
>  

Will apply this tonight unless there are objections.

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/hevc_sei.c b/libavcodec/hevc_sei.c
index a5c7df34b0..953633f4bd 100644
--- a/libavcodec/hevc_sei.c
+++ b/libavcodec/hevc_sei.c
@@ -549,12 +549,6 @@  static int decode_nal_sei_message(GetByteContext *gb, void *logctx, HEVCSEI *s,
     }
 }
 
-static int more_rbsp_data(GetByteContext *gb)
-{
-    return bytestream2_get_bytes_left(gb) > 0 &&
-           bytestream2_peek_byteu(gb) != 0x80;
-}
-
 int ff_hevc_decode_nal_sei(GetBitContext *gb, void *logctx, HEVCSEI *s,
                            const HEVCParamSets *ps, int type)
 {
@@ -569,7 +563,7 @@  int ff_hevc_decode_nal_sei(GetBitContext *gb, void *logctx, HEVCSEI *s,
         ret = decode_nal_sei_message(&gbyte, logctx, s, ps, type);
         if (ret < 0)
             return ret;
-    } while (more_rbsp_data(&gbyte));
+    } while (bytestream2_get_bytes_left(&gbyte) > 0);
     return 1;
 }