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

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

[Mark Thompson]

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);
-- 
2.35.1
_______________________________________________
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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Mark
> Thompson
> Sent: Tuesday, May 3, 2022 2:09 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@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.

Regards,
softworkz



_______________________________________________
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".

Re: [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@ffmpeg.org> On Behalf Of Mark
>> Thompson
>> Sent: Tuesday, May 3, 2022 2:09 AM
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@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
_______________________________________________
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".

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

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Mark
> Thompson
> Sent: Tuesday, May 3, 2022 10:32 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] hwcontext: Skip derivation
> checking for an existing device if options are set
> 
> On 03/05/2022 01:14, Soft Works wrote:
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Mark
> >> Thompson
> >> Sent: Tuesday, May 3, 2022 2:09 AM
> >> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel@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 thought you were saying that when deriving a device with extensionParam1
an existing device should be returned when it was also derived with
extensionParam1 but when there's no existing device of the requested
type _and_ matching parameters, then a new derived device should be
created.

Maybe I understood wrong?


Kind regards,
softworkz





_______________________________________________
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".