Re: [FFmpeg-devel] [PATCH 7/9] avutil/hwcontext_d3d12va: add Flags for resource creation

From: Mark Thompson <sw@jkqxz.net>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 7/9] avutil/hwcontext_d3d12va: add Flags for resource creation
Date: Mon, 22 Jan 2024 21:52:27 +0000
Message-ID: <52035da0-1c0a-48fc-8846-6f2705230fb3@jkqxz.net> (raw)
In-Reply-To: <20240122055756.1142-7-tong1.wu@intel.com>

On 22/01/2024 05:57, tong1.wu-at-intel.com@ffmpeg.org wrote:
> From: Tong Wu <tong1.wu@intel.com>
> 
> Flags field is added to support diffferent resource creation.
> 
> Signed-off-by: Tong Wu <tong1.wu@intel.com>
> ---
>   doc/APIchanges                | 3 +++
>   libavutil/hwcontext_d3d12va.c | 2 +-
>   libavutil/hwcontext_d3d12va.h | 5 +++++
>   libavutil/version.h           | 2 +-
>   4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index e477ed78e0..a33e54dd3b 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>   
>   API changes, most recent first:
>   
> +2024-01-xx - xxxxxxxxxx - lavu 58.37.100 - hwcontext_d3d12va.h
> + Add AVD3D12VAFramesContext.Flags
> +
>   2023-11-xx - xxxxxxxxxx - lavfi 9.16.100 - buffersink.h buffersrc.h
>     Add av_buffersink_get_colorspace and av_buffersink_get_color_range.
>     Add AVBufferSrcParameters.color_space and AVBufferSrcParameters.color_range.
> diff --git a/libavutil/hwcontext_d3d12va.c b/libavutil/hwcontext_d3d12va.c
> index 414dd44290..0d94f48543 100644
> --- a/libavutil/hwcontext_d3d12va.c
> +++ b/libavutil/hwcontext_d3d12va.c
> @@ -237,7 +237,7 @@ static AVBufferRef *d3d12va_pool_alloc(void *opaque, size_t size)
>           .Format           = hwctx->format,
>           .SampleDesc       = {.Count = 1, .Quality = 0 },
>           .Layout           = D3D12_TEXTURE_LAYOUT_UNKNOWN,
> -        .Flags            = D3D12_RESOURCE_FLAG_NONE,
> +        .Flags            = hwctx->Flags,

This seems like a hole in the existing decoder implementation?  How does it work without making D3D12_RESOURCE_FLAG_VIDEO_DECODE_REFERENCE_ONLY textures when required by the device?

>       };
>   
>       frame = av_mallocz(sizeof(AVD3D12VAFrame));
> diff --git a/libavutil/hwcontext_d3d12va.h b/libavutil/hwcontext_d3d12va.h
> index ff06e6f2ef..dc1c17d3f9 100644
> --- a/libavutil/hwcontext_d3d12va.h
> +++ b/libavutil/hwcontext_d3d12va.h
> @@ -129,6 +129,11 @@ typedef struct AVD3D12VAFramesContext {
>        * If unset, will be automatically set.
>        */
>       DXGI_FORMAT format;
> +
> +    /**
> +     * This field is used for resource creation.

This documentation could be better.  Used to do what?  Can it not be set, and what is the behaviour if it isn't?

> +     */
> +    D3D12_RESOURCE_FLAGS Flags;

Use lowercase for structure elements.

>   } AVD3D12VAFramesContext;
>   
>   #endif /* AVUTIL_HWCONTEXT_D3D12VA_H */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 772c4e209c..3ad1a9446c 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,7 +79,7 @@
>    */
>   
>   #define LIBAVUTIL_VERSION_MAJOR  58
> -#define LIBAVUTIL_VERSION_MINOR  36
> +#define LIBAVUTIL_VERSION_MINOR  37
>   #define LIBAVUTIL_VERSION_MICRO 101
>   
>   #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \

It's not good to be changing the user-facing API like this.  Are there any more properties here which really should be user-visible?  Adding them one at a time later would be very unfortunate and make compatibility harder.

Thanks,

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