[FFmpeg-devel] [PATCH] avutil/frame: ensure the frame is writable in av_frame_copy()

[James Almer]

On 3/11/2021 10:56 AM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>   libavutil/frame.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index eab51b6a32..ec79d053e1 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -800,6 +800,8 @@ int av_frame_copy(AVFrame *dst, const AVFrame *src)
>>   {
>>       if (dst->format != src->format || dst->format < 0)
>>           return AVERROR(EINVAL);
>> +    if (!av_frame_is_writable(dst))
>> +        return AVERROR(EINVAL);
>>   
>>       if (dst->width > 0 && dst->height > 0)
>>           return frame_copy_video(dst, src);
>>
> This will break scenarios where one owns all the references to a frame's
> data without the frame being writable?

It would, but that's by design. We define a frame as writable when its 
buffers are both reference counted and there's at most a single 
reference to them. Who owns any of the references does not play a part 
in that.
There's av_image_copy() if you really want to just force copying and 
take the risk, but to me an AVFrame helper function like this must 
properly follow the struct's requirements.

FATE doesn't break, so that scenario is apparently currently not 
happening within the libraries. And if it was, it would require the 
target frame to be made writable.

That being said, if someone wants to give introducing a method to 
signal/identify ownership of references a try, that would be cool indeed.