On 10/5/2020 6:43 PM, Michael Niedermayer wrote: > On Mon, Oct 05, 2020 at 01:30:07AM -0300, James Almer wrote: >> On 10/4/2020 6:02 PM, James Almer wrote: >>> On 10/4/2020 5:57 PM, Michael Niedermayer wrote: >>>> On Sun, Oct 04, 2020 at 05:04:05PM -0300, James Almer wrote: >>>>> On 10/4/2020 4:41 PM, Michael Niedermayer wrote: >>>>>> Fixes: OOM >>>>>> Fixes: >>>>>> 23817/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-6300869057576960 >>>>>> >>>>>> Found-by: continuous fuzzing process >>>>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>>>>> Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc> >>>>>> --- >>>>>> libavcodec/h2645_parse.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c >>>>>> index 0f98b49fbe..61105a6eb5 100644 >>>>>> --- a/libavcodec/h2645_parse.c >>>>>> +++ b/libavcodec/h2645_parse.c >>>>>> @@ -467,7 +467,7 @@ int ff_h2645_packet_split(H2645Packet *pkt, const >>>>>> uint8_t *buf, int length, >>>>>> memset(pkt->nals + pkt->nals_allocated, 0, >>>>>> sizeof(*pkt->nals)); >>>>>> >>>>>> nal = &pkt->nals[pkt->nb_nals]; >>>>>> - nal->skipped_bytes_pos_size = 1024; // initial buffer size >>>>>> + nal->skipped_bytes_pos_size = FFMIN(1024, >>>>>> 1+(extract_length>>4)); // initial buffer size >>>>> >>>>> Why is there even an initial buffer? Why not just let >>>>> ff_h2645_extract_rbsp() allocate it when needed? >>>> >>>> i wondered that too and assumed it was done that way to avoid spending >>>> cpu cycles on reallocations later >>> >>> Many streams don't need to escape bytes, so for those, allocating >>> anything at all is a waste. And IMO by using av_fast_realloc() in >>> ff_h2645_extract_rbsp() there's no need for a big enough initial buffer >>> either. >> >> On second thought, even if most av_fast_realloc() calls will be no-ops, >> they may end up being way too many to the point the current behavior >> would be more efficient. >> >> Does moving the allocation of the initial buffer to >> ff_h2645_extract_rbsp() also help with this sample? I assume it's one >> where it generates an absurd amount of NAL units in the packet, but most >> would probably be small enough that they will not really contain bytes >> that need to be escaped, and as such not require a skipped_bytes_pos buffer. > > your variant below works too, yes
What do you think about setting instead nal->skipped_bytes_pos_size to length / 3 when it needs to be resized? The max possible amount of bytes it will skip/strip per NAL is one third of the total NAL size. It's probably better than my previous suggestion, as it still removes the initial buffer outside of ff_h2645_extract_rbsp() and also avoids the unconditional check for nal->skipped_bytes_pos_size == 0 and does not duplicate the buffer's size on each realloc. > the fuzzer testcase is a gazzilion of 1byte NAL units IIRC > > thx > > [...] > > > _______________________________________________ > 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". > _______________________________________________ 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".