On Wed, Dec 11, 2019 at 10:50:54PM +0100, Andreas Rheinhardt wrote: > On Wed, Dec 11, 2019 at 10:38 PM James Almer <jamr...@gmail.com> wrote: > > > On 12/11/2019 6:03 PM, Michael Niedermayer wrote: > > > Its unclear if shortening these NAL units would have no effect on them > > > > > > Fixes: assertion failure > > > Fixes: > > 19286/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-5707990724509696 > > > > > > Found-by: continuous fuzzing process > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> > > > --- > > > libavcodec/cbs_h2645.c | 9 +++++++-- > > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > > > index 5f71d80584..b38b1d99ef 100644 > > > --- a/libavcodec/cbs_h2645.c > > > +++ b/libavcodec/cbs_h2645.c > > > @@ -564,11 +564,16 @@ static int > > cbs_h2645_fragment_add_nals(CodedBitstreamContext *ctx, > > > const H2645NAL *nal = &packet->nals[i]; > > > AVBufferRef *ref; > > > size_t size = nal->size; > > > + int shorten = 1; > > > + > > > + // Do not shorten reserved and unspecified NALs > > > + if (ctx->codec->codec_id == AV_CODEC_ID_H264) { > > > + shorten = nal->type > 0 && nal->type < 23; > > > + } > > > > > > // Remove trailing zeroes. > > > - while (size > 0 && nal->data[size - 1] == 0) > > > + while (shorten && size > 0 && nal->data[size - 1] == 0) > > > --size; > > > - av_assert0(size > 0); > > > > So this is a NAL unit with a bunch of zero bytes? How did it go through
yes > > the filter in h2645_parse? It's supposed to skip any NAL like this. > > > > I guess it triggered this workaround: > /* see commit 3566042a0 */ > if (bytestream2_get_bytes_left(&bc) >= 4 && > bytestream2_peek_be32(&bc) == 0x000001E0) > skip_trailing_zeros = 0; yes > > If I am not mistaken, then all NAL units except one consisting solely of a > single 0x00 (where the RBSP is empty) have to have a rbsp_stop_one_bit. So > the only instance where stripping zeroes is not good is for such a 0x00 > unit. And such a unit should be stripped, because it actually can't be > represented in annex b (which we output) at all. I dont see why it could not be represented. Also the code you quoted above shows that there are NAL units where the removal of zeros would damage the units. I wonder if we should assume that NALs not described in the current H26* specs follow exactly the RBSP syntax and can saftely be truncated. My patch went in the direction of rather leaving that alone. But of course we can do something else or i can try to implement that more completely also preserving that on the input side but having no real world testcase that would not be tested. What do you prefer? you seem to have a strong oppinion here Thanks -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Freedom in capitalist society always remains about the same as it was in ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
