| Message ID | a25e1917-2430-ddc2-ff2c-2fd0cf3e1645@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
2017-05-26 3:19 GMT+02:00 Jun Zhao <mypopydev@gmail.com>: Please explain how we can reproduce the issue you want to fix. Carl Eugen
On 2017/5/26 17:32, Carl Eugen Hoyos wrote: > 2017-05-26 3:19 GMT+02:00 Jun Zhao <mypopydev@gmail.com>: > > Please explain how we can reproduce the issue you want to fix. > > Carl Eugen In h264 Spec, the SPS VUI.hrd.bit_rate_value_minus1 is a ue(v) and "bit_rate_value_minus1[ SchedSelIdx ] shall be in the range of 0 to 2^32 − 2". more details in the Spec E.2.2 HRD parameters semantics http://www.itu.int/rec/T-REC-H.264-201610-S/en page 424. In this case, I think call set_ue_golome() will overwrite the bit stream buffer when SPS VUI.hrd.bit_rate_value_minus1 >= 2^16. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Fri, May 26, 2017 at 09:19:09AM +0800, Jun Zhao wrote: > golomb.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > 228c7180856b65d095dd0b8d59f3d3ff4f65774a 0001-lavc-golomb-Fix-UE-golomb-overwrite-issue.patch > From eabcbf3d41e83f24623e6195d4a0ff86e4d95a80 Mon Sep 17 00:00:00 2001 > From: Jun Zhao <jun.zhao@intel.com> > Date: Fri, 26 May 2017 09:02:29 +0800 > Subject: [PATCH] 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. > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > --- > libavcodec/golomb.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h > index 0833aff468..2c5a969ac1 100644 > --- a/libavcodec/golomb.h > +++ b/libavcodec/golomb.h > @@ -468,7 +468,10 @@ static inline void set_ue_golomb(PutBitContext *pb, int i) > put_bits(pb, ff_ue_golomb_len[i], i + 1); > else { > int e = av_log2(i + 1); > - put_bits(pb, 2 * e + 1, i + 1); > + if (e < 16) > + put_bits(pb, 2 * e + 1, i + 1); > + else > + put_bits32(pb, i + 1); this is wrong if e is 16 or larger the length is 33 bits or longer its never 32 [...]
On 2017/5/29 7:40, Michael Niedermayer wrote: > On Fri, May 26, 2017 at 09:19:09AM +0800, Jun Zhao wrote: >> golomb.h | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> 228c7180856b65d095dd0b8d59f3d3ff4f65774a 0001-lavc-golomb-Fix-UE-golomb-overwrite-issue.patch >> From eabcbf3d41e83f24623e6195d4a0ff86e4d95a80 Mon Sep 17 00:00:00 2001 >> From: Jun Zhao <jun.zhao@intel.com> >> Date: Fri, 26 May 2017 09:02:29 +0800 >> Subject: [PATCH] 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. >> >> Signed-off-by: Jun Zhao <jun.zhao@intel.com> >> --- >> libavcodec/golomb.h | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h >> index 0833aff468..2c5a969ac1 100644 >> --- a/libavcodec/golomb.h >> +++ b/libavcodec/golomb.h >> @@ -468,7 +468,10 @@ static inline void set_ue_golomb(PutBitContext *pb, int i) >> put_bits(pb, ff_ue_golomb_len[i], i + 1); >> else { >> int e = av_log2(i + 1); >> - put_bits(pb, 2 * e + 1, i + 1); >> + if (e < 16) >> + put_bits(pb, 2 * e + 1, i + 1); >> + else >> + put_bits32(pb, i + 1); > > this is wrong > > if e is 16 or larger the length is 33 bits or longer its never > 32 > Yes, this is a wrong fix, how about add set_ue_golomb_long()/ set_ue_golomb_64() for this case ? > [...] > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Wed, May 31, 2017 at 09:01:47AM +0800, Jun Zhao wrote: > > > On 2017/5/29 7:40, Michael Niedermayer wrote: > > On Fri, May 26, 2017 at 09:19:09AM +0800, Jun Zhao wrote: > >> golomb.h | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> 228c7180856b65d095dd0b8d59f3d3ff4f65774a 0001-lavc-golomb-Fix-UE-golomb-overwrite-issue.patch > >> From eabcbf3d41e83f24623e6195d4a0ff86e4d95a80 Mon Sep 17 00:00:00 2001 > >> From: Jun Zhao <jun.zhao@intel.com> > >> Date: Fri, 26 May 2017 09:02:29 +0800 > >> Subject: [PATCH] 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. > >> > >> Signed-off-by: Jun Zhao <jun.zhao@intel.com> > >> --- > >> libavcodec/golomb.h | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h > >> index 0833aff468..2c5a969ac1 100644 > >> --- a/libavcodec/golomb.h > >> +++ b/libavcodec/golomb.h > >> @@ -468,7 +468,10 @@ static inline void set_ue_golomb(PutBitContext *pb, int i) > >> put_bits(pb, ff_ue_golomb_len[i], i + 1); > >> else { > >> int e = av_log2(i + 1); > >> - put_bits(pb, 2 * e + 1, i + 1); > >> + if (e < 16) > >> + put_bits(pb, 2 * e + 1, i + 1); > >> + else > >> + put_bits32(pb, i + 1); > > > > this is wrong > > > > if e is 16 or larger the length is 33 bits or longer its never > > 32 > > > > Yes, this is a wrong fix, how about add set_ue_golomb_long()/ set_ue_golomb_64() for this case ? yes, thats probably a good idea [...]
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h index 0833aff468..2c5a969ac1 100644 --- a/libavcodec/golomb.h +++ b/libavcodec/golomb.h @@ -468,7 +468,10 @@ static inline void set_ue_golomb(PutBitContext *pb, int i) put_bits(pb, ff_ue_golomb_len[i], i + 1); else { int e = av_log2(i + 1); - put_bits(pb, 2 * e + 1, i + 1); + if (e < 16) + put_bits(pb, 2 * e + 1, i + 1); + else + put_bits32(pb, i + 1); } }