[FFmpeg-devel] [PATCH 2/2] avcodec/options: copy the coded_side_data in avcodec_copy_context()

[Aaron Levinson]

On 4/22/2017 10:26 AM, James Almer wrote:
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/options.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/libavcodec/options.c b/libavcodec/options.c
> index b98da9378a..c721aa8d43 100644
> --- a/libavcodec/options.c
> +++ b/libavcodec/options.c
> @@ -190,15 +190,21 @@ void avcodec_free_context(AVCodecContext **pavctx)
>  #if FF_API_COPY_CONTEXT
>  static void copy_context_reset(AVCodecContext *avctx)
>  {
> +    int i;
> +
>      av_opt_free(avctx);
>      av_freep(&avctx->rc_override);
>      av_freep(&avctx->intra_matrix);
>      av_freep(&avctx->inter_matrix);
>      av_freep(&avctx->extradata);
>      av_freep(&avctx->subtitle_header);
> +    for (i = 0; i < avctx->nb_coded_side_data; i++)
> +        av_freep(&avctx->coded_side_data[i].data);
> +    av_freep(&avctx->coded_side_data);
>      av_buffer_unref(&avctx->hw_frames_ctx);
>      avctx->subtitle_header_size = 0;
>      avctx->extradata_size = 0;
> +    avctx->nb_coded_side_data = 0;
>  }
>
>  int avcodec_copy_context(AVCodecContext *dest, const AVCodecContext *src)
> @@ -262,6 +268,28 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      alloc_and_copy_or_fail(subtitle_header, src->subtitle_header_size, 1);
>      av_assert0(dest->subtitle_header_size == src->subtitle_header_size);
>  #undef alloc_and_copy_or_fail
> +    if (src->nb_coded_side_data) {
> +        int i;
> +
> +        dest->nb_coded_side_data = 0;
> +        dest->coded_side_data = av_realloc_array(NULL, src->nb_coded_side_data,
> +                                                 sizeof(*dest->coded_side_data));
> +        if (!dest->coded_side_data)
> +            goto fail;
> +
> +        for (i = 0; i < src->nb_coded_side_data; i++) {
> +            const AVPacketSideData *sd_src = &src->coded_side_data[i];
> +            AVPacketSideData *sd_dst = &dest->coded_side_data[i];
> +
> +            sd_dst->data = av_malloc(sd_src->size);
> +            if (!sd_dst->data)
> +                goto fail;
> +            memcpy(sd_dst->data, sd_src->data, sd_src->size);
> +            sd_dst->size = sd_src->size;
> +            sd_dst->type = sd_src->type;
> +            dest->nb_coded_side_data++;
> +        }
> +    }
>
>      if (src->hw_frames_ctx) {
>          dest->hw_frames_ctx = av_buffer_ref(src->hw_frames_ctx);
>

If src has coded_side_data and nb_coded_side_data set to non-zero 
values, as a result of the earlier call to memcpy(), it will copy those 
values into dest.  Then, if it does a goto to fail early, as a result of 
your changes to copy_context_reset(), it will call av_freep() on those 
elements in dest, even though they are owned by src.  After doing the 
memcpy() call, I think you can fix this by setting dest->coded_side_data 
to NULL and dest->nb_coded_side_data to 0.

Everything else looks good.

Aaron Levinson