| Message ID | 20200322230043.11552-1-michael@niedermayer.cc |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/cbs_h264_syntax_template: fix off by 1 error with slice_group_change_cycle | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On 3/22/2020 8:00 PM, Michael Niedermayer wrote: > Fixes: assertion failure > Fixes: 20390/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5683400772157440 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cbs_h264_syntax_template.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c > index 878d348b94..b7e796e9a8 100644 > --- a/libavcodec/cbs_h264_syntax_template.c > +++ b/libavcodec/cbs_h264_syntax_template.c > @@ -1365,7 +1365,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, > pic_size = (sps->pic_width_in_mbs_minus1 + 1) * > (sps->pic_height_in_map_units_minus1 + 1); > max = (pic_size + pps->slice_group_change_rate_minus1) / > - (pps->slice_group_change_rate_minus1 + 1); > + (pps->slice_group_change_rate_minus1 + 1) + 1; The spec says "The value of slice_group_change_cycle is represented in the bitstream by the following number of bits Ceil( Log2( PicSizeInMapUnits ÷ SliceGroupChangeRate + 1 ) ) The value of slice_group_change_cycle shall be in the range of 0 to Ceil( PicSizeInMapUnits ÷ SliceGroupChangeRate ), inclusive." Are you trying to change the amount of bits read, the max allowed value, or both? > bits = av_log2(2 * max - 1); This should use av_ceil_log2() IMO, so it's clear what it's trying to do. > > u(bits, slice_group_change_cycle, 0, max); >
On Mon, Mar 23, 2020 at 12:30:31AM -0300, James Almer wrote: > On 3/22/2020 8:00 PM, Michael Niedermayer wrote: > > Fixes: assertion failure > > Fixes: 20390/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5683400772157440 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/cbs_h264_syntax_template.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c > > index 878d348b94..b7e796e9a8 100644 > > --- a/libavcodec/cbs_h264_syntax_template.c > > +++ b/libavcodec/cbs_h264_syntax_template.c > > @@ -1365,7 +1365,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, > > pic_size = (sps->pic_width_in_mbs_minus1 + 1) * > > (sps->pic_height_in_map_units_minus1 + 1); > > max = (pic_size + pps->slice_group_change_rate_minus1) / > > - (pps->slice_group_change_rate_minus1 + 1); > > + (pps->slice_group_change_rate_minus1 + 1) + 1; > > The spec says > > "The value of slice_group_change_cycle is represented in the bitstream > by the following number of bits Ceil( Log2( PicSizeInMapUnits ÷ > SliceGroupChangeRate + 1 ) ) > > The value of slice_group_change_cycle shall be in the range of 0 to > Ceil( PicSizeInMapUnits ÷ SliceGroupChangeRate ), inclusive." > > Are you trying to change the amount of bits read, the max allowed value, > or both? the bits, ill send a better patch thanks
diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c index 878d348b94..b7e796e9a8 100644 --- a/libavcodec/cbs_h264_syntax_template.c +++ b/libavcodec/cbs_h264_syntax_template.c @@ -1365,7 +1365,7 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, pic_size = (sps->pic_width_in_mbs_minus1 + 1) * (sps->pic_height_in_map_units_minus1 + 1); max = (pic_size + pps->slice_group_change_rate_minus1) / - (pps->slice_group_change_rate_minus1 + 1); + (pps->slice_group_change_rate_minus1 + 1) + 1; bits = av_log2(2 * max - 1); u(bits, slice_group_change_cycle, 0, max);
Fixes: assertion failure Fixes: 20390/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5683400772157440 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/cbs_h264_syntax_template.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)