[FFmpeg-devel] [PATCH 2/3] avcodec/h264_picture: add ff_h264_replace_picture()

[Andreas Rheinhardt]

James Almer:
> Will remove unnecessary allocations when both src and dst picture contain
> references to the same buffers.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/h264_picture.c | 45 +++++++++++++++++++++++++++++++++++++++
>  libavcodec/h264dec.h      |  1 +
>  2 files changed, 46 insertions(+)
> 
> diff --git a/libavcodec/h264_picture.c b/libavcodec/h264_picture.c
> index 89aef37edd..1073d9e7e0 100644
> --- a/libavcodec/h264_picture.c
> +++ b/libavcodec/h264_picture.c
> @@ -142,6 +142,51 @@ fail:
>      return ret;
>  }
>  
> +int ff_h264_replace_picture(H264Context *h, H264Picture *dst, H264Picture *src)

Is there something that hinders you from making src const? If so, I
don't see it.


> +{
> +    int ret, i;
> +
> +    if (!src->f || !src->f->buf[0]) {
> +        ff_h264_unref_picture(h, dst);
> +        return 0;
> +    }
> +
> +    av_assert0(src->tf.f == src->f);
> +
> +    dst->tf.f = dst->f;
> +    ff_thread_release_buffer(h->avctx, &dst->tf);
> +    ret = ff_thread_ref_frame(&dst->tf, &src->tf);
> +    if (ret < 0)
> +        goto fail;
> +
> +    ret  = av_buffer_replace(&dst->qscale_table_buf, src->qscale_table_buf);
> +    ret |= av_buffer_replace(&dst->mb_type_buf, src->mb_type_buf);
> +    ret |= av_buffer_replace(&dst->pps_buf, src->pps_buf);
> +    if (ret < 0)
> +        goto fail;
> +
> +    for (i = 0; i < 2; i++) {
> +        ret  = av_buffer_replace(&dst->motion_val_buf[i], src->motion_val_buf[i]);
> +        ret |= av_buffer_replace(&dst->ref_index_buf[i], src->ref_index_buf[i]);
> +        if (ret < 0)
> +            goto fail;
> +    }
> +
> +    if (src->hwaccel_picture_private) {

dst in this function is allowed to be used/dirty; so I don't see
anything that precludes dst->hwaccel_picture_private/hwaccel_priv_buf to
be set while the same is not true for src. But then this check means
that dst is not equivalent to src.

> +        ret = av_buffer_replace(&dst->hwaccel_priv_buf, src->hwaccel_priv_buf);
> +        if (ret < 0)
> +            goto fail;
> +        dst->hwaccel_picture_private = dst->hwaccel_priv_buf->data;

This is the only thing that needs to be under if.

- Andreas