[FFmpeg-devel] [PATCH v9 1/5] avformat/subtitles: extend ff_subtitles_queue_insert() to support not yet available events

[Stefano Sabatini]

On date Saturday 2024-03-23 21:25:00 -0500, Marth64 wrote:
> If ff_subtitles_queue_insert() were 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, allow passing a NULL buffer with 0 length
> without attempting to do such operations. 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 <marth64 at proxyid.net>
> ---
>  libavformat/subtitles.c | 24 ++++++++++++++++--------
>  libavformat/subtitles.h |  2 +-
>  2 files changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
> index 3413763c7b..4742ca0667 100644
> --- a/libavformat/subtitles.c
> +++ b/libavformat/subtitles.c
> @@ -21,6 +21,7 @@
>  #include "avformat.h"
>  #include "subtitles.h"
>  #include "avio_internal.h"
> +#include "libavutil/avassert.h"
>  #include "libavutil/avstring.h"
>  
>  void ff_text_init_avio(void *s, FFTextReader *r, AVIOContext *pb)
> @@ -111,15 +112,20 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
>  {
>      AVPacket **subs, *sub;
>  
> +    if (!event)
> +        av_assert1(len == 0);
> +
>      if (merge && q->nb_subs > 0) {
>          /* merge with previous event */
>  
>          int old_len;
>          sub = q->subs[q->nb_subs - 1];
>          old_len = sub->size;
> -        if (av_grow_packet(sub, len) < 0)
> -            return NULL;
> -        memcpy(sub->data + old_len, event, len);
> +        if (event) {
> +            if (av_grow_packet(sub, len) < 0)
> +                return NULL;
> +            memcpy(sub->data + old_len, event, len);
> +        }
>      } else {
>          /* new event */
>  
> @@ -133,14 +139,16 @@ AVPacket *ff_subtitles_queue_insert(FFDemuxSubtitlesQueue *q,
>          sub = av_packet_alloc();
>          if (!sub)
>              return NULL;
> -        if (av_new_packet(sub, len) < 0) {
> -            av_packet_free(&sub);
> -            return NULL;
> +        if (event) {
> +            if (av_new_packet(sub, len) < 0) {
> +                av_packet_free(&sub);
> +                return NULL;
> +            }
> +            memcpy(sub->data, event, len);
>          }

> -        subs[q->nb_subs++] = sub;
>          sub->flags |= AV_PKT_FLAG_KEY;
>          sub->pts = sub->dts = 0;
> -        memcpy(sub->data, event, len);

> +        subs[q->nb_subs++] = sub;

is the move needed? (but not blocking)

LGTM otherwise.