Re: [FFmpeg-devel] [PATCH 2/3] hwcontext_vulkan: add queue and frame locking functions

[Lynne <dev@lynne.ee>]

5 Apr 2022, 11:38 by anton@khirnov.net:

> Quoting Lynne (2022-04-03 16:52:24) 
>
>> +#include <pthread.h>
>>
>
> Either make vulkan depend on threads or make this all conditional somehow.
>

I'll make it optional. The locking functions would do nothing in that case,
with a note for the API user.


>> @@ -4129,7 +4209,19 @@ static int vulkan_frames_derive_to(AVHWFramesContext *dst_fc,
>>  
>>  AVVkFrame *av_vk_frame_alloc(void)
>>  {
>> -    return av_mallocz(sizeof(AVVkFrame));
>> +    AVVkFrame *f = av_mallocz(sizeof(AVVkFrame));
>> +    if (!f)
>> +        return NULL;
>> +
>> +    f->internal = av_mallocz(sizeof(*f->internal));
>>
>
> Doxy says AVVkFrame can be freed with av_free.
>

That's already been broken, since if an API user passed an AVVkFrame
to map from a CUDA, it would allocate the internal field anyway.
What about making the internal field its own struct which would
live in AVFrame->buf[1]? That way, it would get free'd on av_frame_free().
The AVVkFrame field would get deprecated.


> I'd expect some prescription for who calls this and when. Like
> "Any users accessing any queues associated with this device MUST call
>  this callback before manipulating the queue and unlock_queue() after
>  they are done."
>
> Same for lock_frame()
>

Will add stricter notes.


>> +     */
>> +    void (*lock_queue)(AVHWDeviceContext *ctx, int queue_family, int index);
>>
>
> any reason those are signed?
>

Not really, I can change them into uint32_t's, as that's what Vulkan
functions take.


>> +
>> +    /**
>> +     * Similar to lock_queue(), unlocks a queue. Must only be called after it.
>>
> s/similar/complementary/
>

Fixed.

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