diff mbox series

[FFmpeg-devel,3/3] avcodec/vulkan_decode: Un-sparse extensions table

Message ID AS8P250MB0744815E4DB41F77E622ADCD8F5C2@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/vulkan_video: Don't use sparse table | expand

Checks

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

Commit Message

Andreas Rheinhardt March 3, 2024, 11:37 a.m. UTC 
Only three of the 226 (== AV_CODEC_ID_AV1) entries
have been used. Unsparsing this table is especially
important given that this array lives in .data.rel.ro.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Instead of parallel tables, one could also merge VkCodecMap and
VkExtensionProperties (i.e. putting one of the latter inside
the former) if preferred.

 libavcodec/vulkan_decode.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

Comments

Lynne March 3, 2024, 1:03 p.m. UTC | #1 
Mar 3, 2024, 12:35 by andreas.rheinhardt@outlook.com:

> Only three of the 226 (== AV_CODEC_ID_AV1) entries
> have been used. Unsparsing this table is especially
> important given that this array lives in .data.rel.ro.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Instead of parallel tables, one could also merge VkCodecMap and
> VkExtensionProperties (i.e. putting one of the latter inside
> the former) if preferred.
>
> libavcodec/vulkan_decode.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>

LGTM on commits 1 and 3.
Would you mind keeping the designated initializers from
commit 2 but still keeping it into vulkan_video.c?
Would save on needing to deduplicate it in the future.
Andreas Rheinhardt March 3, 2024, 1:08 p.m. UTC | #2 
Lynne:
> Mar 3, 2024, 12:35 by andreas.rheinhardt@outlook.com:
> 
>> Only three of the 226 (== AV_CODEC_ID_AV1) entries
>> have been used. Unsparsing this table is especially
>> important given that this array lives in .data.rel.ro.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> Instead of parallel tables, one could also merge VkCodecMap and
>> VkExtensionProperties (i.e. putting one of the latter inside
>> the former) if preferred.
>>
>> libavcodec/vulkan_decode.c | 28 ++++++++++++++++++++++------
>> 1 file changed, 22 insertions(+), 6 deletions(-)
>>
> 
> LGTM on commits 1 and 3.
> Would you mind keeping the designated initializers from
> commit 2 but still keeping it into vulkan_video.c?
> Would save on needing to deduplicate it in the future.

Deduplicate? There are no common fields for decoding and encoding in
FFVkCodecMap.

- Andreas
Andreas Rheinhardt March 7, 2024, 12:36 a.m. UTC | #3 
Lynne:
> Mar 3, 2024, 12:35 by andreas.rheinhardt@outlook.com:
> 
>> Only three of the 226 (== AV_CODEC_ID_AV1) entries
>> have been used. Unsparsing this table is especially
>> important given that this array lives in .data.rel.ro.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> Instead of parallel tables, one could also merge VkCodecMap and
>> VkExtensionProperties (i.e. putting one of the latter inside
>> the former) if preferred.
>>
>> libavcodec/vulkan_decode.c | 28 ++++++++++++++++++++++------
>> 1 file changed, 22 insertions(+), 6 deletions(-)
>>
> 
> LGTM on commits 1 and 3.
> Would you mind keeping the designated initializers from
> commit 2 but still keeping it into vulkan_video.c?
> Would save on needing to deduplicate it in the future.

I still don't know what you mean by "deduplicating" them in the future.
Anyway, I have now implemented the approach outlined in the comment
above: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-March/322893.html

- Andreas
diff mbox series

Patch

diff --git a/libavcodec/vulkan_decode.c b/libavcodec/vulkan_decode.c
index b80415a019..121ed40f40 100644
--- a/libavcodec/vulkan_decode.c
+++ b/libavcodec/vulkan_decode.c
@@ -28,22 +28,29 @@  typedef struct VkCodecMap {
     VkVideoCodecOperationFlagBitsKHR decode_op;
 } VkCodecMap;
 
+/* The following table and dec_ext below are supposed to be parallel. */
 static const VkCodecMap vk_codec_map[] = {
+#if CONFIG_H264_VULKAN_HWACCEL
     {
         .codec_id         = AV_CODEC_ID_H264,
         .decode_extension = FF_VK_EXT_VIDEO_DECODE_H264,
         .decode_op        = VK_VIDEO_CODEC_OPERATION_DECODE_H264_BIT_KHR,
     },
+#endif
+#if CONFIG_HEVC_VULKAN_HWACCEL
     {
         .codec_id         = AV_CODEC_ID_HEVC,
         .decode_extension = FF_VK_EXT_VIDEO_DECODE_H265,
         .decode_op        = VK_VIDEO_CODEC_OPERATION_DECODE_H265_BIT_KHR,
     },
+#endif
+#if CONFIG_AV1_VULKAN_HWACCEL
     {
         .codec_id         = AV_CODEC_ID_AV1,
         .decode_extension = FF_VK_EXT_VIDEO_DECODE_AV1,
         .decode_op        = 0x01000000, /* TODO fix this */
     },
+#endif
 };
 
 #if CONFIG_H264_VULKAN_HWACCEL
@@ -58,16 +65,19 @@  extern const VkExtensionProperties ff_vk_dec_av1_ext;
 
 static const VkExtensionProperties *dec_ext[] = {
 #if CONFIG_H264_VULKAN_HWACCEL
-    [AV_CODEC_ID_H264] = &ff_vk_dec_h264_ext,
+    &ff_vk_dec_h264_ext,
 #endif
 #if CONFIG_HEVC_VULKAN_HWACCEL
-    [AV_CODEC_ID_HEVC] = &ff_vk_dec_hevc_ext,
+    &ff_vk_dec_hevc_ext,
 #endif
 #if CONFIG_AV1_VULKAN_HWACCEL
-    [AV_CODEC_ID_AV1] = &ff_vk_dec_av1_ext,
+    &ff_vk_dec_av1_ext,
 #endif
 };
 
+_Static_assert(FF_ARRAY_ELEMS(dec_ext) == FF_ARRAY_ELEMS(vk_codec_map),
+               "dec_ext and vk_codec_map out-of-sync");
+
 static const VkCodecMap *get_codecmap(enum AVCodecID codec_id)
 {
     for (size_t i = 0; i < FF_ARRAY_ELEMS(vk_codec_map); i++)
@@ -77,6 +87,11 @@  static const VkCodecMap *get_codecmap(enum AVCodecID codec_id)
     return NULL;
 }
 
+static const VkExtensionProperties *get_extension(const VkCodecMap *vk_codec)
+{
+    return dec_ext[vk_codec - vk_codec_map];
+}
+
 static const VkVideoProfileInfoKHR *get_video_profile(FFVulkanDecodeShared *ctx, enum AVCodecID codec_id)
 {
     const VkVideoProfileListInfoKHR *profile_list;
@@ -772,6 +787,7 @@  static int vulkan_decode_get_profile(AVCodecContext *avctx, AVBufferRef *frames_
     VkResult ret;
     int max_level, base_profile, cur_profile;
     const VkCodecMap *vk_codec = get_codecmap(avctx->codec_id);
+    const VkExtensionProperties *extension = get_extension(vk_codec);
     AVHWFramesContext *frames = (AVHWFramesContext *)frames_ref->data;
     AVHWDeviceContext *device = (AVHWDeviceContext *)frames->device_ref->data;
     AVVulkanDeviceContext *hwctx = device->hwctx;
@@ -890,10 +906,10 @@  static int vulkan_decode_get_profile(AVCodecContext *avctx, AVBufferRef *frames_
            caps->maxActiveReferencePictures);
     av_log(avctx, AV_LOG_VERBOSE, "    Codec header name: '%s' (driver), '%s' (compiled)\n",
            caps->stdHeaderVersion.extensionName,
-           dec_ext[avctx->codec_id]->extensionName);
+           extension->extensionName);
     av_log(avctx, AV_LOG_VERBOSE, "    Codec header version: %i.%i.%i (driver), %i.%i.%i (compiled)\n",
            CODEC_VER(caps->stdHeaderVersion.specVersion),
-           CODEC_VER(dec_ext[avctx->codec_id]->specVersion));
+           CODEC_VER(extension->specVersion));
     av_log(avctx, AV_LOG_VERBOSE, "    Decode modes:%s%s%s\n",
            dec_caps->flags ? "" :
                " invalid",
@@ -1221,7 +1237,7 @@  int ff_vk_decode_init(AVCodecContext *avctx)
     session_create.maxActiveReferencePictures = ctx->caps.maxActiveReferencePictures;
     session_create.pictureFormat = s->hwfc->format[0];
     session_create.referencePictureFormat = session_create.pictureFormat;
-    session_create.pStdHeaderVersion = dec_ext[avctx->codec_id];
+    session_create.pStdHeaderVersion = get_extension(vk_codec);
     session_create.pVideoProfile = profile;
 
     /* Create decode exec context for this specific main thread.