Re: [FFmpeg-devel] [PATCH 03/11] avutil/frame: add functions to check and ensure a side data entry is writable

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

James Almer:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavutil/frame.h     | 19 +++++++++++++++++++
>  libavutil/side_data.c | 25 +++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 49260ae2dd..a707b35087 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -1211,6 +1211,25 @@ void av_frame_side_data_remove(AVFrameSideData ***sd, int *nb_sd,
>  void av_frame_side_data_remove_by_props(AVFrameSideData ***sd, int *nb_sd,
>                                          int props);
>  
> +/**
> + * Check whether the data described by a given AVFrameSideData can be
> + * written to.
> + *
> + * @return 1 if the caller may write to the data, 0 otherwise.
> + */
> +int av_frame_side_data_is_writable(const AVFrameSideData *sd);
> +
> +/**
> + * Create a writable reference for the data described by a given
> + * AVFrameSideData, avoiding data copy if possible.
> + *
> + * @param sd Side data whose data should be made writable.
> + *
> + * @return 0 on success, a negative AVERROR on failure. On failure, the
> + *         side data is unchanged.
> + */
> +int av_frame_side_data_make_writable(AVFrameSideData *sd);

I don't see the point of either of them: The former is just a wrapper
around av_buffer_is_writable() and the latter is basically the same as
av_buffer_make_writable() (and could actually use it).

If this is designed with using something else but flat buffers in the
future in mind, then av_frame_side_data_make_writable() is misdesigned,
because making a non-flat object writable is ambiguous: It is not clear
whether making something writable is meant in a shallow or a deep sense
which may become an issue if future side-data contains references of its
own.

> +
>  /**
>   * @}
>   */
> diff --git a/libavutil/side_data.c b/libavutil/side_data.c
> index 8c57fd838a..beb8ea3212 100644
> --- a/libavutil/side_data.c
> +++ b/libavutil/side_data.c
> @@ -315,3 +315,28 @@ const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *s
>      }
>      return NULL;
>  }
> +
> +int av_frame_side_data_is_writable(const AVFrameSideData *sd)
> +{
> +    return !!av_buffer_is_writable(sd->buf);

av_buffer_is_writable() is documented to only return 0 or 1, so the
binarization is unnecessary.

> +}
> +
> +int av_frame_side_data_make_writable(AVFrameSideData *sd)
> +{
> +    AVBufferRef *buf = NULL;
> +
> +    if (av_buffer_is_writable(sd->buf))
> +        return 0;
> +
> +    buf = av_buffer_alloc(sd->size);
> +    if (!buf)
> +        return AVERROR(ENOMEM);
> +
> +    if (sd->size)
> +        memcpy(buf->data, sd->data, sd->size);
> +    av_buffer_unref(&sd->buf);
> +    sd->buf  = buf;
> +    sd->data = buf->data;
> +
> +    return 0;
> +}

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".