diff mbox series

[FFmpeg-devel,v1] avcodec/cbs_vp8: Use little endian in fixed()

Message ID DS7PR11MB79499AF0B5FB03FBF1876EFCB17A2@DS7PR11MB7949.namprd11.prod.outlook.com
State Accepted
Commit 63dea3c1e10a0692bbdadd0916335bbf9dc8147d
Headers show
Series [FFmpeg-devel,v1] avcodec/cbs_vp8: Use little endian in fixed() | 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

Dai, Jianhui J Jan. 25, 2024, 12:54 a.m. UTC 
This commit adds value range checks to cbs_vp8_read_unsigned_le,
migrates fixed() to use it, and enforces little-endian consistency for
all read methods.

Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com>
---
 libavcodec/cbs_vp8.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Dai, Jianhui J Feb. 23, 2024, 12:43 a.m. UTC | #1 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Dai,
> Jianhui J
> Sent: Thursday, January 25, 2024 8:54 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Use little endian in fixed()
> 
> This commit adds value range checks to cbs_vp8_read_unsigned_le, migrates
> fixed() to use it, and enforces little-endian consistency for all read methods.
> 
> Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com>
> ---
>  libavcodec/cbs_vp8.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index
> 065156c248..7a0752019b 100644
> --- a/libavcodec/cbs_vp8.c
> +++ b/libavcodec/cbs_vp8.c
> @@ -181,9 +181,11 @@ static int cbs_vp8_bool_decoder_read_signed(
>      return 0;
>  }
> 
> -static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
> GetBitContext *gbc,
> -                                 int width, const char *name,
> -                                 const int *subscripts, uint32_t *write_to)
> +static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
> +                                    GetBitContext *gbc, int width,
> +                                    const char *name, const int *subscripts,
> +                                    uint32_t *write_to, uint32_t range_min,
> +                                    uint32_t range_max)
>  {
>      int32_t value;
> 
> @@ -200,6 +202,14 @@ static int
> cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
> 
>      CBS_TRACE_READ_END();
> 
> +    if (value < range_min || value > range_max) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR,
> +               "%s out of range: "
> +               "%" PRIu32 ", but must be in [%" PRIu32 ",%" PRIu32 "].\n",
> +               name, value, range_min, range_max);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
>      *write_to = value;
>      return 0;
>  }
> @@ -246,15 +256,16 @@ static int
> cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
>      do { \
>          uint32_t value; \
>          CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, \
> -                                    SUBSCRIPTS(subs, __VA_ARGS__), &value)); \
> +                                       SUBSCRIPTS(subs, __VA_ARGS__), &value, \
> +                                       0, MAX_UINT_BITS(width))); \
>          current->name = value; \
>      } while (0)
> 
>  #define fixed(width, name, value) \
>      do { \
>          uint32_t fixed_value; \
> -        CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, 0, &fixed_value, \
> -                                   value, value)); \
> +        CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, 0, &fixed_value, \
> +                                       value, value)); \
>      } while (0)
> 
>  #define bc_unsigned_subs(width, prob, enable_trace, name, subs, ...) \

Ping reviewers to help to apply. 

The review history can be found here:
https://patchwork.ffmpeg.org/project/ffmpeg/patch/CH3PR11MB793797554CDB411074364733B1742@CH3PR11MB7937.namprd11.prod.outlook.com/

> --
> 2.25.1
> 
> _______________________________________________
> 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".
Ronald S. Bultje March 26, 2024, 1:14 p.m. UTC | #2 
Hi,

On Wed, Jan 24, 2024 at 7:54 PM Dai, Jianhui J <
jianhui.j.dai-at-intel.com@ffmpeg.org> wrote:

> This commit adds value range checks to cbs_vp8_read_unsigned_le,
> migrates fixed() to use it, and enforces little-endian consistency for
> all read methods.
>
> Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com>
> ---
>  libavcodec/cbs_vp8.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c
> index 065156c248..7a0752019b 100644
> --- a/libavcodec/cbs_vp8.c
> +++ b/libavcodec/cbs_vp8.c
> @@ -181,9 +181,11 @@ static int cbs_vp8_bool_decoder_read_signed(
>      return 0;
>  }
>
> -static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
> GetBitContext *gbc,
> -                                 int width, const char *name,
> -                                 const int *subscripts, uint32_t
> *write_to)
> +static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
> +                                    GetBitContext *gbc, int width,
> +                                    const char *name, const int
> *subscripts,
> +                                    uint32_t *write_to, uint32_t
> range_min,
> +                                    uint32_t range_max)
>  {
>      int32_t value;
>
> @@ -200,6 +202,14 @@ static int
> cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
>
>      CBS_TRACE_READ_END();
>
> +    if (value < range_min || value > range_max) {
> +        av_log(ctx->log_ctx, AV_LOG_ERROR,
> +               "%s out of range: "
> +               "%" PRIu32 ", but must be in [%" PRIu32 ",%" PRIu32 "].\n",
> +               name, value, range_min, range_max);
> +        return AVERROR_INVALIDDATA;
> +    }
> +
>      *write_to = value;
>      return 0;
>  }
> @@ -246,15 +256,16 @@ static int
> cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
>      do { \
>          uint32_t value; \
>          CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, \
> -                                    SUBSCRIPTS(subs, __VA_ARGS__),
> &value)); \
> +                                       SUBSCRIPTS(subs, __VA_ARGS__),
> &value, \
> +                                       0, MAX_UINT_BITS(width))); \
>          current->name = value; \
>      } while (0)
>
>  #define fixed(width, name, value) \
>      do { \
>          uint32_t fixed_value; \
> -        CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, 0,
> &fixed_value, \
> -                                   value, value)); \
> +        CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, 0,
> &fixed_value, \
> +                                       value, value)); \
>      } while (0)
>
>  #define bc_unsigned_subs(width, prob, enable_trace, name, subs, ...) \
> --
> 2.25.1
>

Merged.

Ronald
Dai, Jianhui J March 27, 2024, 12:55 a.m. UTC | #3 
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Ronald
> S. Bultje
> Sent: Tuesday, March 26, 2024 9:14 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v1] avcodec/cbs_vp8: Use little endian in
> fixed()
> 
> Hi,
> 
> On Wed, Jan 24, 2024 at 7:54 PM Dai, Jianhui J < jianhui.j.dai-at-
> intel.com@ffmpeg.org> wrote:
> 
> > This commit adds value range checks to cbs_vp8_read_unsigned_le,
> > migrates fixed() to use it, and enforces little-endian consistency for
> > all read methods.
> >
> > Signed-off-by: Jianhui Dai <jianhui.j.dai@intel.com>
> > ---
> >  libavcodec/cbs_vp8.c | 23 +++++++++++++++++------
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c index
> > 065156c248..7a0752019b 100644
> > --- a/libavcodec/cbs_vp8.c
> > +++ b/libavcodec/cbs_vp8.c
> > @@ -181,9 +181,11 @@ static int cbs_vp8_bool_decoder_read_signed(
> >      return 0;
> >  }
> >
> > -static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
> > GetBitContext *gbc,
> > -                                 int width, const char *name,
> > -                                 const int *subscripts, uint32_t
> > *write_to)
> > +static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
> > +                                    GetBitContext *gbc, int width,
> > +                                    const char *name, const int
> > *subscripts,
> > +                                    uint32_t *write_to, uint32_t
> > range_min,
> > +                                    uint32_t range_max)
> >  {
> >      int32_t value;
> >
> > @@ -200,6 +202,14 @@ static int
> > cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
> >
> >      CBS_TRACE_READ_END();
> >
> > +    if (value < range_min || value > range_max) {
> > +        av_log(ctx->log_ctx, AV_LOG_ERROR,
> > +               "%s out of range: "
> > +               "%" PRIu32 ", but must be in [%" PRIu32 ",%" PRIu32 "].\n",
> > +               name, value, range_min, range_max);
> > +        return AVERROR_INVALIDDATA;
> > +    }
> > +
> >      *write_to = value;
> >      return 0;
> >  }
> > @@ -246,15 +256,16 @@ static int
> > cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
> >      do { \
> >          uint32_t value; \
> >          CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, \
> > -                                    SUBSCRIPTS(subs, __VA_ARGS__),
> > &value)); \
> > +                                       SUBSCRIPTS(subs, __VA_ARGS__),
> > &value, \
> > +                                       0, MAX_UINT_BITS(width))); \
> >          current->name = value; \
> >      } while (0)
> >
> >  #define fixed(width, name, value) \
> >      do { \
> >          uint32_t fixed_value; \
> > -        CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, 0,
> > &fixed_value, \
> > -                                   value, value)); \
> > +        CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, 0,
> > &fixed_value, \
> > +                                       value, value)); \
> >      } while (0)
> >
> >  #define bc_unsigned_subs(width, prob, enable_trace, name, subs, ...)
> > \
> > --
> > 2.25.1
> >
> 
> Merged.

Ronald, Thank you!

> 
> Ronald
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/cbs_vp8.c b/libavcodec/cbs_vp8.c
index 065156c248..7a0752019b 100644
--- a/libavcodec/cbs_vp8.c
+++ b/libavcodec/cbs_vp8.c
@@ -181,9 +181,11 @@  static int cbs_vp8_bool_decoder_read_signed(
     return 0;
 }
 
-static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *gbc,
-                                 int width, const char *name,
-                                 const int *subscripts, uint32_t *write_to)
+static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx,
+                                    GetBitContext *gbc, int width,
+                                    const char *name, const int *subscripts,
+                                    uint32_t *write_to, uint32_t range_min,
+                                    uint32_t range_max)
 {
     int32_t value;
 
@@ -200,6 +202,14 @@  static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
 
     CBS_TRACE_READ_END();
 
+    if (value < range_min || value > range_max) {
+        av_log(ctx->log_ctx, AV_LOG_ERROR,
+               "%s out of range: "
+               "%" PRIu32 ", but must be in [%" PRIu32 ",%" PRIu32 "].\n",
+               name, value, range_min, range_max);
+        return AVERROR_INVALIDDATA;
+    }
+
     *write_to = value;
     return 0;
 }
@@ -246,15 +256,16 @@  static int cbs_vp8_read_unsigned_le(CodedBitstreamContext *ctx, GetBitContext *g
     do { \
         uint32_t value; \
         CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, \
-                                    SUBSCRIPTS(subs, __VA_ARGS__), &value)); \
+                                       SUBSCRIPTS(subs, __VA_ARGS__), &value, \
+                                       0, MAX_UINT_BITS(width))); \
         current->name = value; \
     } while (0)
 
 #define fixed(width, name, value) \
     do { \
         uint32_t fixed_value; \
-        CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, 0, &fixed_value, \
-                                   value, value)); \
+        CHECK(cbs_vp8_read_unsigned_le(ctx, rw, width, #name, 0, &fixed_value, \
+                                       value, value)); \
     } while (0)
 
 #define bc_unsigned_subs(width, prob, enable_trace, name, subs, ...) \