Marth64: > If ff_subtitles_queue_insert() were to given a NULL buffer > with 0 length, it would still attempt to grow the packet > or memcpy depending on if merge option is enabled. > > In this commit, consider a NULL buffer with 0 length as > an empty event and do not attempt to modify the packet. > This way, if a subtitle demuxer happens to pass an empty > cue or wants to use av_get_packet() to read bytes, there > are no unnecessary operations on the packet after it is > allocated. > > Signed-off-by: Marth64 <mart...@proxyid.net> > --- > libavformat/subtitles.c | 4 ++++ > libavformat/subtitles.h | 2 +- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c > index 3413763c7b..38d2ffb8a9 100644 > --- a/libavformat/subtitles.c > +++ b/libavformat/subtitles.c > @@ -117,6 +117,8 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue > *q, > int old_len; > sub = q->subs[q->nb_subs - 1]; > old_len = sub->size; > + if (event == NULL && len == 0) > + return sub;
Checks for NULL are typically written as !event. I'd prefer if (!event) { av_assert1(len == 0); return sub; } as this also makes clear that !event with len > 0 must not happen and also avoids one runtime check for ordinary users (with assert-level 0). > if (av_grow_packet(sub, len) < 0) > return NULL; > memcpy(sub->data + old_len, event, len); > @@ -140,6 +142,8 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue > *q, > subs[q->nb_subs++] = sub; > sub->flags |= AV_PKT_FLAG_KEY; > sub->pts = sub->dts = 0; > + if (event == NULL && len == 0) > + return sub; > memcpy(sub->data, event, len); This will allocate sub->data to a buffer of size AV_INPUT_BUFFER_PADDING_SIZE and usable size of zero. This is not what is intended: You should not allocate anything at all, i.e. skip av_new_packet(). This small buffer (together with the AVBuffer and AVBufferRef allocated for it) will leak with your current patch 2. > } > return sub; > diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h > index 88665663c5..ba162fa503 100644 > --- a/libavformat/subtitles.h > +++ b/libavformat/subtitles.h > @@ -112,7 +112,7 @@ typedef struct { > /** > * Insert a new subtitle event. > * > - * @param event the subtitle line, may not be zero terminated > + * @param event the subtitle line (not zero terminated), or NULL on empty > event on not yet available event > * @param len the length of the event (in strlen() sense, so without '\0') > * @param merge set to 1 if the current event should be concatenated with the > * previous one instead of adding a new entry, 0 otherwise _______________________________________________ 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".