[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 at ffmpeg.org> On Behalf Of Mark
> Thompson
> Sent: Tuesday, May 3, 2022 10:32 PM
> To: ffmpeg-devel at 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 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 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