| Message ID | bfb6b1ad-855e-c5fe-f8f8-606783a83672@jkqxz.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] hwcontext: Skip derivation checking for an existing device if options are set | expand |
| Context | Check | Description |
|---|---|---|
| andriy/configure_x86 | warning | Failed to apply patch |
| yinshiyou/configure_loongarch64 | warning | Failed to apply patch |
> -----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
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
> -----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
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);