[FFmpeg-devel] [PATCH] lavd/lavfi: upgrade to AVFrame.

[Stefano Sabatini]

On date Tuesday 2013-03-12 14:39:43 +0100, Nicolas George encoded:
> Fix a memory leak because the compat implementation of
> AV_BUFFERSINK_FLAG_PEEK is flawed.
> 
> Signed-off-by: Nicolas George <nicolas.george at normalesup.org>
> ---
>  libavdevice/lavfi.c |   47 ++++++++++++++++++++++++++---------------------
>  1 file changed, 26 insertions(+), 21 deletions(-)
> 
> 

> Note: frame->type is not the media type!
> There is no media type in the frame :(

Should we add it? (seems a good idea to me)

> 
> 
> diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c
> index 3b6f0c3..e82b03a 100644
> --- a/libavdevice/lavfi.c
> +++ b/libavdevice/lavfi.c
> @@ -51,6 +51,7 @@ typedef struct {
>      int *sink_stream_map;
>      int *sink_eof;
>      int *stream_sink_map;
> +    AVFrame *decoded_frame;
>  } LavfiContext;
>  
>  static int *create_all_formats(int n)
> @@ -83,6 +84,7 @@ av_cold static int lavfi_read_close(AVFormatContext *avctx)
>      av_freep(&lavfi->stream_sink_map);
>      av_freep(&lavfi->sinks);
>      avfilter_graph_free(&lavfi->graph);
> +    av_frame_free(&lavfi->decoded_frame);
>  
>      return 0;
>  }
> @@ -300,6 +302,9 @@ av_cold static int lavfi_read_header(AVFormatContext *avctx)
>          }
>      }
>  
> +    if (!(lavfi->decoded_frame = av_frame_alloc()))
> +        FAIL(AVERROR(ENOMEM));
> +
>  end:
>      av_free(pix_fmts);
>      avfilter_inout_free(&input_links);
> @@ -314,7 +319,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>      LavfiContext *lavfi = avctx->priv_data;
>      double min_pts = DBL_MAX;
>      int stream_idx, min_pts_sink_idx = 0;
> -    AVFilterBufferRef *ref;
> +    AVFrame *frame = lavfi->decoded_frame;
>      AVPicture pict;
>      int ret, i;
>      int size = 0;
> @@ -329,16 +334,17 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>          if (lavfi->sink_eof[i])
>              continue;
>  
> -        ret = av_buffersink_get_buffer_ref(lavfi->sinks[i],
> -                                       &ref, AV_BUFFERSINK_FLAG_PEEK);
> +        ret = av_buffersink_get_frame_flags(lavfi->sinks[i], frame,
> +                                            AV_BUFFERSINK_FLAG_PEEK);
>          if (ret == AVERROR_EOF) {
>              av_dlog(avctx, "EOF sink_idx:%d\n", i);
>              lavfi->sink_eof[i] = 1;
>              continue;
>          } else if (ret < 0)
>              return ret;
> -        d = av_rescale_q(ref->pts, tb, AV_TIME_BASE_Q);
> +        d = av_rescale_q(frame->pts, tb, AV_TIME_BASE_Q);
>          av_dlog(avctx, "sink_idx:%d time:%f\n", i, d);

> +        av_frame_unref(frame);

Looks good assuming it is correct, in this case we should also update
the buffersink.h docs.

>          if (d < min_pts) {
>              min_pts = d;
> @@ -350,35 +356,34 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>  
>      av_dlog(avctx, "min_pts_sink_idx:%i\n", min_pts_sink_idx);
>  
> -    av_buffersink_get_buffer_ref(lavfi->sinks[min_pts_sink_idx], &ref, 0);
> +    av_buffersink_get_frame_flags(lavfi->sinks[min_pts_sink_idx], frame, 0);
>      stream_idx = lavfi->sink_stream_map[min_pts_sink_idx];
>  
> -    if (ref->video) {
> -        size = avpicture_get_size(ref->format, ref->video->w, ref->video->h);
> +    if (frame->width /* FIXME best way of testing a video */) {
> +        size = avpicture_get_size(frame->format, frame->width, frame->height);
>          if ((ret = av_new_packet(pkt, size)) < 0)
>              return ret;
>  
> -        memcpy(pict.data,     ref->data,     4*sizeof(ref->data[0]));
> -        memcpy(pict.linesize, ref->linesize, 4*sizeof(ref->linesize[0]));
> +        memcpy(pict.data,     frame->data,     4*sizeof(frame->data[0]));
> +        memcpy(pict.linesize, frame->linesize, 4*sizeof(frame->linesize[0]));
>  
> -        avpicture_layout(&pict, ref->format, ref->video->w,
> -                         ref->video->h, pkt->data, size);
> -    } else if (ref->audio) {
> -        size = ref->audio->nb_samples *
> -            av_get_bytes_per_sample(ref->format) *
> -            av_get_channel_layout_nb_channels(ref->audio->channel_layout);
> +        avpicture_layout(&pict, frame->format, frame->width, frame->height,
> +                         pkt->data, size);
> +    } else if (av_frame_get_channels(frame) /* FIXME test audio */) {
> +        size = frame->nb_samples * av_get_bytes_per_sample(frame->format) *
> +                                   av_frame_get_channels(frame);
>          if ((ret = av_new_packet(pkt, size)) < 0)
>              return ret;
> -        memcpy(pkt->data, ref->data[0], size);
> +        memcpy(pkt->data, frame->data[0], size);
>      }
>  
> -    if (ref->metadata) {
> +    if (frame->metadata) {
>          uint8_t *metadata;
>          AVDictionaryEntry *e = NULL;
>          AVBPrint meta_buf;
>  
>          av_bprint_init(&meta_buf, 0, AV_BPRINT_SIZE_UNLIMITED);
> -        while ((e = av_dict_get(ref->metadata, "", e, AV_DICT_IGNORE_SUFFIX))) {
> +        while ((e = av_dict_get(frame->metadata, "", e, AV_DICT_IGNORE_SUFFIX))) {
>              av_bprintf(&meta_buf, "%s", e->key);
>              av_bprint_chars(&meta_buf, '\0', 1);
>              av_bprintf(&meta_buf, "%s", e->value);
> @@ -395,10 +400,10 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
>      }
>  
>      pkt->stream_index = stream_idx;
> -    pkt->pts = ref->pts;
> -    pkt->pos = ref->pos;
> +    pkt->pts = frame->pts;
> +    pkt->pos = av_frame_get_pkt_pos(frame);
>      pkt->size = size;
> -    avfilter_unref_buffer(ref);
> +    av_frame_unref(frame);
>      return size;
>  }

LGTM otherwise and thanks.
-- 
FFmpeg = Furious & Frenzy Mortal Problematic Evil Gymnast