diff mbox series

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

Message ID 20240122055756.1142-7-tong1.wu@intel.com
State New
Headers show
Series [FFmpeg-devel,1/9] avcodec/vaapi_encode: move pic->input_surface initialization to encode_alloc | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Wu, Tong1 Jan. 22, 2024, 5:57 a.m. UTC 
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(-)

Comments

Mark Thompson Jan. 22, 2024, 9:52 p.m. UTC | #1 
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
Wu, Tong1 Jan. 23, 2024, 5:40 a.m. UTC | #2 
>From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Mark
>Thompson
>Sent: Tuesday, January 23, 2024 5:52 AM
>To: ffmpeg-devel@ffmpeg.org
>Subject: Re: [FFmpeg-devel] [PATCH 7/9] avutil/hwcontext_d3d12va: add Flags
>for resource creation
>
>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?

For current decoder implementation it applies to most devices and it's indeed not supported when driver requests this REFERENCE_ONLY flag. It seems that I haven't met this request in my working environment. We may add it later if needed.

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

Will update in V2.

>
>> +     */
>> +    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

AFAIK, I don't think there're more properties that need to be added one by one in a short time. It's going to be just this one that is really needed since we have to give specific flags for recon frames creation during encoding.

Thanks for the opinions and I'll update those in next version.

Best Regards,
Tong


>_______________________________________________
>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".
diff mbox series

Patch

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,
     };
 
     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.
+     */
+    D3D12_RESOURCE_FLAGS Flags;
 } 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, \