Message ID | 20190415211734.3968-5-jamrial@gmail.com |
---|---|
State | Accepted |
Commit | e2f766e13f0350017a0c3c7111b54cda7317e500 |
Headers | show |
On 15/04/2019 22:17, James Almer wrote: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > The sample https://0x0.st/sljR.webm appears to be parsed the exact same way > after this patch. > > libavcodec/cbs_av1.c | 68 ++++++-------------------------------------- > 1 file changed, 8 insertions(+), 60 deletions(-) > > diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c > index 41dd4c97ac..eb2d03ef43 100644 > --- a/libavcodec/cbs_av1.c > +++ b/libavcodec/cbs_av1.c > @@ -207,62 +207,6 @@ static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc, > return 0; > } > > -static int cbs_av1_read_su(CodedBitstreamContext *ctx, GetBitContext *gbc, > - int width, const char *name, > - const int *subscripts, int32_t *write_to) > -{ > - int position; > - int32_t value; > - > - if (ctx->trace_enable) > - position = get_bits_count(gbc); > - > - if (get_bits_left(gbc) < width) { > - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid signed value at " > - "%s: bitstream ended.\n", name); > - return AVERROR_INVALIDDATA; > - } > - > - value = get_sbits(gbc, width); > - > - if (ctx->trace_enable) { > - char bits[33]; > - int i; > - for (i = 0; i < width; i++) > - bits[i] = value & (1 << (width - i - 1)) ? '1' : '0'; Ohright, it was bad here too. (But only ever used on small values in AV1, so it couldn't actually be hit.) > - bits[i] = 0; > - > - ff_cbs_trace_syntax_element(ctx, position, > - name, subscripts, bits, value); > - } > - > - *write_to = value; > - return 0; > -} > - > -static int cbs_av1_write_su(CodedBitstreamContext *ctx, PutBitContext *pbc, > - int width, const char *name, > - const int *subscripts, int32_t value) > -{ > - if (put_bits_left(pbc) < width) > - return AVERROR(ENOSPC); > - > - if (ctx->trace_enable) { > - char bits[33]; > - int i; > - for (i = 0; i < width; i++) > - bits[i] = value & (1 << (width - i - 1)) ? '1' : '0'; > - bits[i] = 0; > - > - ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc), > - name, subscripts, bits, value); > - } > - > - put_sbits(pbc, width, value); > - > - return 0; > -} > - > static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc, > uint32_t n, const char *name, > const int *subscripts, uint32_t *write_to) > @@ -639,8 +583,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc) > > #define xsu(width, name, var, subs, ...) do { \ > int32_t value = 0; \ > - CHECK(cbs_av1_read_su(ctx, rw, width, #name, \ > - SUBSCRIPTS(subs, __VA_ARGS__), &value)); \ > + CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \ > + SUBSCRIPTS(subs, __VA_ARGS__), &value, \ > + MIN_INT_BITS(width), \ > + MAX_INT_BITS(width))); \ > var = value; \ > } while (0) > > @@ -723,8 +669,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc) > } while (0) > > #define xsu(width, name, var, subs, ...) do { \ > - CHECK(cbs_av1_write_su(ctx, rw, width, #name, \ > - SUBSCRIPTS(subs, __VA_ARGS__), var)); \ > + CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \ > + SUBSCRIPTS(subs, __VA_ARGS__), var, \ > + MIN_INT_BITS(width), \ > + MAX_INT_BITS(width))); \ > } while (0) > > #define uvlc(name, range_min, range_max) do { \ > LGTM. Thanks, - Mark
On 4/16/2019 8:10 PM, Mark Thompson wrote: > On 15/04/2019 22:17, James Almer wrote: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> The sample https://0x0.st/sljR.webm appears to be parsed the exact same way >> after this patch. >> >> libavcodec/cbs_av1.c | 68 ++++++-------------------------------------- >> 1 file changed, 8 insertions(+), 60 deletions(-) >> >> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c >> index 41dd4c97ac..eb2d03ef43 100644 >> --- a/libavcodec/cbs_av1.c >> +++ b/libavcodec/cbs_av1.c >> @@ -207,62 +207,6 @@ static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc, >> return 0; >> } >> >> -static int cbs_av1_read_su(CodedBitstreamContext *ctx, GetBitContext *gbc, >> - int width, const char *name, >> - const int *subscripts, int32_t *write_to) >> -{ >> - int position; >> - int32_t value; >> - >> - if (ctx->trace_enable) >> - position = get_bits_count(gbc); >> - >> - if (get_bits_left(gbc) < width) { >> - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid signed value at " >> - "%s: bitstream ended.\n", name); >> - return AVERROR_INVALIDDATA; >> - } >> - >> - value = get_sbits(gbc, width); >> - >> - if (ctx->trace_enable) { >> - char bits[33]; >> - int i; >> - for (i = 0; i < width; i++) >> - bits[i] = value & (1 << (width - i - 1)) ? '1' : '0'; > > Ohright, it was bad here too. (But only ever used on small values in AV1, so it couldn't actually be hit.) > >> - bits[i] = 0; >> - >> - ff_cbs_trace_syntax_element(ctx, position, >> - name, subscripts, bits, value); >> - } >> - >> - *write_to = value; >> - return 0; >> -} >> - >> -static int cbs_av1_write_su(CodedBitstreamContext *ctx, PutBitContext *pbc, >> - int width, const char *name, >> - const int *subscripts, int32_t value) >> -{ >> - if (put_bits_left(pbc) < width) >> - return AVERROR(ENOSPC); >> - >> - if (ctx->trace_enable) { >> - char bits[33]; >> - int i; >> - for (i = 0; i < width; i++) >> - bits[i] = value & (1 << (width - i - 1)) ? '1' : '0'; >> - bits[i] = 0; >> - >> - ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc), >> - name, subscripts, bits, value); >> - } >> - >> - put_sbits(pbc, width, value); >> - >> - return 0; >> -} >> - >> static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc, >> uint32_t n, const char *name, >> const int *subscripts, uint32_t *write_to) >> @@ -639,8 +583,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc) >> >> #define xsu(width, name, var, subs, ...) do { \ >> int32_t value = 0; \ >> - CHECK(cbs_av1_read_su(ctx, rw, width, #name, \ >> - SUBSCRIPTS(subs, __VA_ARGS__), &value)); \ >> + CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \ >> + SUBSCRIPTS(subs, __VA_ARGS__), &value, \ >> + MIN_INT_BITS(width), \ >> + MAX_INT_BITS(width))); \ >> var = value; \ >> } while (0) >> >> @@ -723,8 +669,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc) >> } while (0) >> >> #define xsu(width, name, var, subs, ...) do { \ >> - CHECK(cbs_av1_write_su(ctx, rw, width, #name, \ >> - SUBSCRIPTS(subs, __VA_ARGS__), var)); \ >> + CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \ >> + SUBSCRIPTS(subs, __VA_ARGS__), var, \ >> + MIN_INT_BITS(width), \ >> + MAX_INT_BITS(width))); \ >> } while (0) >> >> #define uvlc(name, range_min, range_max) do { \ >> > > LGTM. > > Thanks, > > - Mark Pushed, thanks!
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c index 41dd4c97ac..eb2d03ef43 100644 --- a/libavcodec/cbs_av1.c +++ b/libavcodec/cbs_av1.c @@ -207,62 +207,6 @@ static int cbs_av1_write_leb128(CodedBitstreamContext *ctx, PutBitContext *pbc, return 0; } -static int cbs_av1_read_su(CodedBitstreamContext *ctx, GetBitContext *gbc, - int width, const char *name, - const int *subscripts, int32_t *write_to) -{ - int position; - int32_t value; - - if (ctx->trace_enable) - position = get_bits_count(gbc); - - if (get_bits_left(gbc) < width) { - av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid signed value at " - "%s: bitstream ended.\n", name); - return AVERROR_INVALIDDATA; - } - - value = get_sbits(gbc, width); - - if (ctx->trace_enable) { - char bits[33]; - int i; - for (i = 0; i < width; i++) - bits[i] = value & (1 << (width - i - 1)) ? '1' : '0'; - bits[i] = 0; - - ff_cbs_trace_syntax_element(ctx, position, - name, subscripts, bits, value); - } - - *write_to = value; - return 0; -} - -static int cbs_av1_write_su(CodedBitstreamContext *ctx, PutBitContext *pbc, - int width, const char *name, - const int *subscripts, int32_t value) -{ - if (put_bits_left(pbc) < width) - return AVERROR(ENOSPC); - - if (ctx->trace_enable) { - char bits[33]; - int i; - for (i = 0; i < width; i++) - bits[i] = value & (1 << (width - i - 1)) ? '1' : '0'; - bits[i] = 0; - - ff_cbs_trace_syntax_element(ctx, put_bits_count(pbc), - name, subscripts, bits, value); - } - - put_sbits(pbc, width, value); - - return 0; -} - static int cbs_av1_read_ns(CodedBitstreamContext *ctx, GetBitContext *gbc, uint32_t n, const char *name, const int *subscripts, uint32_t *write_to) @@ -639,8 +583,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc) #define xsu(width, name, var, subs, ...) do { \ int32_t value = 0; \ - CHECK(cbs_av1_read_su(ctx, rw, width, #name, \ - SUBSCRIPTS(subs, __VA_ARGS__), &value)); \ + CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \ + SUBSCRIPTS(subs, __VA_ARGS__), &value, \ + MIN_INT_BITS(width), \ + MAX_INT_BITS(width))); \ var = value; \ } while (0) @@ -723,8 +669,10 @@ static size_t cbs_av1_get_payload_bytes_left(GetBitContext *gbc) } while (0) #define xsu(width, name, var, subs, ...) do { \ - CHECK(cbs_av1_write_su(ctx, rw, width, #name, \ - SUBSCRIPTS(subs, __VA_ARGS__), var)); \ + CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \ + SUBSCRIPTS(subs, __VA_ARGS__), var, \ + MIN_INT_BITS(width), \ + MAX_INT_BITS(width))); \ } while (0) #define uvlc(name, range_min, range_max) do { \
Signed-off-by: James Almer <jamrial@gmail.com> --- The sample https://0x0.st/sljR.webm appears to be parsed the exact same way after this patch. libavcodec/cbs_av1.c | 68 ++++++-------------------------------------- 1 file changed, 8 insertions(+), 60 deletions(-)