[FFmpeg-devel] [PATCH 23/50] avformat/mux: use av_packet_alloc() to allocate packets

[Andreas Rheinhardt]

James Almer:
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavformat/internal.h |  5 +++++
>  libavformat/mux.c      | 44 ++++++++++++++++++++++--------------------
>  libavformat/options.c  |  6 ++++++
>  libavformat/utils.c    |  1 +
>  4 files changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/libavformat/internal.h b/libavformat/internal.h
> index d0db331b96..69a7caff93 100644
> --- a/libavformat/internal.h
> +++ b/libavformat/internal.h
> @@ -92,6 +92,11 @@ struct AVFormatInternal {
>       */
>      struct AVPacketList *parse_queue;
>      struct AVPacketList *parse_queue_end;
> +
> +    /**
> +     * Used to hold temporary packets.
> +     */
> +    AVPacket *pkt;
>      /**
>       * Remaining size available for raw_packet_buffer, in bytes.
>       */
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 84c56ac6ba..3600e74a50 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -1052,7 +1052,9 @@ int ff_interleaved_peek(AVFormatContext *s, int stream,
>      AVPacketList *pktl = s->internal->packet_buffer;
>      while (pktl) {
>          if (pktl->pkt.stream_index == stream) {
> -            *pkt = pktl->pkt;
> +            int ret = av_packet_ref(pkt, &pktl->pkt);
> +            if (ret < 0)
> +                return ret;

This will lead to memleaks, because up until now the callers just
received a non-ownership packets, ergo they did not unref the packet.
(The fate-movenc test fails with this when run under valgrind/asan.)

Returning a pointer to a const AVPacket and signaling the offsetted
timestamps via other pointer arguments would avoid this problem.

>              if (add_offset) {
>                  AVStream *st = s->streams[pkt->stream_index];
>                  int64_t offset = st->internal->mux_ts_offset;
> @@ -1208,7 +1210,7 @@ static int write_packets_common(AVFormatContext *s, AVPacket *pkt, int interleav
>  
>  int av_write_frame(AVFormatContext *s, AVPacket *in)
>  {
> -    AVPacket local_pkt, *pkt = &local_pkt;
> +    AVPacket *pkt = s->internal->pkt;
>      int ret;
>  
>      if (!in) {
> @@ -1229,6 +1231,7 @@ int av_write_frame(AVFormatContext *s, AVPacket *in)
>           * The following avoids copying in's data unnecessarily.
>           * Copying side data is unavoidable as a bitstream filter
>           * may change it, e.g. free it on errors. */
> +        av_packet_unref(pkt);
>          pkt->buf  = NULL;
>          pkt->data = in->data;
>          pkt->size = in->size;
> @@ -1270,14 +1273,14 @@ int av_interleaved_write_frame(AVFormatContext *s, AVPacket *pkt)
>  int av_write_trailer(AVFormatContext *s)
>  {
>      int i, ret1, ret = 0;
> -    AVPacket pkt = {0};
> -    av_init_packet(&pkt);
> +    AVPacket *pkt = s->internal->pkt;
>  
> +    av_packet_unref(pkt);
>      for (i = 0; i < s->nb_streams; i++) {
>          if (s->streams[i]->internal->bsfc) {
> -            ret1 = write_packets_from_bsfs(s, s->streams[i], &pkt, 1/*interleaved*/);
> +            ret1 = write_packets_from_bsfs(s, s->streams[i], pkt, 1/*interleaved*/);
>              if (ret1 < 0)
> -                av_packet_unref(&pkt);
> +                av_packet_unref(pkt);
>              if (ret >= 0)
>                  ret = ret1;
>          }
> @@ -1351,7 +1354,7 @@ static void uncoded_frame_free(void *unused, uint8_t *data)
>  static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index,
>                                          AVFrame *frame, int interleaved)
>  {
> -    AVPacket pkt, *pktp;
> +    AVPacket *pkt = s->internal->pkt;
>  
>      av_assert0(s->oformat);
>      if (!s->oformat->write_uncoded_frame) {
> @@ -1360,18 +1363,17 @@ static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index,
>      }
>  
>      if (!frame) {
> -        pktp = NULL;
> +        pkt = NULL;
>      } else {
>          size_t   bufsize = sizeof(frame) + AV_INPUT_BUFFER_PADDING_SIZE;
>          AVFrame **framep = av_mallocz(bufsize);
>  
>          if (!framep)
>              goto fail;
> -        pktp = &pkt;
> -        av_init_packet(&pkt);
> -        pkt.buf = av_buffer_create((void *)framep, bufsize,
> +        av_packet_unref(pkt);
> +        pkt->buf = av_buffer_create((void *)framep, bufsize,
>                                     uncoded_frame_free, NULL, 0);
> -        if (!pkt.buf) {
> +        if (!pkt->buf) {
>              av_free(framep);
>      fail:
>              av_frame_free(&frame);
> @@ -1379,17 +1381,17 @@ static int write_uncoded_frame_internal(AVFormatContext *s, int stream_index,
>          }
>          *framep = frame;
>  
> -        pkt.data         = (void *)framep;
> -        pkt.size         = sizeof(frame);
> -        pkt.pts          =
> -        pkt.dts          = frame->pts;
> -        pkt.duration     = frame->pkt_duration;
> -        pkt.stream_index = stream_index;
> -        pkt.flags |= AV_PKT_FLAG_UNCODED_FRAME;
> +        pkt->data         = (void *)framep;
> +        pkt->size         = sizeof(frame);
> +        pkt->pts          =
> +        pkt->dts          = frame->pts;
> +        pkt->duration     = frame->pkt_duration;
> +        pkt->stream_index = stream_index;
> +        pkt->flags |= AV_PKT_FLAG_UNCODED_FRAME;
>      }
>  
> -    return interleaved ? av_interleaved_write_frame(s, pktp) :
> -                         av_write_frame(s, pktp);
> +    return interleaved ? av_interleaved_write_frame(s, pkt) :
> +                         av_write_frame(s, pkt);
>  }
>  
>  int av_write_uncoded_frame(AVFormatContext *s, int stream_index,
> diff --git a/libavformat/options.c b/libavformat/options.c
> index 59e0389815..8d7c4fe4cb 100644
> --- a/libavformat/options.c
> +++ b/libavformat/options.c
> @@ -220,6 +220,12 @@ AVFormatContext *avformat_alloc_context(void)
>          av_free(ic);
>          return NULL;
>      }
> +    internal->pkt = av_packet_alloc();
> +    if (!internal->pkt) {
> +        av_free(internal);
> +        av_free(ic);
> +        return NULL;
> +    }
>      avformat_get_context_defaults(ic);
>      ic->internal = internal;
>      ic->internal->offset = AV_NOPTS_VALUE;
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index fb3299503e..2587bedc05 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -4445,6 +4445,7 @@ void avformat_free_context(AVFormatContext *s)
>      av_freep(&s->chapters);
>      av_dict_free(&s->metadata);
>      av_dict_free(&s->internal->id3v2_meta);
> +    av_packet_free(&s->internal->pkt);
>      av_freep(&s->streams);
>      flush_packet_queue(s);
>      av_freep(&s->internal);
>