| Message ID | 20220429221826.31666-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/2] tools/target_dec_fuzzer: Adjust threshold or CFHD | expand |
| 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 |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On Sat, Apr 30, 2022 at 12:18:26AM +0200, Michael Niedermayer wrote:
> Fixes: Assertion failure
it was this
av_assert1((get_bits_count(gb) % 8) == 0);
that failed IIRC
[...]
Michael Niedermayer: > Fixes: Assertion failure > Fixes: 46662/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-4947860854013952 > > This also results in more frames to be decoded from fate samples > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/h2645_parse.c | 2 +- > .../ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 | 10 ++++++++++ > tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 | 3 +++ > 3 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > index 03780680c6..78ab22b76e 100644 > --- a/libavcodec/h2645_parse.c > +++ b/libavcodec/h2645_parse.c > @@ -292,7 +292,7 @@ static int hevc_parse_nal_header(H2645NAL *nal, void *logctx) > { > GetBitContext *gb = &nal->gb; > > - if (get_bits1(gb) != 0) > + if (get_bits_left(gb) < 16 || get_bits1(gb) != 0) > return AVERROR_INVALIDDATA; > > nal->type = get_bits(gb, 6); > diff --git a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > index 0c930f6556..3283925e38 100644 > --- a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > +++ b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > @@ -25,6 +25,16 @@ > 0, 19, 19, 1, 599040, 0x4227009b > 0, 20, 20, 1, 599040, 0x1bda8be4 > 0, 21, 21, 1, 599040, 0xd1d5dcb4 > +0, 22, 22, 1, 599040, 0x58b2edb3 > +0, 23, 23, 1, 599040, 0xd1f795d8 > +0, 24, 24, 1, 599040, 0x3331d5e6 > +0, 25, 25, 1, 599040, 0x5e5ec2c9 > +0, 26, 26, 1, 599040, 0x3b907bf5 > +0, 27, 27, 1, 599040, 0xefcbf471 > +0, 28, 28, 1, 599040, 0x2769a578 > +0, 29, 29, 1, 599040, 0x812ce986 > +0, 30, 30, 1, 599040, 0xf07c212c > +0, 31, 31, 1, 599040, 0xb5476890 > 0, 32, 32, 1, 599040, 0x00a0249f > 0, 33, 33, 1, 599040, 0x7263f7cf > 0, 34, 34, 1, 599040, 0x47054be4 > diff --git a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > index e661ff245e..776267b59c 100644 > --- a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > +++ b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > @@ -70,6 +70,9 @@ > 0, 64, 64, 1, 149760, 0x3362678b > 0, 65, 65, 1, 149760, 0x6e7fc851 > 0, 66, 66, 1, 149760, 0x33f96449 > +0, 67, 67, 1, 149760, 0xd9d05007 > +0, 75, 75, 1, 149760, 0x477f2cf2 > +0, 76, 76, 1, 149760, 0xe1f9ccd0 > 0, 77, 77, 1, 149760, 0xb3ba8cfb > 0, 78, 78, 1, 149760, 0x64787995 > 0, 79, 79, 1, 149760, 0xc10de4c4 get_bit_length currently presumes every NALU to contain rbsp_trailing_bits. Yet this is not true for the End of Sequence/Bitstream units which are just headers without RBSP. For these units, get_bit_length might truncate them -- it does so for end of sequence units in H.264. It would not be a serious issue for H.265, as the semantics of nuh_temporal_id_plus1 require nuh_temporal_id_plus1 to be 1 for End of Sequence/Bitstream units. Nevertheless I think this should be coupled with a patch that does not truncate the NAL unit if it is just a header. - Andreas
Andreas Rheinhardt: > Michael Niedermayer: >> Fixes: Assertion failure >> Fixes: 46662/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-4947860854013952 >> >> This also results in more frames to be decoded from fate samples >> >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> --- >> libavcodec/h2645_parse.c | 2 +- >> .../ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 | 10 ++++++++++ >> tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 | 3 +++ >> 3 files changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c >> index 03780680c6..78ab22b76e 100644 >> --- a/libavcodec/h2645_parse.c >> +++ b/libavcodec/h2645_parse.c >> @@ -292,7 +292,7 @@ static int hevc_parse_nal_header(H2645NAL *nal, void *logctx) >> { >> GetBitContext *gb = &nal->gb; >> >> - if (get_bits1(gb) != 0) >> + if (get_bits_left(gb) < 16 || get_bits1(gb) != 0) >> return AVERROR_INVALIDDATA; >> >> nal->type = get_bits(gb, 6); >> diff --git a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 >> index 0c930f6556..3283925e38 100644 >> --- a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 >> +++ b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 >> @@ -25,6 +25,16 @@ >> 0, 19, 19, 1, 599040, 0x4227009b >> 0, 20, 20, 1, 599040, 0x1bda8be4 >> 0, 21, 21, 1, 599040, 0xd1d5dcb4 >> +0, 22, 22, 1, 599040, 0x58b2edb3 >> +0, 23, 23, 1, 599040, 0xd1f795d8 >> +0, 24, 24, 1, 599040, 0x3331d5e6 >> +0, 25, 25, 1, 599040, 0x5e5ec2c9 >> +0, 26, 26, 1, 599040, 0x3b907bf5 >> +0, 27, 27, 1, 599040, 0xefcbf471 >> +0, 28, 28, 1, 599040, 0x2769a578 >> +0, 29, 29, 1, 599040, 0x812ce986 >> +0, 30, 30, 1, 599040, 0xf07c212c >> +0, 31, 31, 1, 599040, 0xb5476890 >> 0, 32, 32, 1, 599040, 0x00a0249f >> 0, 33, 33, 1, 599040, 0x7263f7cf >> 0, 34, 34, 1, 599040, 0x47054be4 >> diff --git a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 >> index e661ff245e..776267b59c 100644 >> --- a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 >> +++ b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 >> @@ -70,6 +70,9 @@ >> 0, 64, 64, 1, 149760, 0x3362678b >> 0, 65, 65, 1, 149760, 0x6e7fc851 >> 0, 66, 66, 1, 149760, 0x33f96449 >> +0, 67, 67, 1, 149760, 0xd9d05007 >> +0, 75, 75, 1, 149760, 0x477f2cf2 >> +0, 76, 76, 1, 149760, 0xe1f9ccd0 >> 0, 77, 77, 1, 149760, 0xb3ba8cfb >> 0, 78, 78, 1, 149760, 0x64787995 >> 0, 79, 79, 1, 149760, 0xc10de4c4 > > get_bit_length currently presumes every NALU to contain > rbsp_trailing_bits. Yet this is not true for the End of > Sequence/Bitstream units which are just headers without RBSP. For these > units, get_bit_length might truncate them -- it does so for end of > sequence units in H.264. It would not be a serious issue for H.265, as > the semantics of nuh_temporal_id_plus1 require nuh_temporal_id_plus1 to > be 1 for End of Sequence/Bitstream units. Nevertheless I think this > should be coupled with a patch that does not truncate the NAL unit if it > is just a header. > 1. I just sent a patch implementing the above: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297923.html Please confirm that it actually fixes the testcase its commit message claims to fix. 2. The RAP_B_Bossen_1 and NoOutPrior_A_Qualcomm_1 (where the testcases change due to your patch) contain completely fine end of sequence NALUs. Because they are valid, stripping them (as your patch does) is not ok (e.g. these units would even be discarded when using hevc_metadata). There are two bugs with these units: a) Our parser puts them at the beginning of their NALUs, yet they should be at the end of the (preceding) NALU. b) When remuxing the samples to Matroska with mkvmerge (which puts these units at the end of their packets), the output is the same as with raw input, i.e. decoder still misses some frames. So somehow these units confuse the decoder. - Andreas
On Wed, Jun 22, 2022 at 01:05:22PM +0200, Andreas Rheinhardt wrote: > Andreas Rheinhardt: > > Michael Niedermayer: > >> Fixes: Assertion failure > >> Fixes: 46662/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-4947860854013952 > >> > >> This also results in more frames to be decoded from fate samples > >> > >> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >> --- > >> libavcodec/h2645_parse.c | 2 +- > >> .../ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 | 10 ++++++++++ > >> tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 | 3 +++ > >> 3 files changed, 14 insertions(+), 1 deletion(-) > >> > >> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c > >> index 03780680c6..78ab22b76e 100644 > >> --- a/libavcodec/h2645_parse.c > >> +++ b/libavcodec/h2645_parse.c > >> @@ -292,7 +292,7 @@ static int hevc_parse_nal_header(H2645NAL *nal, void *logctx) > >> { > >> GetBitContext *gb = &nal->gb; > >> > >> - if (get_bits1(gb) != 0) > >> + if (get_bits_left(gb) < 16 || get_bits1(gb) != 0) > >> return AVERROR_INVALIDDATA; > >> > >> nal->type = get_bits(gb, 6); > >> diff --git a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > >> index 0c930f6556..3283925e38 100644 > >> --- a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > >> +++ b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 > >> @@ -25,6 +25,16 @@ > >> 0, 19, 19, 1, 599040, 0x4227009b > >> 0, 20, 20, 1, 599040, 0x1bda8be4 > >> 0, 21, 21, 1, 599040, 0xd1d5dcb4 > >> +0, 22, 22, 1, 599040, 0x58b2edb3 > >> +0, 23, 23, 1, 599040, 0xd1f795d8 > >> +0, 24, 24, 1, 599040, 0x3331d5e6 > >> +0, 25, 25, 1, 599040, 0x5e5ec2c9 > >> +0, 26, 26, 1, 599040, 0x3b907bf5 > >> +0, 27, 27, 1, 599040, 0xefcbf471 > >> +0, 28, 28, 1, 599040, 0x2769a578 > >> +0, 29, 29, 1, 599040, 0x812ce986 > >> +0, 30, 30, 1, 599040, 0xf07c212c > >> +0, 31, 31, 1, 599040, 0xb5476890 > >> 0, 32, 32, 1, 599040, 0x00a0249f > >> 0, 33, 33, 1, 599040, 0x7263f7cf > >> 0, 34, 34, 1, 599040, 0x47054be4 > >> diff --git a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > >> index e661ff245e..776267b59c 100644 > >> --- a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > >> +++ b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 > >> @@ -70,6 +70,9 @@ > >> 0, 64, 64, 1, 149760, 0x3362678b > >> 0, 65, 65, 1, 149760, 0x6e7fc851 > >> 0, 66, 66, 1, 149760, 0x33f96449 > >> +0, 67, 67, 1, 149760, 0xd9d05007 > >> +0, 75, 75, 1, 149760, 0x477f2cf2 > >> +0, 76, 76, 1, 149760, 0xe1f9ccd0 > >> 0, 77, 77, 1, 149760, 0xb3ba8cfb > >> 0, 78, 78, 1, 149760, 0x64787995 > >> 0, 79, 79, 1, 149760, 0xc10de4c4 > > > > get_bit_length currently presumes every NALU to contain > > rbsp_trailing_bits. Yet this is not true for the End of > > Sequence/Bitstream units which are just headers without RBSP. For these > > units, get_bit_length might truncate them -- it does so for end of > > sequence units in H.264. It would not be a serious issue for H.265, as > > the semantics of nuh_temporal_id_plus1 require nuh_temporal_id_plus1 to > > be 1 for End of Sequence/Bitstream units. Nevertheless I think this > > should be coupled with a patch that does not truncate the NAL unit if it > > is just a header. > > > > 1. I just sent a patch implementing the above: > https://ffmpeg.org/pipermail/ffmpeg-devel/2022-June/297923.html > Please confirm that it actually fixes the testcase its commit message > claims to fix. confirmed! thx > 2. The RAP_B_Bossen_1 and NoOutPrior_A_Qualcomm_1 (where the testcases > change due to your patch) contain completely fine end of sequence NALUs. > Because they are valid, stripping them (as your patch does) is not ok > (e.g. these units would even be discarded when using hevc_metadata). > There are two bugs with these units: > a) Our parser puts them at the beginning of their NALUs, yet they should > be at the end of the (preceding) NALU. > b) When remuxing the samples to Matroska with mkvmerge (which puts these > units at the end of their packets), the output is the same as with raw > input, i.e. decoder still misses some frames. So somehow these units > confuse the decoder. [...]
diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c index 03780680c6..78ab22b76e 100644 --- a/libavcodec/h2645_parse.c +++ b/libavcodec/h2645_parse.c @@ -292,7 +292,7 @@ static int hevc_parse_nal_header(H2645NAL *nal, void *logctx) { GetBitContext *gb = &nal->gb; - if (get_bits1(gb) != 0) + if (get_bits_left(gb) < 16 || get_bits1(gb) != 0) return AVERROR_INVALIDDATA; nal->type = get_bits(gb, 6); diff --git a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 index 0c930f6556..3283925e38 100644 --- a/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 +++ b/tests/ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 @@ -25,6 +25,16 @@ 0, 19, 19, 1, 599040, 0x4227009b 0, 20, 20, 1, 599040, 0x1bda8be4 0, 21, 21, 1, 599040, 0xd1d5dcb4 +0, 22, 22, 1, 599040, 0x58b2edb3 +0, 23, 23, 1, 599040, 0xd1f795d8 +0, 24, 24, 1, 599040, 0x3331d5e6 +0, 25, 25, 1, 599040, 0x5e5ec2c9 +0, 26, 26, 1, 599040, 0x3b907bf5 +0, 27, 27, 1, 599040, 0xefcbf471 +0, 28, 28, 1, 599040, 0x2769a578 +0, 29, 29, 1, 599040, 0x812ce986 +0, 30, 30, 1, 599040, 0xf07c212c +0, 31, 31, 1, 599040, 0xb5476890 0, 32, 32, 1, 599040, 0x00a0249f 0, 33, 33, 1, 599040, 0x7263f7cf 0, 34, 34, 1, 599040, 0x47054be4 diff --git a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 index e661ff245e..776267b59c 100644 --- a/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 +++ b/tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 @@ -70,6 +70,9 @@ 0, 64, 64, 1, 149760, 0x3362678b 0, 65, 65, 1, 149760, 0x6e7fc851 0, 66, 66, 1, 149760, 0x33f96449 +0, 67, 67, 1, 149760, 0xd9d05007 +0, 75, 75, 1, 149760, 0x477f2cf2 +0, 76, 76, 1, 149760, 0xe1f9ccd0 0, 77, 77, 1, 149760, 0xb3ba8cfb 0, 78, 78, 1, 149760, 0x64787995 0, 79, 79, 1, 149760, 0xc10de4c4
Fixes: Assertion failure Fixes: 46662/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HEVC_fuzzer-4947860854013952 This also results in more frames to be decoded from fate samples Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/h2645_parse.c | 2 +- .../ref/fate/hevc-conformance-NoOutPrior_A_Qualcomm_1 | 10 ++++++++++ tests/ref/fate/hevc-conformance-RAP_B_Bossen_1 | 3 +++ 3 files changed, 14 insertions(+), 1 deletion(-)