Re: [FFmpeg-devel] libavcodec: add Vulkan common video decoding code

[Lynne <dev@lynne.ee>]

May 18, 2023, 10:54 by anton@khirnov.net:

>> commit adb671b921d006255597ac126f85adb05f9d6677
>> Author: Lynne <dev@lynne.ee>
>> Date:   Mon Jan 16 07:23:27 2023 +0100
>>
>>  libavcodec: add Vulkan common video decoding code
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index d99f7bd25a..362ea31e3e 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -1289,7 +1289,7 @@ SKIPHEADERS-$(CONFIG_XVMC)             += xvmc.h
>>  SKIPHEADERS-$(CONFIG_VAAPI)            += vaapi_decode.h vaapi_hevc.h vaapi_encode.h
>>  SKIPHEADERS-$(CONFIG_VDPAU)            += vdpau.h vdpau_internal.h
>>  SKIPHEADERS-$(CONFIG_VIDEOTOOLBOX)     += videotoolbox.h vt_internal.h
>> -SKIPHEADERS-$(CONFIG_VULKAN)           += vulkan.h vulkan_video.h
>> +SKIPHEADERS-$(CONFIG_VULKAN)           += vulkan.h vulkan_video.h vulkan_decode.h
>>  SKIPHEADERS-$(CONFIG_V4L2_M2M)         += v4l2_buffers.h v4l2_context.h v4l2_m2m.h
>>  SKIPHEADERS-$(CONFIG_ZLIB)             += zlib_wrapper.h
>>  
>> diff --git a/libavcodec/vulkan_decode.c b/libavcodec/vulkan_decode.c
>> new file mode 100644
>> index 0000000000..d07b9aa3eb
>> --- /dev/null
>> +++ b/libavcodec/vulkan_decode.c
>> @@ -0,0 +1,1182 @@
>> +/*
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>> + */
>> +
>> +#include "vulkan_video.h"
>> +#include "vulkan_decode.h"
>> +#include "config_components.h"
>> +#include "libavutil/hwcontext_internal.h"
>>
>
> what for?
>
>> +static AVFrame *vk_get_dpb_pool(FFVulkanDecodeShared *ctx)
>> +{
>> +    AVFrame *avf = av_frame_alloc();
>> +    AVHWFramesContext *dpb_frames = (AVHWFramesContext *)ctx->dpb_hwfc_ref->data;
>> +    if (!avf)
>> +        return NULL;
>> +
>> +    avf->hw_frames_ctx = av_buffer_ref(ctx->dpb_hwfc_ref);
>> +    if (!avf->hw_frames_ctx)
>> +        av_frame_free(&avf);
>> +    avf->buf[0] = av_buffer_pool_get(dpb_frames->pool);
>> +    if (!avf->buf[0])
>> +        av_frame_free(&avf);
>> +    avf->data[0] = avf->buf[0]->data;
>>
>
> Why is this not av_hwframe_get_buffer()?
>

Didn't occur to me. Fixed.


>> +void ff_vk_decode_free_frame(FFVulkanDecodeContext *dec, FFVulkanDecodePicture *vp)
>> +{
>> +    FFVulkanFunctions *vk;
>> +    VkSemaphoreWaitInfo sem_wait;
>> +    FFVulkanDecodeShared *ctx;
>> +
>> +    // TODO: investigate why this happens
>> +    if (!dec || !dec->shared_ref) {
>>
>
> My guess is that this is called from a different thread than the one
> whose hwaccel_priv_data you gave to ff_hwaccel_frame_priv_alloc().
> You have to attach everything you need to hwaccel_priv_buf itself.
>

This was an old todo which I fixed previously. Removed.


>> +               avcodec_get_name(avctx->codec_id),
>> +               avcodec_profile_name(avctx->codec_id, avctx->profile),
>> +               avcodec_profile_name(avctx->codec_id, base_profile));
>> +        goto repeat;
>>
>
> This function is long and ugly enough even without backward gotos. What
> would Dijkstra say?
>

I tried to do it with a function, but the result was more sphagetti
and code duplication, due to needing to handle the return code.
I've commented the goto parts better.


>> +#endif /* AVCODEC_VULKAN_DECODE_H */
>> diff --git a/libavutil/vulkan.c b/libavutil/vulkan.c
>> index 0ab90c8f3c..db47956198 100644
>> --- a/libavutil/vulkan.c
>> +++ b/libavutil/vulkan.c
>> @@ -510,8 +510,8 @@ void ff_vk_exec_discard_deps(FFVulkanContext *s, FFVkExecContext *e)
>>  AVVkFrame *vkf = (AVVkFrame *)f->data[0];
>>  vkfc->unlock_frame(hwfc, vkf);
>>  e->frame_locked[j] = 0;
>> -            e->frame_update[j] = 0;
>>  }
>> +        e->frame_update[j] = 0;
>>
>
> unrelated?
>

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