Re: [FFmpeg-devel] [PATCH 55/97] Vulkan patchset part 2 - hwcontext rewrite and filtering

[Anton Khirnov <anton@khirnov.net>]

Quoting Lynne (2023-05-16 16:46:45)
> May 16, 2023, 15:41 by anton@khirnov.net:
> 
> > Quoting Lynne (2023-05-11 20:13:29)
> >
> >> >> diff --git a/libavutil/vulkan.h b/libavutil/vulkan.h
> >> >> index 4bd1c9fc00..4c38dbc2e6 100644
> >> >> --- a/libavutil/vulkan.h
> >> >> +++ b/libavutil/vulkan.h
> >> >> @@ -216,6 +216,9 @@ typedef struct FFVulkanContext {
> >> >>  VkPhysicalDeviceProperties2 props;
> >> >>  VkPhysicalDeviceDriverProperties driver_props;
> >> >>  VkPhysicalDeviceMemoryProperties mprops;
> >> >> +    VkQueueFamilyQueryResultStatusPropertiesKHR *query_props;
> >> >> +    VkQueueFamilyVideoPropertiesKHR *video_props;
> >> >> +    VkQueueFamilyProperties2 *qf_props;
> >> >>
> >> >
> >> > How does the user of these know how many elements are in each array?
> >> >
> >>
> >> They don't have to, we read the total number of queues available
> >> for the device, so all indices are always available.
> >>
> >
> > "all indices"?
> >
> > The allocated size of these arrays is purely local to
> > ff_vk_load_props(), so there is no safe way for any outside caller to
> > know it.
> >
> 
> The init function queries the driver for the total number of queue family indices,
> allocates an array of that amount for each structure, and reads the properties
> into the array.
> API users then index into the array based on the queue family index.
> API users cannot index outside of the array ever, as the queue family index
> they receive is always AVVulkanDeviceContext.queue_family_index (or the
> transfer, compute, encode, or decode queue family index member of that structure).
> The queue family index members of that structure are checked upon initialization
> to not be larger than what the driver returns.
> 
> Hence, there's no need for them to know how large the array is, as
> it is allocated for all possible indices.

That's way too much indirection and way too much code that has to make
the exact same unstated assumption on what the actual size is. In my
experience, it is almost always a good idea to be explicit: store the
exact array size right next to the array itself.

If nothing else, it will be very helpful for the person debugging the
inevitable invalid accesses.

-- 
Anton Khirnov
_______________________________________________
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".