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 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?

> +        }
> +        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.

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.

(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.

> + *
> + * @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.)

> + * @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.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to