On 2017/6/7 11:17, Jun Zhao wrote: > > > On 2017/6/7 9:22, Michael Niedermayer wrote: >> On Mon, Jun 05, 2017 at 08:43:35AM +0800, Jun Zhao wrote: >>> V2: Add Add set_ue_golomb_long() to support 32bits UE golomb and update the >>> unit test. >> >>> golomb.h | 20 +++++++++++++++++++- >>> put_bits.h | 35 +++++++++++++++++++++++++++++++++++ >>> tests/golomb.c | 19 +++++++++++++++++++ >>> 3 files changed, 73 insertions(+), 1 deletion(-) >>> 491565dd491fc4ebd1717069d9c7655bfe0bd08a >>> 0001-lavc-golomb-Fix-UE-golomb-overwrite-issue.patch >>> From 6fe36e4e2a41f70e2a41c5eba90b5143b4eeba7b Mon Sep 17 00:00:00 2001 >>> From: Jun Zhao <jun.z...@intel.com> >>> Date: Fri, 2 Jun 2017 15:05:49 +0800 >>> Subject: [PATCH V2] lavc/golomb: Fix UE golomb overwrite issue. >>> >>> put_bits just support write up to 31 bits, when write 32 bit in >>> put_bits, it's will overwrite the bit buffer, because the default >>> assert level is 0, the av_assert2(n <= 31 && value < (1U << n)) >>> in put_bits can not be trigger runtime. Add set_ue_golomb_long() >>> to support 32bits UE golomb. >>> >>> Signed-off-by: Jun Zhao <jun.z...@intel.com> >>> --- >>> libavcodec/golomb.h | 20 +++++++++++++++++++- >>> libavcodec/put_bits.h | 35 +++++++++++++++++++++++++++++++++++ >>> libavcodec/tests/golomb.c | 19 +++++++++++++++++++ >>> 3 files changed, 73 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h >>> index 0833aff468..47ab884282 100644 >>> --- a/libavcodec/golomb.h >>> +++ b/libavcodec/golomb.h >>> @@ -458,7 +458,7 @@ static inline int get_te(GetBitContext *s, int r, char >>> *file, const char *func, >>> #endif /* TRACE */ >>> >>> /** >>> - * write unsigned exp golomb code. >>> + * write unsigned exp golomb code. 2^16-2 at most. >>> */ >>> static inline void set_ue_golomb(PutBitContext *pb, int i) >>> { >>> @@ -473,6 +473,24 @@ static inline void set_ue_golomb(PutBitContext *pb, >>> int i) >>> } >>> >>> /** >>> + * write unsigned exp golomb code. 2^32-2 at most. >>> + */ >>> +static inline void set_ue_golomb_long(PutBitContext *pb, uint32_t i) >>> +{ >>> + av_assert2(i <= (0xffffffff - 2)); >>> + >>> + if (i < 256) >>> + put_bits(pb, ff_ue_golomb_len[i], i + 1); >>> + else { >> >> Please add {} for if else so its if { } else >> > > Ok, will add {} for if. > >>> + int e = av_log2(i + 1); >>> + if (e < 16) >>> + put_bits(pb, 2 * e + 1, i + 1); >>> + else >> >>> + put_bits64(pb, 2 * e + 1, i + 1); >> >> put_bits64 tests for <32 it tests for ==64 neither are possible >> here. And this is a inline function so these impossible code pathes >> might get duplicated many times >> >> [...] > > I think av_assert2(i <= (0xffffffff - 2)) have cover this condition, and maybe > av_assert0(i <= (0xffffffff - 2)) is a better choice for this assert. >
I make a mistake for this comment, will clean the code logic to use put_bit64 when e >=16 >> >> >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel