[FFmpeg-devel] [PATCH] hwcontext: Skip derivation checking for an existing device if options are set

[Mark Thompson]

On 03/05/2022 01:14, Soft Works wrote:
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Mark
>> Thompson
>> Sent: Tuesday, May 3, 2022 2:09 AM
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel at ffmpeg.org>
>> Subject: [FFmpeg-devel] [PATCH] hwcontext: Skip derivation checking
>> for an existing device if options are set
>>
>> If options are set then the user definitely intends to create a new
>> device, so we shouldn't attempt to return an existing one.
>> ---
>>    libavutil/hwcontext.c | 26 ++++++++++++++++----------
>>    1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
>> index ab9ad3703e..c4e01e0e78 100644
>> --- a/libavutil/hwcontext.c
>> +++ b/libavutil/hwcontext.c
>> @@ -653,18 +653,24 @@ int
>> av_hwdevice_ctx_create_derived_opts(AVBufferRef **dst_ref_ptr,
>>        AVHWDeviceContext *dst_ctx, *tmp_ctx;
>>        int ret = 0;
>>
>> -    tmp_ref = src_ref;
>> -    while (tmp_ref) {
>> -        tmp_ctx = (AVHWDeviceContext*)tmp_ref->data;
>> -        if (tmp_ctx->type == type) {
>> -            dst_ref = av_buffer_ref(tmp_ref);
>> -            if (!dst_ref) {
>> -                ret = AVERROR(ENOMEM);
>> -                goto fail;
>> +    // If we were derived (possibly transitively) from a device of
>> the
>> +    // target type then we want to return that original device,
>> unless
>> +    // options are set in which case we can skip this check because
>> it
>> +    // is definitely intended to create a new device.
>> +    if (!options) {
>> +        tmp_ref = src_ref;
>> +        while (tmp_ref) {
>> +            tmp_ctx = (AVHWDeviceContext*)tmp_ref->data;
>> +            if (tmp_ctx->type == type) {
>> +                dst_ref = av_buffer_ref(tmp_ref);
>> +                if (!dst_ref) {
>> +                    ret = AVERROR(ENOMEM);
>> +                    goto fail;
>> +                }
>> +                goto done;
>>                }
>> -            goto done;
>> +            tmp_ref = tmp_ctx->internal->source_device;
>>            }
>> -        tmp_ref = tmp_ctx->internal->source_device;
>>        }
>>
>>        dst_ref = av_hwdevice_ctx_alloc(type);
>> --
> 
> This doesn't implement the matching of device parameters, which you
> were talking about:
> 
> You said that when deriving a device, an existing device should
> only be returned when the parameters are the same.
> 
> This patch is doing something quite different.

Why would that be useful in the current design?  As the documentation on create_derived states, it has two independent uses - this is the "retrieve source device" case, and for that it does not make sense to set the options argument because no new device is being created.

I will clarify the doxy on _opts.

- Mark