[FFmpeg-devel,1/3] avutil/frame: make frame copy functions hwframe aware

---
 libavutil/frame.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

On 3/27/2020 5:43 PM, Timo Rothenpieler wrote:
> ---
>  libavutil/frame.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index e4038096c2..0c64f4a422 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -25,6 +25,7 @@
>  #include "imgutils.h"
>  #include "mem.h"
>  #include "samplefmt.h"
> +#include "hwcontext.h"
>  
>  #if FF_API_FRAME_GET_SET
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, best_effort_timestamp)
> @@ -626,9 +627,22 @@ int av_frame_make_writable(AVFrame *frame)
>      tmp.channels       = frame->channels;
>      tmp.channel_layout = frame->channel_layout;
>      tmp.nb_samples     = frame->nb_samples;
> -    ret = av_frame_get_buffer(&tmp, 32);
> -    if (ret < 0)
> -        return ret;
> +
> +    if (frame->hw_frames_ctx) {
> +        ret = av_hwframe_get_buffer(frame->hw_frames_ctx, &tmp, 0);
> +        if (ret < 0)
> +            return ret;
> +
> +        tmp.hw_frames_ctx = av_buffer_ref(frame->hw_frames_ctx);

This is already done by av_hwframe_get_buffer().

> +        if (!tmp.hw_frames_ctx) {
> +            av_frame_unref(&tmp);
> +            return AVERROR(ENOMEM);
> +        }
> +    } else {
> +        ret = av_frame_get_buffer(&tmp, 32);
> +        if (ret < 0)
> +            return ret;
> +    }
>  
>      ret = av_frame_copy(&tmp, frame);
>      if (ret < 0) {
> @@ -752,6 +766,9 @@ static int frame_copy_video(AVFrame *dst, const AVFrame *src)
>          dst->height < src->height)
>          return AVERROR(EINVAL);
>  
> +    if (src->hw_frames_ctx || dst->hw_frames_ctx)
> +        return av_hwframe_transfer_data(dst, src, 0);

Not all the hwcontext implementations support hw -> hw transfers, like
OpenCL and Vulkan.

OpenCL returns EINVAL but Vulkan ENOSYS, so that's fun. The former
should be changed i think.

> +
>      planes = av_pix_fmt_count_planes(dst->format);
>      for (i = 0; i < planes; i++)
>          if (!dst->data[i] || !src->data[i])
>

On 27.03.2020 21:53, James Almer wrote:
> On 3/27/2020 5:43 PM, Timo Rothenpieler wrote:
>> ---
>>   libavutil/frame.c | 23 ++++++++++++++++++++---
>>   1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index e4038096c2..0c64f4a422 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -25,6 +25,7 @@
>>   #include "imgutils.h"
>>   #include "mem.h"
>>   #include "samplefmt.h"
>> +#include "hwcontext.h"
>>   
>>   #if FF_API_FRAME_GET_SET
>>   MAKE_ACCESSORS(AVFrame, frame, int64_t, best_effort_timestamp)
>> @@ -626,9 +627,22 @@ int av_frame_make_writable(AVFrame *frame)
>>       tmp.channels       = frame->channels;
>>       tmp.channel_layout = frame->channel_layout;
>>       tmp.nb_samples     = frame->nb_samples;
>> -    ret = av_frame_get_buffer(&tmp, 32);
>> -    if (ret < 0)
>> -        return ret;
>> +
>> +    if (frame->hw_frames_ctx) {
>> +        ret = av_hwframe_get_buffer(frame->hw_frames_ctx, &tmp, 0);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        tmp.hw_frames_ctx = av_buffer_ref(frame->hw_frames_ctx);
> 
> This is already done by av_hwframe_get_buffer().

Indeed. Fixed locally.

>> +        if (!tmp.hw_frames_ctx) {
>> +            av_frame_unref(&tmp);
>> +            return AVERROR(ENOMEM);
>> +        }
>> +    } else {
>> +        ret = av_frame_get_buffer(&tmp, 32);
>> +        if (ret < 0)
>> +            return ret;
>> +    }
>>   
>>       ret = av_frame_copy(&tmp, frame);
>>       if (ret < 0) {
>> @@ -752,6 +766,9 @@ static int frame_copy_video(AVFrame *dst, const AVFrame *src)
>>           dst->height < src->height)
>>           return AVERROR(EINVAL);
>>   
>> +    if (src->hw_frames_ctx || dst->hw_frames_ctx)
>> +        return av_hwframe_transfer_data(dst, src, 0);
> 
> Not all the hwcontext implementations support hw -> hw transfers, like
> OpenCL and Vulkan.
> 
> OpenCL returns EINVAL but Vulkan ENOSYS, so that's fun. The former
> should be changed i think.

The copy would fail either way for hwframes, so I intentionally did not 
put any further checks here, and leave that kinda logic to 
av_hwframe_transfer_data.