diff mbox

[FFmpeg-devel] avcodec/cbs_av1: change the assert in trailing_bits to accept zero bits when reading

Message ID 20190210201200.2480-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Feb. 10, 2019, 8:12 p.m. UTC
If nb_bits is zero when reading an OBU, then it's not a bug in CBS but an
invalid bitstream, and we should abort gracefully instead.

Signed-off-by: James Almer <jamrial@gmail.com>
---
rav1e is currently encoding invalid Metadata OBUs without trailing bits, which
are triggering the assert when parsed by CBS. This change makes sure to instead
report the bitstream as invalid and gracefully return with an error code
instead of crashing.

 libavcodec/cbs_av1_syntax_template.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

James Almer Feb. 10, 2019, 8:45 p.m. UTC | #1
On 2/10/2019 5:12 PM, James Almer wrote:
> If nb_bits is zero when reading an OBU, then it's not a bug in CBS but an
> invalid bitstream, and we should abort gracefully instead.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> rav1e is currently encoding invalid Metadata OBUs without trailing bits, which
> are triggering the assert when parsed by CBS. This change makes sure to instead
> report the bitstream as invalid and gracefully return with an error code
> instead of crashing.
> 
>  libavcodec/cbs_av1_syntax_template.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index 48f4fab514..02b4ed221c 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -45,7 +45,11 @@ static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, RWContext *rw, int nb
>  {
>      int err;
>  
> +#ifdef READ
> +    av_assert0(nb_bits >= 0);
> +#else
>      av_assert0(nb_bits > 0);
> +#endif
>  
>      fixed(1, trailing_one_bit, 1);
>      --nb_bits;

Better version approved by Mark on IRC pushed instead.
Vittorio Giovara Feb. 13, 2019, 9:38 p.m. UTC | #2
On Sun, Feb 10, 2019 at 3:12 PM James Almer <jamrial@gmail.com> wrote:

> If nb_bits is zero when reading an OBU, then it's not a bug in CBS but an
> invalid bitstream, and we should abort gracefully instead.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> rav1e is currently encoding invalid Metadata OBUs without trailing bits,
> which
> are triggering the assert when parsed by CBS. This change makes sure to
> instead
> report the bitstream as invalid and gracefully return with an error code
> instead of crashing.
>

In case anyone is interested, this got fixed tonight, rav1e can produce
conformant bitstreams when encoding metadata now.
diff mbox

Patch

diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
index 48f4fab514..02b4ed221c 100644
--- a/libavcodec/cbs_av1_syntax_template.c
+++ b/libavcodec/cbs_av1_syntax_template.c
@@ -45,7 +45,11 @@  static int FUNC(trailing_bits)(CodedBitstreamContext *ctx, RWContext *rw, int nb
 {
     int err;
 
+#ifdef READ
+    av_assert0(nb_bits >= 0);
+#else
     av_assert0(nb_bits > 0);
+#endif
 
     fixed(1, trailing_one_bit, 1);
     --nb_bits;