| Message ID | 20210311220921.28899-1-suji.velupillai@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavu: add VKAPI hwcontext implementation | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Mar 11, 2021, 23:09 by suji.velupillai@broadcom.com: > From: Suji Velupillai <suji.velupillai@broadcom.com> > > Initial commit to add VKAPI hardware accelerator implementation. > The depedency component vkil source code can be obtained from github > https://github.com/Broadcom/vkil > Is this code for hardware that no one will ever be able to get, like the last time something like this was sent? We've already had to turn down one such patch which added support for hardware some company wanted but literally no one else could get. The difference is this one is fully open-source as far as the code goes, since the kernel driver's going to be in 5.12, which is enough to call it good in my book, though it's still iffy. > +static int vkapi_transfer_get_formats(AVHWFramesContext *ctx, > + enum AVHWFrameTransferDirection dir, > + enum AVPixelFormat **formats) > +{ > + enum AVPixelFormat *fmts; > + int ret = 0; > + > + // this allocation freed in hwcontext::transfer_data_alloc > + fmts = av_malloc_array(2, sizeof(*fmts)); > + if (!fmts) { > + av_log(ctx, AV_LOG_ERROR, "vkapi_transfer_get_formats failed\n"); > + ret = AVERROR(ENOMEM); > + } else { > + fmts[0] = ctx->sw_format; > + fmts[1] = AV_PIX_FMT_NONE; > + *formats = fmts; > + } > + > + return ret; > +} > + > +static int vkapi_convert_AV2VK_Frame(vkil_buffer_surface *dst, > + const AVFrame *src) > Why the all-caps on AV2VK? And a capital 'F'. Just make it all lowercase, that is our code style. > + // populate the vkil_surface structure with the origin pointer on the host > + ret = vkapi_convert_AV2VK_Frame(surface, src); > + if (ret) { > + ret = av_image_alloc(tmp_data, linesize, src->width, src->height, > + src->format, VKIL_BUF_ALIGN); > + if (ret < 0) > + goto fail; > + > + av_image_copy(tmp_data, linesize, (const uint8_t **)src->data, > + src->linesize, src->format, src->width, src->height); > + > + for (i = 0; i < VKIL_BUF_NPLANES; i++) { > + surface->plane_top[i]= tmp_data[i]; > + surface->plane_bot[i]= tmp_data[VKIL_BUF_NPLANES + i]; > + surface->stride[i] = linesize[i]; > + } > + } > + > + surface->quality = dst->quality; > + > + ilctx = hw_framectx->ilctx; > + if (!ilctx) { > + ret = AVERROR(EINVAL); > + goto fail; > + } > + > + // blocking call as ffmpeg assumes the transfer is complete on return > No, it doesn't. Just ref the frame and unref it the next time a user tries to upload a new frame, and if the previous upload hasn't finished, block until it has. That way you can have asynchronous uploading, but the downloading has to be synchronous. I'm fine with this being left as-is for now though. > +/** > + * Allocated as AVHWDeviceContext.hwctx > + */ > +typedef struct VKAPIDeviceContext { > + /** > + * Holds pointers to hardware specific functions > + */ > + vkil_api *ilapi; > + /** > + * Internal functions definitions > + */ > + /** > + * Get the hwprops reference from the AVFrame:data[3] > + */ > + int (*frame_ref_hwprops)(const AVFrame *frame, void *phw_surface_desc); > Definitely doesn't need to be in this code. Just remove it and duplicate it wherever it's used elsewhere. > + /** > + * Set the hwprops into AVFrame:data[3] > + */ > + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface *hw_surface_desc); > Same as above. Again, all fields are already public. Users should do this themselves. > + /** > + * Get the hwprops from AVFrame:data[3] > + */ > + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface *hw_surface_desc); > Same. > + /** > + * Check if format is in an array > + */ > + int (*fmt_is_in)(int fmt, const int *fmts); > Same... > + /** > + * Convert AVPixelFormat to VKAPI equivalent pixel format > + */ > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); > This function can stay, but it needs no state at all, so just put it out side of the structure. Name it: const VkFormat *av_vkil_from_pixfmt(enum AVPixelFormat p); The *vk* namespace is taken by Vulkan already. > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > index 46ef211add..3ae607a3d6 100644 > --- a/libavutil/pixfmt.h > +++ b/libavutil/pixfmt.h > @@ -348,6 +348,12 @@ enum AVPixelFormat { > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V) > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > + /** > + * VKAPI hardware acceleration. > + * data[3] contains a pointer to the vkil_buffer_surface structure > Why [3]? Why not [0]? Is this some horrible hack to allow for some extremely weird software frames with an additional hardware frame? Or to simplify checking for whether a frame is software or hardware, despite the fact the format field already tells you exactly what it is, and planar YUVA frames will break the check? I think unless you have very good reasons, this should be [0].
On 3/11/2021 7:09 PM, suji.velupillai@broadcom.com wrote: > From: Suji Velupillai <suji.velupillai@broadcom.com> > > Initial commit to add VKAPI hardware accelerator implementation. > The depedency component vkil source code can be obtained from github > https://github.com/Broadcom/vkil > > Signed-off-by: Suji Velupillai <suji.velupillai@broadcom.com> > --- > configure | 8 +- > doc/APIchanges | 4 + > libavutil/Makefile | 2 + > libavutil/hwcontext.c | 4 + > libavutil/hwcontext.h | 1 + > libavutil/hwcontext_internal.h | 1 + > libavutil/hwcontext_vkapi.c | 522 +++++++++++++++++++++++++++++++++ > libavutil/hwcontext_vkapi.h | 104 +++++++ > libavutil/pixdesc.c | 4 + > libavutil/pixfmt.h | 6 + > 10 files changed, 655 insertions(+), 1 deletion(-) > create mode 100644 libavutil/hwcontext_vkapi.c > create mode 100644 libavutil/hwcontext_vkapi.h [...] > diff --git a/doc/APIchanges b/doc/APIchanges > index 13350c0db0..ccab2e6465 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,10 @@ libavutil: 2017-10-21 > > API changes, most recent first: > > +2021-03-11 - xxxxxxxxxx - lavu yy.yy.yyy - hwcontext.h > + Add AV_PIX_FMT_VKAPI > + Add AV_HWDEVICE_TYPE_VKAPI and implementation. Should ideally also mention the structs in hwcontext_vkapi.h in some form. > + > 2021-03-10 - xxxxxxxxxx - lavf 58.72.100 - avformat.h > Change AVBufferRef related AVStream function and struct size > parameter and fields type to size_t at next major bump. > diff --git a/libavutil/Makefile b/libavutil/Makefile > index 27bafe9e12..4044b133a3 100644 > --- a/libavutil/Makefile > +++ b/libavutil/Makefile > @@ -45,6 +45,7 @@ HEADERS = adler32.h \ > hwcontext_vaapi.h \ > hwcontext_videotoolbox.h \ > hwcontext_vdpau.h \ > + hwcontext_vkapi.h \ Add this header to the SKIPHEADERS list below in this file. > hwcontext_vulkan.h \ > imgutils.h \ > intfloat.h \ > @@ -185,6 +186,7 @@ OBJS-$(CONFIG_QSV) += hwcontext_qsv.o > OBJS-$(CONFIG_VAAPI) += hwcontext_vaapi.o > OBJS-$(CONFIG_VIDEOTOOLBOX) += hwcontext_videotoolbox.o > OBJS-$(CONFIG_VDPAU) += hwcontext_vdpau.o > +OBJS-$(CONFIG_VKAPI) += hwcontext_vkapi.o > OBJS-$(CONFIG_VULKAN) += hwcontext_vulkan.o > > OBJS += $(COMPAT_OBJS:%=../compat/%) [...] > diff --git a/libavutil/hwcontext_vkapi.h b/libavutil/hwcontext_vkapi.h > new file mode 100644 > index 0000000000..096602b42e > --- /dev/null > +++ b/libavutil/hwcontext_vkapi.h > @@ -0,0 +1,104 @@ > +/* > + * Copyright (c) 2018 Broadcom > + * > + * 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 > + */ > + > +#ifndef AVUTIL_HWCONTEXT_VKAPI_H > +#define AVUTIL_HWCONTEXT_VKAPI_H > + > +#include <vkil_api.h> > + > +#define VKAPI_METADATA_PLANE 4 > + > +/** > + * @file > + * API-specific header for AV_HWDEVICE_TYPE_VKAPI. > + */ > + > +/** > + * Allocated as AVHWDeviceContext.hwctx > + */ > +typedef struct VKAPIDeviceContext { As this is an installed header, all structs and defines must have the AV prefix. > + /** > + * Holds pointers to hardware specific functions > + */ > + vkil_api *ilapi; > + /** > + * Internal functions definitions > + */ > + /** > + * Get the hwprops reference from the AVFrame:data[3] > + */ > + int (*frame_ref_hwprops)(const AVFrame *frame, void *phw_surface_desc); > + /** > + * Set the hwprops into AVFrame:data[3] > + */ > + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface *hw_surface_desc); > + /** > + * Get the hwprops from AVFrame:data[3] > + */ > + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface *hw_surface_desc); > + /** > + * Check if format is in an array > + */ > + int (*fmt_is_in)(int fmt, const int *fmts); > + /** > + * Convert AVPixelFormat to VKAPI equivalent pixel format > + */ > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); > + /** > + * Get no of buffer count reference in the hardware pool > + */ > + int (*get_pool_occupancy)(AVHWFramesContext *ctx); > +} VKAPIDeviceContext; > + > +/** > + * Contains color information for hardware > + */ > +typedef struct VKAPIColorContext { > + enum AVColorRange range; > + enum AVColorPrimaries primaries; > + enum AVColorTransferCharacteristic trc; > + enum AVColorSpace space; > + enum AVChromaLocation chroma_location; > +} VKAPIColorContext; > + > +/** > + * Allocated as AVHWFramesContext.hwctx > + */ > +typedef struct VKAPIFramesContext { > + /** > + * Handle to a hardware frame context > + */ > + uint32_t handle; > + /** > + * Hardware component port associated to the frame context > + */ > + uint32_t port_id; > + uint32_t extra_port_id; > + /** > + * Color information > + */ > + VKAPIColorContext color; > + /** > + * ilcontext associated to the frame context > + */ > + vkil_context *ilctx; > +} VKAPIFramesContext; > + > +#endif /* AVUTIL_HWCONTEXT_VKAPI_H */ > diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c > index 2a919461a5..1d2f242e57 100644 > --- a/libavutil/pixdesc.c > +++ b/libavutil/pixdesc.c > @@ -2391,6 +2391,10 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { > }, > .flags = AV_PIX_FMT_FLAG_PLANAR, > }, > + [AV_PIX_FMT_VKAPI] = { > + .name = "vkapi", > + .flags = AV_PIX_FMT_FLAG_HWACCEL, > + }, > [AV_PIX_FMT_VULKAN] = { > .name = "vulkan", > .flags = AV_PIX_FMT_FLAG_HWACCEL, > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > index 46ef211add..3ae607a3d6 100644 > --- a/libavutil/pixfmt.h > +++ b/libavutil/pixfmt.h > @@ -348,6 +348,12 @@ enum AVPixelFormat { > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V) > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > + /** > + * VKAPI hardware acceleration. > + * data[3] contains a pointer to the vkil_buffer_surface structure > + */ > + AV_PIX_FMT_VKAPI, New values must always be added at the end, right before AV_PIX_FMT_NB, to not break ABI. > + > /** > * Vulkan hardware images. > * >
Thank you for the review, please see inline. On Thu, Mar 11, 2021 at 2:56 PM Lynne <dev@lynne.ee> wrote: > Mar 11, 2021, 23:09 by suji.velupillai@broadcom.com: > > > From: Suji Velupillai <suji.velupillai@broadcom.com> > > > > Initial commit to add VKAPI hardware accelerator implementation. > > The depedency component vkil source code can be obtained from github > > https://github.com/Broadcom/vkil > > > > Is this code for hardware that no one will ever be able to get, > like the last time something like this was sent? > We've already had to turn down one such patch which added > support for hardware some company wanted but literally no > one else could get. > > The difference is this one is fully open-source as far as the code > goes, since the kernel driver's going to be in 5.12, which is enough > to call it good in my book, though it's still iffy. > > Yes, this hardware has fully open sourced drivers and utilities. The hardware is currently available for early access customers now. In future, it will be more widely available. > > +static int vkapi_transfer_get_formats(AVHWFramesContext *ctx, > > + enum AVHWFrameTransferDirection > dir, > > + enum AVPixelFormat **formats) > > +{ > > + enum AVPixelFormat *fmts; > > + int ret = 0; > > + > > + // this allocation freed in hwcontext::transfer_data_alloc > > + fmts = av_malloc_array(2, sizeof(*fmts)); > > + if (!fmts) { > > + av_log(ctx, AV_LOG_ERROR, "vkapi_transfer_get_formats > failed\n"); > > + ret = AVERROR(ENOMEM); > > + } else { > > + fmts[0] = ctx->sw_format; > > + fmts[1] = AV_PIX_FMT_NONE; > > + *formats = fmts; > > + } > > + > > + return ret; > > +} > > + > > +static int vkapi_convert_AV2VK_Frame(vkil_buffer_surface *dst, > > + const AVFrame *src) > > > > Why the all-caps on AV2VK? And a capital 'F'. Just make it all lowercase, > that is our code style. > Sorry I missed that, will fix it. > > > + // populate the vkil_surface structure with the origin pointer on > the host > > + ret = vkapi_convert_AV2VK_Frame(surface, src); > > + if (ret) { > > + ret = av_image_alloc(tmp_data, linesize, src->width, > src->height, > > + src->format, VKIL_BUF_ALIGN); > > + if (ret < 0) > > + goto fail; > > + > > + av_image_copy(tmp_data, linesize, (const uint8_t **)src->data, > > + src->linesize, src->format, src->width, > src->height); > > + > > + for (i = 0; i < VKIL_BUF_NPLANES; i++) { > > + surface->plane_top[i]= tmp_data[i]; > > + surface->plane_bot[i]= tmp_data[VKIL_BUF_NPLANES + i]; > > + surface->stride[i] = linesize[i]; > > + } > > + } > > + > > + surface->quality = dst->quality; > > + > > + ilctx = hw_framectx->ilctx; > > + if (!ilctx) { > > + ret = AVERROR(EINVAL); > > + goto fail; > > + } > > + > > + // blocking call as ffmpeg assumes the transfer is complete on > return > > > > No, it doesn't. Just ref the frame and unref it the next time a user > tries to upload a new frame, and if the previous upload hasn't > finished, block until it has. > That way you can have asynchronous uploading, but > the downloading has to be synchronous. I'm fine with this being > left as-is for now though. > Okay thank you. > > > > +/** > > + * Allocated as AVHWDeviceContext.hwctx > > + */ > > +typedef struct VKAPIDeviceContext { > > + /** > > + * Holds pointers to hardware specific functions > > + */ > > + vkil_api *ilapi; > > + /** > > + * Internal functions definitions > > + */ > > + /** > > + * Get the hwprops reference from the AVFrame:data[3] > > + */ > > + int (*frame_ref_hwprops)(const AVFrame *frame, void > *phw_surface_desc); > > > > Definitely doesn't need to be in this code. Just remove it and > duplicate it wherever it's used elsewhere. > > Okay, will do. > > > + /** > > + * Set the hwprops into AVFrame:data[3] > > + */ > > + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface > *hw_surface_desc); > > > > Same as above. Again, all fields are already public. > Users should do this themselves. > > > + /** > > + * Get the hwprops from AVFrame:data[3] > > + */ > > + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface > *hw_surface_desc); > > > > Same. > > > > + /** > > + * Check if format is in an array > > + */ > > + int (*fmt_is_in)(int fmt, const int *fmts); > > > > Same... > > Okay will remove the above functions. > > + /** > > + * Convert AVPixelFormat to VKAPI equivalent pixel format > > + */ > > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); > > > > This function can stay, but it needs no state at all, so just put > it out side of the structure. Name it: > const VkFormat *av_vkil_from_pixfmt(enum AVPixelFormat p); > The *vk* namespace is taken by Vulkan already. > Make sense, can I change it to vkapi instead to be consistent? > > > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > > index 46ef211add..3ae607a3d6 100644 > > --- a/libavutil/pixfmt.h > > +++ b/libavutil/pixfmt.h > > @@ -348,6 +348,12 @@ enum AVPixelFormat { > > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 > plane for the UV components, which are interleaved (first byte U and the > following byte V) > > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > > > + /** > > + * VKAPI hardware acceleration. > > + * data[3] contains a pointer to the vkil_buffer_surface structure > > > > Why [3]? Why not [0]? Is this some horrible hack to allow > for some extremely weird software frames with an additional > hardware frame? Or to simplify checking for whether a frame > is software or hardware, despite the fact the format field > already tells you exactly what it is, and planar YUVA frames > will break the check? > I think unless you have very good reasons, this should be [0]. > Actually there is no particular reason for it to be [3], looking through code initially, some using [3] such as VAAPI, QSV, DXVA2, etc. So we used the same, but now I see others using [0] such as DRM, VULKAN. I'll change it [0], which makes sense. Thank you > _______________________________________________ > 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".
Mar 12, 2021, 02:04 by suji.velupillai@broadcom.com: > Thank you for the review, please see inline. > > On Thu, Mar 11, 2021 at 2:56 PM Lynne <dev@lynne.ee> wrote: > >> > + /** >> > + * Convert AVPixelFormat to VKAPI equivalent pixel format >> > + */ >> > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); >> > >> >> This function can stay, but it needs no state at all, so just put >> it out side of the structure. Name it: >> const VkFormat *av_vkil_from_pixfmt(enum AVPixelFormat p); >> The *vk* namespace is taken by Vulkan already. >> > > Make sense, can I change it to vkapi instead to be consistent? > What about expanding "vk" shortenings to "valkyrie" instead? vkapi is kinda really close to meaning vulkan api -_-
On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: > Initial commit to add VKAPI hardware accelerator implementation. > The depedency component vkil source code can be obtained from github > https://github.com/Broadcom/vkil This has no license, no readme, no description. What is the hardware supported? Does it work for rPI?
> 2021年3月12日 下午5:12，Jean-Baptiste Kempf <jb@videolan.org> 写道： > > On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: >> Initial commit to add VKAPI hardware accelerator implementation. >> The depedency component vkil source code can be obtained from github >> https://github.com/Broadcom/vkil > > This has no license, no readme, no description. I saw the License in source code: https://github.com/Broadcom/vkil/blob/master/src/vk_buffers.h /* SPDX-License-Identifier: Apache-2.0 */ /* * Copyright 2018-2020 Broadcom. */ Apache-2.0 ? > > What is the hardware supported? > Does it work for rPI? > > -- > Jean-Baptiste Kempf - President > +33 672 704 734 > _______________________________________________ > 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". Thanks Steven Liu
On Fri, 12 Mar 2021, at 11:17, Steven Liu wrote: > > 2021年3月12日 下午5:12，Jean-Baptiste Kempf <jb@videolan.org> 写道： > > > > On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: > >> Initial commit to add VKAPI hardware accelerator implementation. > >> The depedency component vkil source code can be obtained from github > >> https://github.com/Broadcom/vkil > > > > This has no license, no readme, no description. > I saw the License in source code: License in a file is not the same as in the core of the project.
On Fri, 12 Mar 2021, at 11:28, Jean-Baptiste Kempf wrote: > On Fri, 12 Mar 2021, at 11:17, Steven Liu wrote: > > > 2021年3月12日 下午5:12，Jean-Baptiste Kempf <jb@videolan.org> 写道： > > > > > > On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: > > >> Initial commit to add VKAPI hardware accelerator implementation. > > >> The depedency component vkil source code can be obtained from github > > >> https://github.com/Broadcom/vkil > > > > > > This has no license, no readme, no description. > > I saw the License in source code: > > License in a file is not the same as in the core of the project. Not to mention that if Apache2 it is, this makes the code LGPLv3.
On Fri, Mar 12, 2021 at 1:12 AM Jean-Baptiste Kempf <jb@videolan.org> wrote: > On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: > > Initial commit to add VKAPI hardware accelerator implementation. > > The depedency component vkil source code can be obtained from github > > https://github.com/Broadcom/vkil > > This has no license, no readme, no description. > I'll have this addressed in the github. > > What is the hardware supported? > It's an M.2 module connected via PCIe that supports video transcoding. Some high level info can be found here https://engineering.fb.com/2019/03/14/data-center-engineering/accelerating-infrastructure/ Does it work for rPI? > No > > -- > Jean-Baptiste Kempf - President > +33 672 704 734 > _______________________________________________ > 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".
On 11/03/2021 22:09, suji.velupillai@broadcom.com wrote: > From: Suji Velupillai <suji.velupillai@broadcom.com> > > Initial commit to add VKAPI hardware accelerator implementation. > The depedency component vkil source code can be obtained from github > https://github.com/Broadcom/vkil > > Signed-off-by: Suji Velupillai <suji.velupillai@broadcom.com> > --- > configure | 8 +- > doc/APIchanges | 4 + > libavutil/Makefile | 2 + > libavutil/hwcontext.c | 4 + > libavutil/hwcontext.h | 1 + > libavutil/hwcontext_internal.h | 1 + > libavutil/hwcontext_vkapi.c | 522 +++++++++++++++++++++++++++++++++ > libavutil/hwcontext_vkapi.h | 104 +++++++ > libavutil/pixdesc.c | 4 + > libavutil/pixfmt.h | 6 + > 10 files changed, 655 insertions(+), 1 deletion(-) > create mode 100644 libavutil/hwcontext_vkapi.c > create mode 100644 libavutil/hwcontext_vkapi.h Where has the "vkapi" name come from? It seems to be consistently called "vkil" in that repository. If you are making up the name for this, please consider making it less confusing: * The standard Vulkan API already effectively owns the "vk" namespace prefix, and colliding with that in a related project is unhelpful. * Indeed, Vulkan already uses the "VKAPI" name in its headers when marking ABIs (see <https://github.com/ARM-software/vulkan-sdk/blob/master/include/vulkan/vk_platform.h#L35>). * Current search results for "vkapi" show it is also used by APIs for the VK social network. > ... > diff --git a/libavutil/hwcontext_vkapi.h b/libavutil/hwcontext_vkapi.h > new file mode 100644 > index 0000000000..096602b42e > --- /dev/null > +++ b/libavutil/hwcontext_vkapi.h > @@ -0,0 +1,104 @@ > +/* > + * Copyright (c) 2018 Broadcom > + * > + * 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 > + */ > + > +#ifndef AVUTIL_HWCONTEXT_VKAPI_H > +#define AVUTIL_HWCONTEXT_VKAPI_H > + > +#include <vkil_api.h> > + > +#define VKAPI_METADATA_PLANE 4 > + > +/** > + * @file > + * API-specific header for AV_HWDEVICE_TYPE_VKAPI. > + */ > + > +/** > + * Allocated as AVHWDeviceContext.hwctx > + */ > +typedef struct VKAPIDeviceContext { > + /** > + * Holds pointers to hardware specific functions > + */ > + vkil_api *ilapi; > + /** > + * Internal functions definitions > + */ > + /** > + * Get the hwprops reference from the AVFrame:data[3] > + */ > + int (*frame_ref_hwprops)(const AVFrame *frame, void *phw_surface_desc); > + /** > + * Set the hwprops into AVFrame:data[3] > + */ > + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface *hw_surface_desc); > + /** > + * Get the hwprops from AVFrame:data[3] > + */ > + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface *hw_surface_desc); > + /** > + * Check if format is in an array > + */ > + int (*fmt_is_in)(int fmt, const int *fmts); > + /** > + * Convert AVPixelFormat to VKAPI equivalent pixel format > + */ > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); > + /** > + * Get no of buffer count reference in the hardware pool > + */ > + int (*get_pool_occupancy)(AVHWFramesContext *ctx); > +} VKAPIDeviceContext; This structure has two uses: * To be filled by the user when they already have an instance of the device and want to use it with libav*. * To provide information to the user about an instance of the device created inside libav*. To that end, the "ilapi" field makes sense to me (the user provides their vkil API handle), but they also need to provide some sort of reference to the actual device (a vkil_context handle, maybe?). I don't think any of the other fields make sense; it looks like you are trying to expose some internals in a confusing way - why would a user creating this structure want to set those function pointers? > +/** > + * Contains color information for hardware > + */ > +typedef struct VKAPIColorContext { > + enum AVColorRange range; > + enum AVColorPrimaries primaries; > + enum AVColorTransferCharacteristic trc; > + enum AVColorSpace space; > + enum AVChromaLocation chroma_location; > +} VKAPIColorContext; > + > +/** > + * Allocated as AVHWFramesContext.hwctx > + */ > +typedef struct VKAPIFramesContext { > + /** > + * Handle to a hardware frame context > + */ > + uint32_t handle; > + /** > + * Hardware component port associated to the frame context > + */ > + uint32_t port_id; > + uint32_t extra_port_id; > + /** > + * Color information > + */ > + VKAPIColorContext color; > + /** > + * ilcontext associated to the frame context > + */ > + vkil_context *ilctx; > +} VKAPIFramesContext; > + > +#endif /* AVUTIL_HWCONTEXT_VKAPI_H */ The vkil_context looks like it should be part of the device. So sort of handle information for the context of the frame makes sense, ok. The port_id fields don't seem to be used at all in your implementation, which strongly suggests that they should not be here. The colour information you are including here is generally represented per-frame, so attaching it to a frame context seems dubious. > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > index 46ef211add..3ae607a3d6 100644 > --- a/libavutil/pixfmt.h > +++ b/libavutil/pixfmt.h > @@ -348,6 +348,12 @@ enum AVPixelFormat { > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V) > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > + /** > + * VKAPI hardware acceleration. > + * data[3] contains a pointer to the vkil_buffer_surface structure > + */ > + AV_PIX_FMT_VKAPI, As already noted, please use data[0] (data[3] is unhelpfully baked into some older formats for historical reasons, apologies for any confusion there). Also, you seem to have references to an additional field store in data[4], the meaning of which needs to be documented here as well. To understand what is going on with this hwcontext it would be very helpful to see some components using it (even in prototype form). Thanks, - Mark
On 12/03/2021 18:29, Suji Velupillai wrote: > On Fri, Mar 12, 2021 at 1:12 AM Jean-Baptiste Kempf <jb@videolan.org> wrote: > >> On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: >>> Initial commit to add VKAPI hardware accelerator implementation. >>> The depedency component vkil source code can be obtained from github >>> https://github.com/Broadcom/vkil >> >> This has no license, no readme, no description. >> > I'll have this addressed in the github. > > >> >> What is the hardware supported? >> > It's an M.2 module connected via PCIe that supports video transcoding. Some > high level info can be found here > https://engineering.fb.com/2019/03/14/data-center-engineering/accelerating-infrastructure/ > > Does it work for rPI? >> > No So it's some sort of custom hardware for Facebook servers, and presumably therefore of no use to anyone outside Facebook? Why is this being submitted to FFmpeg? - Mark
On Fri, Mar 12, 2021 at 2:06 PM Mark Thompson <sw@jkqxz.net> wrote: > On 12/03/2021 18:29, Suji Velupillai wrote: > > On Fri, Mar 12, 2021 at 1:12 AM Jean-Baptiste Kempf <jb@videolan.org> > wrote: > > > >> On Thu, 11 Mar 2021, at 23:09, suji.velupillai@broadcom.com wrote: > >>> Initial commit to add VKAPI hardware accelerator implementation. > >>> The depedency component vkil source code can be obtained from github > >>> https://github.com/Broadcom/vkil > >> > >> This has no license, no readme, no description. > >> > > I'll have this addressed in the github. > > > > > >> > >> What is the hardware supported? > >> > > It's an M.2 module connected via PCIe that supports video transcoding. > Some > > high level info can be found here > > > https://engineering.fb.com/2019/03/14/data-center-engineering/accelerating-infrastructure/ > > > > Does it work for rPI? > >> > > No > > So it's some sort of custom hardware for Facebook servers, and presumably > therefore of no use to anyone outside Facebook? Why is this being > submitted to FFmpeg? > > The servers are from Facebook's contribution to the Open Compute Project. See Facebook Contributions to Open Compute Project (OCP) <https://www.opencompute.org/contributions?refinementList%5Bcontributor%5D%5B0%5D=Facebook&refinementList%5Bproject%5D=&refinementList%5Bfamily%5D=&page=1&configure%5BfacetFilters%5D%5B0%5D=archived%3Afalse> . The Video Transcoding PCIe cards are standard M.2 form factor and can be used in standard Servers or Desktops that accept M.2 cards. Kernel driver for the hardware is already in kernel 5.11. > - Mark > _______________________________________________ > 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".
Thank you Mark for your feedback. Please see inline On Fri, Mar 12, 2021 at 1:14 PM Mark Thompson <sw@jkqxz.net> wrote: > On 11/03/2021 22:09, suji.velupillai@broadcom.com wrote: > > From: Suji Velupillai <suji.velupillai@broadcom.com> > > > > Initial commit to add VKAPI hardware accelerator implementation. > > The depedency component vkil source code can be obtained from github > > https://github.com/Broadcom/vkil > > > > Signed-off-by: Suji Velupillai <suji.velupillai@broadcom.com> > > --- > > configure | 8 +- > > doc/APIchanges | 4 + > > libavutil/Makefile | 2 + > > libavutil/hwcontext.c | 4 + > > libavutil/hwcontext.h | 1 + > > libavutil/hwcontext_internal.h | 1 + > > libavutil/hwcontext_vkapi.c | 522 +++++++++++++++++++++++++++++++++ > > libavutil/hwcontext_vkapi.h | 104 +++++++ > > libavutil/pixdesc.c | 4 + > > libavutil/pixfmt.h | 6 + > > 10 files changed, 655 insertions(+), 1 deletion(-) > > create mode 100644 libavutil/hwcontext_vkapi.c > > create mode 100644 libavutil/hwcontext_vkapi.h > > Where has the "vkapi" name come from? It seems to be consistently called > "vkil" in that repository. valkyrie is the hardware name. vkil refers to VK Interface Layer. vkapi is the name given to all ffmpeg API's. > If you are making up the name for this, please consider making it less > confusing: > * The standard Vulkan API already effectively owns the "vk" namespace > prefix, and colliding with that in a related project is unhelpful. > * Indeed, Vulkan already uses the "VKAPI" name in its headers when > marking ABIs (see < > https://github.com/ARM-software/vulkan-sdk/blob/master/include/vulkan/vk_platform.h#L35 > >). > * Current search results for "vkapi" show it is also used by APIs for the > VK social network. > Okay I will rename all with VKAPI and VK_* reference to avoid conflicts/confusion. Kernel driver code consistently used "bcm_vk", also Google search points correctly to the kernel driver for this card. Would it be okay to switch to bcm_vk instead of vkapi and vk? > > > ... > diff --git a/libavutil/hwcontext_vkapi.h > b/libavutil/hwcontext_vkapi.h > > new file mode 100644 > > index 0000000000..096602b42e > > --- /dev/null > > +++ b/libavutil/hwcontext_vkapi.h > > @@ -0,0 +1,104 @@ > > +/* > > + * Copyright (c) 2018 Broadcom > > + * > > + * 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 > > + */ > > + > > +#ifndef AVUTIL_HWCONTEXT_VKAPI_H > > +#define AVUTIL_HWCONTEXT_VKAPI_H > > + > > +#include <vkil_api.h> > > + > > +#define VKAPI_METADATA_PLANE 4 > > + > > +/** > > + * @file > > + * API-specific header for AV_HWDEVICE_TYPE_VKAPI. > > + */ > > + > > +/** > > + * Allocated as AVHWDeviceContext.hwctx > > + */ > > +typedef struct VKAPIDeviceContext { > > + /** > > + * Holds pointers to hardware specific functions > > + */ > > + vkil_api *ilapi; > > + /** > > + * Internal functions definitions > > + */ > > + /** > > + * Get the hwprops reference from the AVFrame:data[3] > > + */ > > + int (*frame_ref_hwprops)(const AVFrame *frame, void > *phw_surface_desc); > > + /** > > + * Set the hwprops into AVFrame:data[3] > > + */ > > + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface > *hw_surface_desc); > > + /** > > + * Get the hwprops from AVFrame:data[3] > > + */ > > + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface > *hw_surface_desc); > > + /** > > + * Check if format is in an array > > + */ > > + int (*fmt_is_in)(int fmt, const int *fmts); > > + /** > > + * Convert AVPixelFormat to VKAPI equivalent pixel format > > + */ > > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); > > + /** > > + * Get no of buffer count reference in the hardware pool > > + */ > > + int (*get_pool_occupancy)(AVHWFramesContext *ctx); > > +} VKAPIDeviceContext; > > This structure has two uses: > > * To be filled by the user when they already have an instance of the > device and want to use it with libav*. > * To provide information to the user about an instance of the device > created inside libav*. > > To that end, the "ilapi" field makes sense to me (the user provides their > vkil API handle), but they also need to provide some sort of reference to > the actual device (a vkil_context handle, maybe?). > > I don't think any of the other fields make sense; it looks like you are > trying to expose some internals in a confusing way - why would a user > creating this structure want to set those function pointers? > > Agreed, Lynne also pointed this out, so I removed it and moved it within the library that really needs the functions. > > +/** > > + * Contains color information for hardware > > + */ > > +typedef struct VKAPIColorContext { > > + enum AVColorRange range; > > + enum AVColorPrimaries primaries; > > + enum AVColorTransferCharacteristic trc; > > + enum AVColorSpace space; > > + enum AVChromaLocation chroma_location; > > +} VKAPIColorContext; > > + > > +/** > > + * Allocated as AVHWFramesContext.hwctx > > + */ > > +typedef struct VKAPIFramesContext { > > + /** > > + * Handle to a hardware frame context > > + */ > > + uint32_t handle; > > + /** > > + * Hardware component port associated to the frame context > > + */ > > + uint32_t port_id; > > + uint32_t extra_port_id; > > + /** > > + * Color information > > + */ > > + VKAPIColorContext color; > > + /** > > + * ilcontext associated to the frame context > > + */ > > + vkil_context *ilctx; > > +} VKAPIFramesContext; > > + > > +#endif /* AVUTIL_HWCONTEXT_VKAPI_H */ > > The vkil_context looks like it should be part of the device. > > So sort of handle information for the context of the frame makes sense, ok. > > The port_id fields don't seem to be used at all in your implementation, > which strongly suggests that they should not be here. > > The colour information you are including here is generally represented > per-frame, so attaching it to a frame context seems dubious. Both of those fields are used in the libavcodec, but it makes sense to be within the codecs struct itself. I'll review the code and change it. > > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > > index 46ef211add..3ae607a3d6 100644 > > --- a/libavutil/pixfmt.h > > +++ b/libavutil/pixfmt.h > > @@ -348,6 +348,12 @@ enum AVPixelFormat { > > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y > and 1 plane for the UV components, which are interleaved (first byte U and > the following byte V) > > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped > > > > + /** > > + * VKAPI hardware acceleration. > > + * data[3] contains a pointer to the vkil_buffer_surface structure > > + */ > > + AV_PIX_FMT_VKAPI, > > As already noted, please use data[0] (data[3] is unhelpfully baked into > some older formats for historical reasons, apologies for any confusion > there). > > No problem. Thank you for clarification, agreed, I'll change it to data[0]. Also, you seem to have references to an additional field store in data[4], > the meaning of which needs to be documented here as well. > Okay > > > To understand what is going on with this hwcontext it would be very > helpful to see some components using it (even in prototype form). > We have working encoder/decoder and scaler functionalities for this hardware in ffmpeg framework, which I will be sending it for review. I thought to get the hwcontext in first for feedback. It needs some clean up and changes in code based on this patch review also. I will do that as soon as possible. > > Thanks, > > - Mark > _______________________________________________ > 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".
On Wed, 17 Mar 2021 at 00:14, Suji Velupillai <suji.velupillai@broadcom.com> wrote: > Thank you Mark for your feedback. Please see inline > > On Fri, Mar 12, 2021 at 1:14 PM Mark Thompson <sw@jkqxz.net> wrote: > > > On 11/03/2021 22:09, suji.velupillai@broadcom.com wrote: > > > From: Suji Velupillai <suji.velupillai@broadcom.com> > > > > > > Initial commit to add VKAPI hardware accelerator implementation. > > > The depedency component vkil source code can be obtained from github > > > https://github.com/Broadcom/vkil > > > > > > Signed-off-by: Suji Velupillai <suji.velupillai@broadcom.com> > > > --- > > > configure | 8 +- > > > doc/APIchanges | 4 + > > > libavutil/Makefile | 2 + > > > libavutil/hwcontext.c | 4 + > > > libavutil/hwcontext.h | 1 + > > > libavutil/hwcontext_internal.h | 1 + > > > libavutil/hwcontext_vkapi.c | 522 > +++++++++++++++++++++++++++++++++ > > > libavutil/hwcontext_vkapi.h | 104 +++++++ > > > libavutil/pixdesc.c | 4 + > > > libavutil/pixfmt.h | 6 + > > > 10 files changed, 655 insertions(+), 1 deletion(-) > > > create mode 100644 libavutil/hwcontext_vkapi.c > > > create mode 100644 libavutil/hwcontext_vkapi.h > > > > Where has the "vkapi" name come from? It seems to be consistently called > > "vkil" in that repository. > > valkyrie is the hardware name. vkil refers to VK Interface Layer. vkapi is > the name given to all ffmpeg API's. > > > > > If you are making up the name for this, please consider making it less > > confusing: > > * The standard Vulkan API already effectively owns the "vk" namespace > > prefix, and colliding with that in a related project is unhelpful. > > * Indeed, Vulkan already uses the "VKAPI" name in its headers when > > marking ABIs (see < > > > https://github.com/ARM-software/vulkan-sdk/blob/master/include/vulkan/vk_platform.h#L35 > > >). > > * Current search results for "vkapi" show it is also used by APIs for the > > VK social network. > > > > Okay I will rename all with VKAPI and VK_* reference to avoid > conflicts/confusion. > Kernel driver code consistently used "bcm_vk", also Google search points > correctly to the kernel driver for this card. > Would it be okay to switch to bcm_vk instead of vkapi and vk? > > > > > > > ... > diff --git a/libavutil/hwcontext_vkapi.h > > b/libavutil/hwcontext_vkapi.h > > > new file mode 100644 > > > index 0000000000..096602b42e > > > --- /dev/null > > > +++ b/libavutil/hwcontext_vkapi.h > > > @@ -0,0 +1,104 @@ > > > +/* > > > + * Copyright (c) 2018 Broadcom > > > + * > > > + * 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 > > > + */ > > > + > > > +#ifndef AVUTIL_HWCONTEXT_VKAPI_H > > > +#define AVUTIL_HWCONTEXT_VKAPI_H > > > + > > > +#include <vkil_api.h> > > > + > > > +#define VKAPI_METADATA_PLANE 4 > > > + > > > +/** > > > + * @file > > > + * API-specific header for AV_HWDEVICE_TYPE_VKAPI. > > > + */ > > > + > > > +/** > > > + * Allocated as AVHWDeviceContext.hwctx > > > + */ > > > +typedef struct VKAPIDeviceContext { > > > + /** > > > + * Holds pointers to hardware specific functions > > > + */ > > > + vkil_api *ilapi; > > > + /** > > > + * Internal functions definitions > > > + */ > > > + /** > > > + * Get the hwprops reference from the AVFrame:data[3] > > > + */ > > > + int (*frame_ref_hwprops)(const AVFrame *frame, void > > *phw_surface_desc); > > > + /** > > > + * Set the hwprops into AVFrame:data[3] > > > + */ > > > + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface > > *hw_surface_desc); > > > + /** > > > + * Get the hwprops from AVFrame:data[3] > > > + */ > > > + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface > > *hw_surface_desc); > > > + /** > > > + * Check if format is in an array > > > + */ > > > + int (*fmt_is_in)(int fmt, const int *fmts); > > > + /** > > > + * Convert AVPixelFormat to VKAPI equivalent pixel format > > > + */ > > > + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); > > > + /** > > > + * Get no of buffer count reference in the hardware pool > > > + */ > > > + int (*get_pool_occupancy)(AVHWFramesContext *ctx); > > > +} VKAPIDeviceContext; > > > > This structure has two uses: > > > > * To be filled by the user when they already have an instance of the > > device and want to use it with libav*. > > * To provide information to the user about an instance of the device > > created inside libav*. > > > > To that end, the "ilapi" field makes sense to me (the user provides their > > vkil API handle), but they also need to provide some sort of reference to > > the actual device (a vkil_context handle, maybe?). > > > > I don't think any of the other fields make sense; it looks like you are > > trying to expose some internals in a confusing way - why would a user > > creating this structure want to set those function pointers? > > > > Agreed, Lynne also pointed this out, so I removed it and moved it within > the library that really needs the functions. > > > > > +/** > > > + * Contains color information for hardware > > > + */ > > > +typedef struct VKAPIColorContext { > > > + enum AVColorRange range; > > > + enum AVColorPrimaries primaries; > > > + enum AVColorTransferCharacteristic trc; > > > + enum AVColorSpace space; > > > + enum AVChromaLocation chroma_location; > > > +} VKAPIColorContext; > > > + > > > +/** > > > + * Allocated as AVHWFramesContext.hwctx > > > + */ > > > +typedef struct VKAPIFramesContext { > > > + /** > > > + * Handle to a hardware frame context > > > + */ > > > + uint32_t handle; > > > + /** > > > + * Hardware component port associated to the frame context > > > + */ > > > + uint32_t port_id; > > > + uint32_t extra_port_id; > > > + /** > > > + * Color information > > > + */ > > > + VKAPIColorContext color; > > > + /** > > > + * ilcontext associated to the frame context > > > + */ > > > + vkil_context *ilctx; > > > +} VKAPIFramesContext; > > > + > > > +#endif /* AVUTIL_HWCONTEXT_VKAPI_H */ > > > > The vkil_context looks like it should be part of the device. > > > > So sort of handle information for the context of the frame makes sense, > ok. > > > > The port_id fields don't seem to be used at all in your implementation, > > which strongly suggests that they should not be here. > > > > The colour information you are including here is generally represented > > per-frame, so attaching it to a frame context seems dubious. > > > Both of those fields are used in the libavcodec, but it makes sense to be > within the codecs struct itself. I'll review the code and change it. > > > > > diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h > > > index 46ef211add..3ae607a3d6 100644 > > > --- a/libavutil/pixfmt.h > > > +++ b/libavutil/pixfmt.h > > > @@ -348,6 +348,12 @@ enum AVPixelFormat { > > > AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y > > and 1 plane for the UV components, which are interleaved (first byte U > and > > the following byte V) > > > AV_PIX_FMT_NV42, ///< as above, but U and V bytes are > swapped > > > > > > + /** > > > + * VKAPI hardware acceleration. > > > + * data[3] contains a pointer to the vkil_buffer_surface structure > > > + */ > > > + AV_PIX_FMT_VKAPI, > > > > As already noted, please use data[0] (data[3] is unhelpfully baked into > > some older formats for historical reasons, apologies for any confusion > > there). > > > > No problem. Thank you for clarification, agreed, I'll change it to > data[0]. > > Also, you seem to have references to an additional field store in data[4], > > the meaning of which needs to be documented here as well. > > > Okay > > > > > > > To understand what is going on with this hwcontext it would be very > > helpful to see some components using it (even in prototype form). > > > > We have working encoder/decoder and scaler functionalities for this > hardware in ffmpeg framework, which I will be sending it for review. I > thought to get the hwcontext in first for feedback. It needs some clean up > and changes in code based on this patch review also. I will do that as soon > as possible. > > > > > > Thanks, > > > > - Mark > > Hello there, Any updates on this? Warm regards, Dennis.
diff --git a/configure b/configure index d11942fced..73eb4539ee 100755 --- a/configure +++ b/configure @@ -348,6 +348,7 @@ External library support: --disable-vaapi disable Video Acceleration API (mainly Unix/Intel) code [autodetect] --disable-vdpau disable Nvidia Video Decode and Presentation API for Unix code [autodetect] --disable-videotoolbox disable VideoToolbox code [autodetect] + --disable-vkapi disable VKAPI code [autodetect] Toolchain options: --arch=ARCH select architecture [$arch] @@ -1844,6 +1845,7 @@ HWACCEL_AUTODETECT_LIBRARY_LIST=" vaapi vdpau videotoolbox + vkapi v4l2_m2m xvmc " @@ -2919,6 +2921,7 @@ nvdec_deps="ffnvcodec" vaapi_x11_deps="xlib" videotoolbox_hwaccel_deps="videotoolbox pthreads" videotoolbox_hwaccel_extralibs="-framework QuartzCore" +vkapi_deps="libvkil" xvmc_deps="X11_extensions_XvMClib_h" av1_d3d11va_hwaccel_deps="d3d11va DXVA_PicParams_AV1" @@ -3708,7 +3711,7 @@ avformat_deps="avcodec avutil" avformat_suggest="libm network zlib" avresample_deps="avutil" avresample_suggest="libm" -avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan videotoolbox corefoundation corevideo coremedia bcrypt" +avutil_suggest="clock_gettime ffnvcodec libm libdrm libmfx opencl user32 vaapi vulkan vkapi videotoolbox corefoundation corevideo coremedia bcrypt" postproc_deps="avutil gpl" postproc_suggest="libm" swresample_deps="avutil" @@ -6734,6 +6737,9 @@ enabled vdpau && enabled vdpau && check_lib vdpau_x11 "vdpau/vdpau.h vdpau/vdpau_x11.h" vdp_device_create_x11 -lvdpau -lX11 +enabled vkapi && + check_lib libvkil vkil_api.h vkil_create_api -lvkil + enabled crystalhd && check_lib crystalhd "stdint.h libcrystalhd/libcrystalhd_if.h" DtsCrystalHDVersion -lcrystalhd enabled vulkan && diff --git a/doc/APIchanges b/doc/APIchanges index 13350c0db0..ccab2e6465 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,10 @@ libavutil: 2017-10-21 API changes, most recent first: +2021-03-11 - xxxxxxxxxx - lavu yy.yy.yyy - hwcontext.h + Add AV_PIX_FMT_VKAPI + Add AV_HWDEVICE_TYPE_VKAPI and implementation. + 2021-03-10 - xxxxxxxxxx - lavf 58.72.100 - avformat.h Change AVBufferRef related AVStream function and struct size parameter and fields type to size_t at next major bump. diff --git a/libavutil/Makefile b/libavutil/Makefile index 27bafe9e12..4044b133a3 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -45,6 +45,7 @@ HEADERS = adler32.h \ hwcontext_vaapi.h \ hwcontext_videotoolbox.h \ hwcontext_vdpau.h \ + hwcontext_vkapi.h \ hwcontext_vulkan.h \ imgutils.h \ intfloat.h \ @@ -185,6 +186,7 @@ OBJS-$(CONFIG_QSV) += hwcontext_qsv.o OBJS-$(CONFIG_VAAPI) += hwcontext_vaapi.o OBJS-$(CONFIG_VIDEOTOOLBOX) += hwcontext_videotoolbox.o OBJS-$(CONFIG_VDPAU) += hwcontext_vdpau.o +OBJS-$(CONFIG_VKAPI) += hwcontext_vkapi.o OBJS-$(CONFIG_VULKAN) += hwcontext_vulkan.o OBJS += $(COMPAT_OBJS:%=../compat/%) diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c index d13d0f7c9b..4c13db6578 100644 --- a/libavutil/hwcontext.c +++ b/libavutil/hwcontext.c @@ -59,6 +59,9 @@ static const HWContextType * const hw_table[] = { #if CONFIG_MEDIACODEC &ff_hwcontext_type_mediacodec, #endif +#if CONFIG_VKAPI + &ff_hwcontext_type_vkapi, +#endif #if CONFIG_VULKAN &ff_hwcontext_type_vulkan, #endif @@ -76,6 +79,7 @@ static const char *const hw_type_names[] = { [AV_HWDEVICE_TYPE_VDPAU] = "vdpau", [AV_HWDEVICE_TYPE_VIDEOTOOLBOX] = "videotoolbox", [AV_HWDEVICE_TYPE_MEDIACODEC] = "mediacodec", + [AV_HWDEVICE_TYPE_VKAPI] = "vkapi", [AV_HWDEVICE_TYPE_VULKAN] = "vulkan", }; diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h index 04d19d89c2..b862f45aa3 100644 --- a/libavutil/hwcontext.h +++ b/libavutil/hwcontext.h @@ -36,6 +36,7 @@ enum AVHWDeviceType { AV_HWDEVICE_TYPE_DRM, AV_HWDEVICE_TYPE_OPENCL, AV_HWDEVICE_TYPE_MEDIACODEC, + AV_HWDEVICE_TYPE_VKAPI, AV_HWDEVICE_TYPE_VULKAN, }; diff --git a/libavutil/hwcontext_internal.h b/libavutil/hwcontext_internal.h index e6266494ac..c5270759a4 100644 --- a/libavutil/hwcontext_internal.h +++ b/libavutil/hwcontext_internal.h @@ -173,6 +173,7 @@ extern const HWContextType ff_hwcontext_type_vaapi; extern const HWContextType ff_hwcontext_type_vdpau; extern const HWContextType ff_hwcontext_type_videotoolbox; extern const HWContextType ff_hwcontext_type_mediacodec; +extern const HWContextType ff_hwcontext_type_vkapi; extern const HWContextType ff_hwcontext_type_vulkan; #endif /* AVUTIL_HWCONTEXT_INTERNAL_H */ diff --git a/libavutil/hwcontext_vkapi.c b/libavutil/hwcontext_vkapi.c new file mode 100644 index 0000000000..089b0a47d6 --- /dev/null +++ b/libavutil/hwcontext_vkapi.c @@ -0,0 +1,522 @@ +/* + * Copyright (c) 2018 Broadcom + * + * 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 "avassert.h" +#include "buffer.h" +#include "buffer_internal.h" +#include "common.h" +#include "hwcontext.h" +#include "hwcontext_internal.h" +#include "hwcontext_vkapi.h" +#include "imgutils.h" +#include "mem.h" +#include "pixdesc.h" +#include "pixfmt.h" + +#define VKAPI_FRAME_ALIGNMENT 256 + +static const enum AVPixelFormat supported_formats[] = { + AV_PIX_FMT_NV12, + AV_PIX_FMT_NV21, + AV_PIX_FMT_P010, + AV_PIX_FMT_P016, + AV_PIX_FMT_VKAPI, + AV_PIX_FMT_NONE, +}; + +/** + * Check if the format fmt is in an array + * + * @param fmt the format to check if exists + * @param fmts the array to check the format + * @return 1 if format is presnet in an array, 0 otherwise + */ +static int vkapi_fmt_is_in(int fmt, const int *fmts) +{ + const int *p; + + for (p = fmts; *p != -1; p++) { + if (fmt == *p) + return 1; + } + + return 0; +} + +/** + * Convert the pixel format from the AVPixelFormat to a vk_format_type + * equivalent. + * + * @param pixel_format an AVPixelFormat + * @return a vkil_format_type if match found, otherwise return an error code + */ +static int vkapi_av2vk_fmt(enum AVPixelFormat pixel_format) +{ + switch (pixel_format) { + case AV_PIX_FMT_NV12: + return VK_FORMAT_NV12; + case AV_PIX_FMT_NV21: + return VK_FORMAT_NV21; + case AV_PIX_FMT_P010: + return VK_FORMAT_P010; + case AV_PIX_FMT_VKAPI: + return VK_FORMAT_YOL2; + default: + return AVERROR(EINVAL); + } +} + +/** + * Provide the number of buffer currently referenced in the hardware pool + * + * @param ctx the hardware frames context + * @return number of buffer in use in the internal pool + */ +static int vkapi_get_pool_occupancy(AVHWFramesContext *ctx) +{ + int val = atomic_load(&ctx->internal->pool_internal->refcount); + + return (val - 1); +} + +/** + * Get the reference on the frame hwprops + * + * @param frame the ffmpeg AVFrame structure + * @param phw_surface_desc vkil buffer surface associated with the AVFrame + * @return 0 if succesfull, otherwise return an error code + */ +static int vkapi_frame_ref_hwprops(const AVFrame *frame, void *phw_surface_desc) +{ + void **hw_surface_desc = phw_surface_desc; + + if (frame->format != AV_PIX_FMT_VKAPI) + return AVERROR_BUG; + + *hw_surface_desc = frame->data[3]; + + if (!(*hw_surface_desc)) + return AVERROR_BUG; + + return 0; +} + +/** + * Set a ffmpeg AVFrame with the vkil buffer surface properties + * + * @param frame the ffmpeg AVFrame structure + * @param hw_surface_desc the vkil buffer surface to be associated with the AVFrame + * @return 0 if succesfull, otherwise return an error code + */ +static int vkapi_frame_set_hwprops(AVFrame *frame, + const vkil_buffer_surface *hw_surface_desc) +{ + if (frame->format != AV_PIX_FMT_VKAPI) + return AVERROR_BUG; + + // vkil buffer surface description is stored in ffmpeg AVframe:data[3] + av_assert0(frame->data[3]); + + // the "hard copy" only the buffer descriptor + memcpy((vkil_buffer_surface *)frame->data[3], hw_surface_desc, + sizeof(vkil_buffer_surface)); + + return 0; +} + +/** + * Retrieve vkil buffer surface properties associated with an ffmpeg AVFrame + * + * @param frame the ffmpeg AVFrame structure + * @param hw_surface_desc a vkil buffer surface, where where to extract the property + * @return 0 if succesfull, otherwise return an error code + */ +static int vkapi_frame_get_hwprops(const AVFrame *frame, + vkil_buffer_surface *hw_surface_desc) +{ + if (frame->format != AV_PIX_FMT_VKAPI) + return AVERROR_BUG; + + // the frame is hw tunneled (hw format) + // vkil buffer surface description is stored in ffmpeg AVframe:data[3] + av_assert0(frame->data[3]); + + // the "hard copy" only the buffer descriptor + memcpy(hw_surface_desc, (vkil_buffer_surface *)frame->data[3], + sizeof(vkil_buffer_surface)); + + return 0; +} + +static void vkapi_pool_release(void *opaque, uint8_t *data) +{ + av_free(data); +} + +static AVBufferRef *vkapi_pool_alloc(void *opaque, int size) +{ + AVHWFramesContext *ctx = opaque; + AVBufferRef *ret_ptr = NULL; + vkil_buffer_surface *hw_surface_desc; + + hw_surface_desc = av_mallocz(sizeof(vkil_buffer_surface)); + if (!hw_surface_desc) { + av_log(hw_surface_desc, AV_LOG_ERROR, "av_mallocz failed\n"); + goto out; + } + + ret_ptr = av_buffer_create((uint8_t *)hw_surface_desc, + sizeof(*hw_surface_desc), vkapi_pool_release, + NULL, AV_BUFFER_FLAG_READONLY); + if (!ret_ptr) { + av_log(ctx, AV_LOG_ERROR, "av_buffer_create failed\n"); + av_free(hw_surface_desc); + } + +out: + return ret_ptr; +} + +static int vkapi_frames_init(AVHWFramesContext *ctx) +{ + int aligned_width = FFALIGN(ctx->width, VKAPI_FRAME_ALIGNMENT); + int ret = AVERROR(EINVAL); + int valid_format = vkapi_fmt_is_in(ctx->sw_format, supported_formats); + VKAPIFramesContext *vk_framectx = ctx->hwctx; + + if (!valid_format) + goto fail; + + // set default VKAPIFramesContext + vk_framectx->color.range = AVCOL_RANGE_UNSPECIFIED; + vk_framectx->color.primaries = AVCOL_PRI_UNSPECIFIED; + vk_framectx->color.trc = AVCOL_TRC_UNSPECIFIED; + vk_framectx->color.space = AVCOL_SPC_UNSPECIFIED; + + if (!ctx->pool) { + int size; + + switch (ctx->sw_format) { + case AV_PIX_FMT_VKAPI: + size = sizeof(void *); + break; + case AV_PIX_FMT_NV12: + case AV_PIX_FMT_NV21: + size = (aligned_width * ctx->height * 3) >> 1; + break; + case AV_PIX_FMT_P010: + case AV_PIX_FMT_P016: + size = aligned_width * ctx->height * 3; + break; + default: + goto fail; + } + + ctx->internal->pool_internal = av_buffer_pool_init2(size, ctx, vkapi_pool_alloc, NULL); + if (!ctx->internal->pool_internal) { + ret = AVERROR(ENOMEM); + goto fail; + } + } + + return 0; + +fail: + av_log(ctx, AV_LOG_ERROR, "vkapi_frames_init failed on error %d\n", ret); + return ret; + +} + +static int vkapi_get_buffer(AVHWFramesContext *ctx, AVFrame *frame) +{ + int ret = 0; + + frame->buf[0] = av_buffer_pool_get(ctx->pool); + if (!frame->buf[0]) { + av_log(ctx, AV_LOG_ERROR, "av_buffer_pool_get failed\n"); + ret = AVERROR(ENOMEM); + } else { + // vkil data are stored in frame->data[3] + frame->data[3] = frame->buf[0]->data; + frame->format = AV_PIX_FMT_VKAPI; + frame->width = ctx->width; + frame->height = ctx->height; + } + + return ret; +} + +static int vkapi_transfer_get_formats(AVHWFramesContext *ctx, + enum AVHWFrameTransferDirection dir, + enum AVPixelFormat **formats) +{ + enum AVPixelFormat *fmts; + int ret = 0; + + // this allocation freed in hwcontext::transfer_data_alloc + fmts = av_malloc_array(2, sizeof(*fmts)); + if (!fmts) { + av_log(ctx, AV_LOG_ERROR, "vkapi_transfer_get_formats failed\n"); + ret = AVERROR(ENOMEM); + } else { + fmts[0] = ctx->sw_format; + fmts[1] = AV_PIX_FMT_NONE; + *formats = fmts; + } + + return ret; +} + +static int vkapi_convert_AV2VK_Frame(vkil_buffer_surface *dst, + const AVFrame *src) +{ + int i; + + av_assert0(src->interlaced_frame == 0); + + dst->max_size.width = src->width; + dst->max_size.height = src->height; + dst->format = vkapi_av2vk_fmt(src->format); + dst->quality = src->quality; + dst->prefix.type = VKIL_BUF_SURFACE; + dst->prefix.port_id = 0; // set to default (first port) + dst->prefix.flags = 0; // other fields set to zero (default value) + + for (i = 0; i < VKIL_BUF_NPLANES; i++) { + dst->plane_top[i] = src->data[i]; + dst->plane_bot[i] = src->data[i + VKIL_BUF_NPLANES]; + dst->stride[i] = src->linesize[i]; + // sanity check: vk structure requires 32 bits alignment + if (((uintptr_t)dst->plane_top[i] & (VKIL_BUF_ALIGN -1)) || + ((uintptr_t)dst->plane_bot[i] & (VKIL_BUF_ALIGN -1)) || + (dst->stride[i] &(VKIL_BUF_ALIGN -1))) + return AVERROR(EINVAL); + } + + return 0; +} + +static int vkapi_transfer_data_from(AVHWFramesContext *ctx, AVFrame *dst, + const AVFrame *src) +{ + int ret; + int32_t size; + VKAPIFramesContext *hw_framectx; + AVHWFramesContext *avhw_framectx; + VKAPIDeviceContext *hw_devicectx; + vkil_buffer_surface *surface; + vkil_buffer metadata; + + hw_devicectx = ctx->device_ctx->hwctx; + + av_assert0(src->hw_frames_ctx->data); + avhw_framectx = (AVHWFramesContext *)src->hw_frames_ctx->data; + + av_assert0(avhw_framectx->hwctx); + hw_framectx = avhw_framectx->hwctx; + vkapi_frame_ref_hwprops(src, &surface); + + // populate the vkil_surface structure with the destination pointer on the host + vkapi_convert_AV2VK_Frame(surface, dst); + + // blocking call as ffmpeg assume the transfer is complete on return + ret = hw_devicectx->ilapi->transfer_buffer2(hw_framectx->ilctx, surface, + VK_CMD_DOWNLOAD | VK_CMD_OPT_BLOCKING, + &size); + if (ret < 0) + goto fail; + + + if (src->data[VKAPI_METADATA_PLANE]) { + // Dereference if any transferred frame that has an associated metadata buffer + // no need to transfer it back to host + metadata.handle = (uint32_t)src->data[VKAPI_METADATA_PLANE]; + metadata.type = VKIL_BUF_META_DATA; + metadata.ref = 1; + ret = hw_devicectx->ilapi->xref_buffer(hw_framectx->ilctx, &metadata, -1, + VK_CMD_RUN | VK_CMD_OPT_BLOCKING); + if (ret) + goto fail; + } + + return 0; + +fail: + av_log(ctx, AV_LOG_ERROR, "failure %d on vkapi_transfer_data_to\n", ret); + return AVERROR(EINVAL); +} + +static int vkapi_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst, + const AVFrame *src) +{ + int ret; + int32_t i, size = 0; + VKAPIFramesContext *hw_framectx; + AVHWFramesContext *avhw_framectx; + VKAPIDeviceContext *hw_devicectx; + vkil_buffer_surface *surface; + vkil_context *ilctx; + uint8_t *tmp_data[4] = {NULL, NULL, NULL, NULL}; + int linesize[4]; + + av_assert0(VKIL_BUF_NPLANES * 2 <= 4); + + hw_devicectx = ctx->device_ctx->hwctx; + + av_assert0(dst->hw_frames_ctx->data); + avhw_framectx = (AVHWFramesContext *)dst->hw_frames_ctx->data; + + av_assert0(avhw_framectx->hwctx); + hw_framectx = avhw_framectx->hwctx; + + ret = vkapi_frame_ref_hwprops(dst, &surface); + if (ret < 0) { + ret = AVERROR(EINVAL); + goto fail; + } + + // populate the vkil_surface structure with the origin pointer on the host + ret = vkapi_convert_AV2VK_Frame(surface, src); + if (ret) { + ret = av_image_alloc(tmp_data, linesize, src->width, src->height, + src->format, VKIL_BUF_ALIGN); + if (ret < 0) + goto fail; + + av_image_copy(tmp_data, linesize, (const uint8_t **)src->data, + src->linesize, src->format, src->width, src->height); + + for (i = 0; i < VKIL_BUF_NPLANES; i++) { + surface->plane_top[i]= tmp_data[i]; + surface->plane_bot[i]= tmp_data[VKIL_BUF_NPLANES + i]; + surface->stride[i] = linesize[i]; + } + } + + surface->quality = dst->quality; + + ilctx = hw_framectx->ilctx; + if (!ilctx) { + ret = AVERROR(EINVAL); + goto fail; + } + + // blocking call as ffmpeg assumes the transfer is complete on return + ret = hw_devicectx->ilapi->transfer_buffer2(ilctx, surface, + VK_CMD_UPLOAD | VK_CMD_OPT_BLOCKING, + &size); + if (ret < 0) { + ret = AVERROR(EINVAL); + goto fail; + } + + hw_framectx->handle = surface->prefix.handle; + + if (tmp_data[0]) + av_free(tmp_data[0]); + + return 0; + +fail: + if (tmp_data[0]) + av_free(tmp_data[0]); + + av_log(ctx, AV_LOG_ERROR, "failure %d on vkapi_transfer_data_to\n", ret); + return ret; +} + +static void vkapi_device_uninit(AVHWDeviceContext *ctx) +{ + VKAPIDeviceContext *hwctx; + + av_assert0(ctx); + av_assert0(ctx->hwctx); + + hwctx = ctx->hwctx; + + if (hwctx->ilapi) + vkil_destroy_api((void **)&hwctx->ilapi); +} + +static int vkapi_device_init(AVHWDeviceContext *ctx) +{ + VKAPIDeviceContext *hwctx; + int ret = AVERROR_EXTERNAL; + + av_assert0(ctx); + av_assert0(ctx->hwctx); + + hwctx = ctx->hwctx; + hwctx->frame_set_hwprops = vkapi_frame_set_hwprops; + hwctx->frame_get_hwprops = vkapi_frame_get_hwprops; + hwctx->frame_ref_hwprops = vkapi_frame_ref_hwprops; + hwctx->fmt_is_in = vkapi_fmt_is_in; + hwctx->av2vk_fmt = vkapi_av2vk_fmt; + hwctx->get_pool_occupancy = vkapi_get_pool_occupancy; + + if (!(hwctx->ilapi = vkil_create_api())) { + av_log(ctx, AV_LOG_ERROR, "ctx->ilapi failed to be created\n"); + goto out; + } + + if (!hwctx->ilapi->init) { + av_log(ctx, AV_LOG_ERROR, "ctx->ilapi not properly initialized\n"); + goto out; + } + + ret = 0; + +out: + if (ret) + vkapi_device_uninit(ctx); + + return ret; +} + +static int vkapi_device_create(AVHWDeviceContext *ctx, const char *device, + AVDictionary *opts, int flags) +{ + /* nothing to do at this time */ + return 0; +} + +const HWContextType ff_hwcontext_type_vkapi = { + .type = AV_HWDEVICE_TYPE_VKAPI, + .name = "VKAPI", + + .device_hwctx_size = sizeof(VKAPIDeviceContext), + .frames_hwctx_size = sizeof(VKAPIFramesContext), + + .device_create = vkapi_device_create, + .device_init = vkapi_device_init, + .device_uninit = vkapi_device_uninit, + + .frames_init = vkapi_frames_init, + .frames_get_buffer = vkapi_get_buffer, + + .transfer_get_formats = vkapi_transfer_get_formats, + .transfer_data_to = vkapi_transfer_data_to, + .transfer_data_from = vkapi_transfer_data_from, + + .pix_fmts = (const enum AVPixelFormat[]) { + AV_PIX_FMT_VKAPI, + AV_PIX_FMT_NONE + }, +}; diff --git a/libavutil/hwcontext_vkapi.h b/libavutil/hwcontext_vkapi.h new file mode 100644 index 0000000000..096602b42e --- /dev/null +++ b/libavutil/hwcontext_vkapi.h @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2018 Broadcom + * + * 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 + */ + +#ifndef AVUTIL_HWCONTEXT_VKAPI_H +#define AVUTIL_HWCONTEXT_VKAPI_H + +#include <vkil_api.h> + +#define VKAPI_METADATA_PLANE 4 + +/** + * @file + * API-specific header for AV_HWDEVICE_TYPE_VKAPI. + */ + +/** + * Allocated as AVHWDeviceContext.hwctx + */ +typedef struct VKAPIDeviceContext { + /** + * Holds pointers to hardware specific functions + */ + vkil_api *ilapi; + /** + * Internal functions definitions + */ + /** + * Get the hwprops reference from the AVFrame:data[3] + */ + int (*frame_ref_hwprops)(const AVFrame *frame, void *phw_surface_desc); + /** + * Set the hwprops into AVFrame:data[3] + */ + int (*frame_set_hwprops)(AVFrame *frame, const vkil_buffer_surface *hw_surface_desc); + /** + * Get the hwprops from AVFrame:data[3] + */ + int (*frame_get_hwprops)(const AVFrame *frame, vkil_buffer_surface *hw_surface_desc); + /** + * Check if format is in an array + */ + int (*fmt_is_in)(int fmt, const int *fmts); + /** + * Convert AVPixelFormat to VKAPI equivalent pixel format + */ + int (*av2vk_fmt)(enum AVPixelFormat pixel_format); + /** + * Get no of buffer count reference in the hardware pool + */ + int (*get_pool_occupancy)(AVHWFramesContext *ctx); +} VKAPIDeviceContext; + +/** + * Contains color information for hardware + */ +typedef struct VKAPIColorContext { + enum AVColorRange range; + enum AVColorPrimaries primaries; + enum AVColorTransferCharacteristic trc; + enum AVColorSpace space; + enum AVChromaLocation chroma_location; +} VKAPIColorContext; + +/** + * Allocated as AVHWFramesContext.hwctx + */ +typedef struct VKAPIFramesContext { + /** + * Handle to a hardware frame context + */ + uint32_t handle; + /** + * Hardware component port associated to the frame context + */ + uint32_t port_id; + uint32_t extra_port_id; + /** + * Color information + */ + VKAPIColorContext color; + /** + * ilcontext associated to the frame context + */ + vkil_context *ilctx; +} VKAPIFramesContext; + +#endif /* AVUTIL_HWCONTEXT_VKAPI_H */ diff --git a/libavutil/pixdesc.c b/libavutil/pixdesc.c index 2a919461a5..1d2f242e57 100644 --- a/libavutil/pixdesc.c +++ b/libavutil/pixdesc.c @@ -2391,6 +2391,10 @@ static const AVPixFmtDescriptor av_pix_fmt_descriptors[AV_PIX_FMT_NB] = { }, .flags = AV_PIX_FMT_FLAG_PLANAR, }, + [AV_PIX_FMT_VKAPI] = { + .name = "vkapi", + .flags = AV_PIX_FMT_FLAG_HWACCEL, + }, [AV_PIX_FMT_VULKAN] = { .name = "vulkan", .flags = AV_PIX_FMT_FLAG_HWACCEL, diff --git a/libavutil/pixfmt.h b/libavutil/pixfmt.h index 46ef211add..3ae607a3d6 100644 --- a/libavutil/pixfmt.h +++ b/libavutil/pixfmt.h @@ -348,6 +348,12 @@ enum AVPixelFormat { AV_PIX_FMT_NV24, ///< planar YUV 4:4:4, 24bpp, 1 plane for Y and 1 plane for the UV components, which are interleaved (first byte U and the following byte V) AV_PIX_FMT_NV42, ///< as above, but U and V bytes are swapped + /** + * VKAPI hardware acceleration. + * data[3] contains a pointer to the vkil_buffer_surface structure + */ + AV_PIX_FMT_VKAPI, + /** * Vulkan hardware images. *