On 20/02/17 05:37, wm4 wrote:
> On Sun, 19 Feb 2017 18:46:23 +0000
> Mark Thompson <[email protected]> wrote:
>
>> Creates a new device context from another of a different type which
>> refers to the same underlying hardware.
>> ---
>> libavutil/hwcontext.c | 78
>> ++++++++++++++++++++++++++++++++++++++++++
>> libavutil/hwcontext.h | 16 +++++++++
>> libavutil/hwcontext_internal.h | 8 +++++
>> 3 files changed, 102 insertions(+)
>>
>> diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c
>> index 608da6872..1f9442622 100644
>> --- a/libavutil/hwcontext.c
>> +++ b/libavutil/hwcontext.c
>> @@ -65,6 +65,8 @@ static void hwdevice_ctx_free(void *opaque, uint8_t *data)
>> if (ctx->free)
>> ctx->free(ctx);
>>
>> + av_buffer_unref(&ctx->internal->source_device);
>> +
>> av_freep(&ctx->hwctx);
>> av_freep(&ctx->internal->priv);
>> av_freep(&ctx->internal);
>> @@ -535,6 +537,82 @@ fail:
>> return ret;
>> }
>>
>> +int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ref_ptr,
>> + enum AVHWDeviceType type,
>> + AVBufferRef *src_ref, int flags)
>> +{
>> + AVBufferRef *dst_ref = NULL, *tmp_ref;
>> + AVHWDeviceContext *dst_ctx, *src_ctx, *tmp_ctx;
>> + int ret = 0;
>> +
>> + src_ctx = (AVHWDeviceContext*)src_ref->data;
>> + if (src_ctx->type == type) {
>> + // Currently disallowed. It is possible that deriving a device of
>> + // the same type could do something useful (device partitioning?),
>> + // but for now just don't support it.
>> + return AVERROR(EINVAL);
>
> It could just be a no-nop (basically creating an alias). Whether that'd
> be useful, who knows.
If you can come up with any case where that is useful, then sure. I couldn't
think of one, so making it always fail rather than doing something unhelpful
seemed reasonable.
>> + }
>> +
>> + // If the source device was originally derived from a device of the
>> + // target type (possibly through more intermediates), then return a
>> + // new reference to the original device.
>> + tmp_ref = src_ctx->internal->source_device;
>> + 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;
>
> In fact, it seems this searches and returns a new reference if a
> "similar" device is found. So my suggestion above would make sense?
The point of this is that it allows X -> Y -> X derivation to succeed when
derivation is only possible in one direction. For example, VAAPI -> OpenCL ->
VAAPI: OpenCL -> VAAPI isn't possible in isolation, but we would still like it
to work in this case because it is helpful with hwmap in filter graphs.
>> + }
>> + tmp_ref = tmp_ctx->internal->source_device;
>> + }
>> +
>> + // Attempt to derive a new device from the given source device, or
>> + // any of it's ancestors.
>> + dst_ref = av_hwdevice_ctx_alloc(type);
>> + if (!dst_ref) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + dst_ctx = (AVHWDeviceContext*)dst_ref->data;
>> +
>> + tmp_ref = src_ref;
>> + while (tmp_ref) {
>> + tmp_ctx = (AVHWDeviceContext*)tmp_ref->data;
>> + if (dst_ctx->internal->hw_type->device_derive) {
>> + ret = dst_ctx->internal->hw_type->device_derive(dst_ctx,
>> + tmp_ctx,
>> + flags);
>> + if (ret == 0) {
>> + dst_ctx->internal->source_device = av_buffer_ref(src_ref);
>> + if (!dst_ctx->internal->source_device) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> + goto done;
>> + }
>> + if (ret != AVERROR(ENOSYS))
>> + goto fail;
>> + }
>> + tmp_ref = tmp_ctx->internal->source_device;
>> + }
>
> Pretty scary, but I guess it does the trick. I hope we won't have to
> think about generalized device graphs, and it'll always stay a simple
> list.
Too late :) The example in 0/24 is initially VAAPI -> OpenCL, and we are
making use of this magic to do the OpenCL -> QSV derivation by going back up
the tree to the VAAPI device and deriving from there.
> One thing that makes me wonder is, if you derive a device type B from a
> device of type A multiple times, it will call device_derive() for each
> device of type A created. Except if device A is derived from a device C
> of type B, then the first loop will create a new reference to device C
> if a device of type B is requested. That makes me wonder whether the
> first loop is even worth having, since such use-cases imply the
> device_derive implementation should be either fast or cache the derived
> devices. Or maybe all "shared" devices should share a cache of derived
> devices.
The derivation structure is currently a tree with a clear root, though leaves
might be copies of devices higher in the tree. As long as we don't introduce
both X -> Y and Y -> X derivations which end up with different semantics then I
don't think this is a worry.
> (It's probably fine to ignore what I just wrote, I might be worrying
> about nothing.)
>
>> +
>> + ret = AVERROR(ENOSYS);
>> + goto fail;
>> +
>> +done:
>> + *dst_ref_ptr = dst_ref;
>> + return 0;
>> +
>> +fail:
>> + av_buffer_unref(&dst_ref);
>> + *dst_ref_ptr = NULL;
>> + return ret;
>> +}
>> +
>> static void ff_hwframe_unmap(void *opaque, uint8_t *data)
>> {
>> HWMapDescriptor *hwmap = (HWMapDescriptor*)data;
>> diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h
>> index 037ca6422..a31799267 100644
>> --- a/libavutil/hwcontext.h
>> +++ b/libavutil/hwcontext.h
>> @@ -269,6 +269,22 @@ int av_hwdevice_ctx_create(AVBufferRef **device_ctx,
>> enum AVHWDeviceType type,
>> const char *device, AVDictionary *opts, int
>> flags);
>>
>> /**
>> + * Create a new device of the specified type from an existing device.
>
> Maybe it could mention that it can also return a new reference to an
> existing, cached derived device.
Yeah, it should just describe the process above.
>> + *
>> + * @param dst_ctx On success, a reference to the newly-created
>> + * AVHWDeviceContext.
>> + * @param type The type of the new device to create.
>> + * @param src_ctx A reference to an existing AVHWDeviceContext which will be
>> + * used to create the new device.
>> + * @param flags Currently unused; should be set to zero.
>
> I'm really not sure about all those unused "flags" parameters we're
> adding to the hwdevice APIs. If we need something new, the flags
> parameter will most likely be unused.
>
> (Just compare with those "reserved" parameters in win32 APIs.)
I add a use for one of the them in a later patch.
>> + * @return Zero on success, a negative AVERROR code on failure.
>> + */
>> +int av_hwdevice_ctx_create_derived(AVBufferRef **dst_ctx,
>> + enum AVHWDeviceType type,
>> + AVBufferRef *src_ctx, int flags);
>> +
>> +
>> +/**
>> * Allocate an AVHWFramesContext tied to a given device context.
>> *
>> * @param device_ctx a reference to a AVHWDeviceContext. This function will
>> make
>> diff --git a/libavutil/hwcontext_internal.h b/libavutil/hwcontext_internal.h
>> index 11e3a68e4..66f54142e 100644
>> --- a/libavutil/hwcontext_internal.h
>> +++ b/libavutil/hwcontext_internal.h
>> @@ -66,6 +66,8 @@ typedef struct HWContextType {
>>
>> int (*device_create)(AVHWDeviceContext *ctx, const char
>> *device,
>> AVDictionary *opts, int flags);
>> + int (*device_derive)(AVHWDeviceContext *dst_ctx,
>> + AVHWDeviceContext *src_ctx, int
>> flags);
>>
>> int (*device_init)(AVHWDeviceContext *ctx);
>> void (*device_uninit)(AVHWDeviceContext *ctx);
>> @@ -95,6 +97,12 @@ typedef struct HWContextType {
>> struct AVHWDeviceInternal {
>> const HWContextType *hw_type;
>> void *priv;
>> +
>> + /**
>> + * For a derived device, a reference to the original device
>> + * context it was derived from.
>> + */
>> + AVBufferRef *source_device;
>> };
>>
>> struct AVHWFramesInternal {
>
> In general seems like an ok idea.
Thanks,
- Mark
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
