[vlc-devel] [PATCH v4] linux: kernel mode setting (KMS) vout plugin
Thomas Guillem
thomas at gllm.fr
Tue Jun 26 15:08:58 CEST 2018
Hello,
This plugin is well written. This is a very good start for DRM support into VLC. Personally, I would love to have OpenGL support with DRM, by using our OpenGL vout with a DRM window plugin and the EGL DRM extension.
I was able to test it and have one remarks:
Why do you set the DRM master mode (drmSetMaster()) ?
It forces to run VLC as root. I tried to remove this part and was able to run your plugin as a normal user (with video group perm).
Maybe you should not fail if drmSetMaster fails ?
On Tue, Jun 26, 2018, at 10:30, Steve Lhomme wrote:
> On 6/26/2018 10:06 AM, Steve Lhomme wrote:
> > On 6/11/2018 2:34 PM, Juha-Pekka Heikkila wrote:
> >> Add new kernel mode setting video output plugin.
> >> By default it tries to match vlc suggested video mode to anything
> >> kms api offers. There are command line parameters to control
> >> which video format to use for both vlc and for drm.
> >>
> >> kms-vlc-chroma parameter to force VLC to use FourCC as output.
> >> kms-drm-chroma parameter to overide DRM requested framebuffer type.
> >>
> >> v4: rebase
> >>
> >> v3: Increased HW frame amount to 3 to be able to reduce stall
> >> from kernel. HW frame amount can be now changed by changing
> >> defined MAXHWBUF. General cleanup.
> >> Rémi Denis-Courmont: I kept module description in beginning
> >> of kms.c, otherwise my plugin look different from all other
> >> plugins in this. I changed desciption for my commandline
> >
> > Not all plugins. We now try to avoid this. It's even easier to find a
> > module definition by scrolling down at the bottom of the file.
> >
> >> parameters in hope to underline their different function.
> >> Destruction of HW buffers is now moved to picture destroy
> >> callback.
> >>
> >> v2, Rémi Denis-Courmont: Multiple picture pools are gone as well as
> >> TTY code which I found out I don't need.
> >> vlc_fourcc_GetYUVFallback/vlc_fourcc_GetRGBFallback I didn't find
> >> easy way to do back'n'forth mapping of what kernel offers and what
> >> vlc offers thus I have simple table of most common formats which
> >> my plugin uses to find 'best' match.
> >>
> >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> >> ---
> >> configure.ac | 5 +
> >> modules/video_output/Makefile.am | 10 +
> >> modules/video_output/kms.c | 829
> >> +++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 844 insertions(+)
> >> create mode 100644 modules/video_output/kms.c
> >>
> >> diff --git a/configure.ac b/configure.ac
> >> index dfb8ecd..564c32b 100644
> >> --- a/configure.ac
> >> +++ b/configure.ac
> >> @@ -3392,6 +3392,11 @@ AC_CHECK_HEADER([linux/fb.h], [
> >> ])
> >> dnl
> >> +dnl Linux kernel mode setting module
> >> +dnl
> >> +PKG_ENABLE_MODULES_VLC([KMS], [], [libdrm >= 2.4.83], [Linux kernel
> >> mode setting output], [auto])
> >> +
> >> +dnl
> >> dnl libcaca plugin
> >> dnl
> >> PKG_ENABLE_MODULES_VLC([CACA], [], [caca >= 0.99.beta14], [libcaca
> >> output],[auto])
> >> diff --git a/modules/video_output/Makefile.am
> >> b/modules/video_output/Makefile.am
> >> index 5d18a36..2be8758 100644
> >> --- a/modules/video_output/Makefile.am
> >> +++ b/modules/video_output/Makefile.am
> >> @@ -422,6 +422,16 @@ EXTRA_LTLIBRARIES += libfb_plugin.la
> >> vout_LTLIBRARIES += $(LTLIBfb)
> >> +### Kernel Mode Setting ###
> >> +
> >> +libkms_plugin_la_SOURCES = video_output/kms.c
> >> +libkms_plugin_la_CFLAGS = $(AM_CFLAGS) $(KMS_CFLAGS)
> >> +libkms_plugin_la_LIBADD = $(KMS_LIBS)
> >> +libkms_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(voutdir)'
> >> +EXTRA_LTLIBRARIES += libkms_plugin.la
> >> +vout_LTLIBRARIES += $(LTLIBkms)
> >> +
> >> +
> >> ### Coloured ASCII art ###
> >> libcaca_plugin_la_SOURCES = video_output/caca.c
> >> libcaca_plugin_la_CFLAGS = $(AM_CFLAGS) $(CACA_CFLAGS)
> >> diff --git a/modules/video_output/kms.c b/modules/video_output/kms.c
> >> new file mode 100644
> >> index 0000000..e26c38b
> >> --- /dev/null
> >> +++ b/modules/video_output/kms.c
> >> @@ -0,0 +1,829 @@
> >> +/*****************************************************************************
> >>
> >> + * kms.c : kernel mode setting plugin for vlc
> >> +
> >> *****************************************************************************
> >> + * Copyright © 2018 Intel Corporation
> >> + *
> >> + * Permission is hereby granted, free of charge, to any person
> >> obtaining a
> >> + * copy of this software and associated documentation files (the
> >> "Software"),
> >> + * to deal in the Software without restriction, including without
> >> limitation
> >> + * the rights to use, copy, modify, merge, publish, distribute,
> >> sublicense,
> >> + * and/or sell copies of the Software, and to permit persons to whom
> >> the
> >> + * Software is furnished to do so, subject to the following conditions:
> >> + *
> >> + * The above copyright notice and this permission notice (including
> >> the next
> >> + * paragraph) shall be included in all copies or substantial
> >> portions of the
> >> + * Software.
> >> + *
> >> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> >> EXPRESS OR
> >> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> >> MERCHANTABILITY,
> >> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO
> >> EVENT SHALL
> >> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> >> OR OTHER
> >> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> >> ARISING
> >> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> >> OTHER DEALINGS
> >> + * IN THE SOFTWARE.
> >> +
> >> *****************************************************************************/
> >> +
> >> +/*****************************************************************************
> >>
> >> + * Preamble
> >> +
> >> *****************************************************************************/
> >> +
> >> +#ifdef HAVE_CONFIG_H
> >> +# include "config.h"
> >> +#endif
> >> +
> >> +#include <fcntl.h>
> >> +#include <sys/mman.h>
> >> +
> >> +#include <xf86drm.h>
> >> +#include <xf86drmMode.h>
> >> +#include <drm_fourcc.h>
> >> +
> >> +#include <vlc_common.h>
> >> +#include <vlc_plugin.h>
> >> +#include <vlc_vout_display.h>
> >> +#include <vlc_picture_pool.h>
> >> +#include <vlc_fs.h>
> >> +
> >> +/*****************************************************************************
> >>
> >> + * Module descriptor
> >> +
> >> *****************************************************************************/
> >> +#define KMS_VAR "kms"
> >> +
> >> +#define DEVICE_TEXT N_("Framebuffer device")
> >> +#define DEVICE_LONGTEXT N_(\
> >> + "Framebuffer device to use for rendering (usually
> >> /dev/dri/card0).")
> >> +
> >> +#define VLC_CHROMA_TEXT N_("Image format used by VLC")
> >> +#define VLC_CHROMA_LONGTEXT N_("Chroma fourcc request to VLC for
> >> output format")
> >> +
> >> +#define DRM_CHROMA_TEXT N_("Image format used by DRM")
> >> +#define DRM_CHROMA_LONGTEXT N_("Chroma fourcc override for DRM
> >> framebuffer format selection")
> >> +
> >> +static int Open (vlc_object_t *);
> >> +static void Close(vlc_object_t *);
> >> +
> >> +vlc_module_begin ()
> >> + set_shortname("kms")
> >> + set_category(CAT_VIDEO)
> >> + set_subcategory(SUBCAT_VIDEO_VOUT)
> >> + add_loadfile(KMS_VAR, "/dev/dri/card0", DEVICE_TEXT,
> >> DEVICE_LONGTEXT)
> >> +
> >> + add_string( "kms-vlc-chroma", NULL, VLC_CHROMA_TEXT,
> >> VLC_CHROMA_LONGTEXT,
> >> + true)
> >> + add_string( "kms-drm-chroma", NULL, DRM_CHROMA_TEXT,
> >> DRM_CHROMA_LONGTEXT,
> >> + true)
> >> + set_description(N_("Linux kernel mode setting video output"))
> >> + set_capability("vout display", 30)
> >> + set_callbacks(Open, Close)
> >> +vlc_module_end ()
> >> +
> >> +/*****************************************************************************
> >>
> >> + * Local prototypes
> >> +
> >> *****************************************************************************/
> >> +
> >> +/*
> >> + * how many hw buffers are allocated for page flipping. I think
> >> + * 3 is enough so we shouldn't get unexpected stall from kernel.
> >> + */
> >> +#define MAXHWBUF 3
> >> +
> >> +typedef enum { drvSuccess, drvTryNext, drvFail } deviceRval;
> >> +
> >> +struct vout_display_sys_t {
> >> +/*
> >> + * buffer information
> >> + */
> >> + uint32_t width;
> >> + uint32_t height;
> >> + uint32_t stride;
> >> + uint32_t size;
> >> + uint32_t offsets[PICTURE_PLANE_MAX];
> >> +
> >> + uint32_t handle[MAXHWBUF];
> >> + uint8_t *map[MAXHWBUF];
> >> +
> >> + uint32_t fb[MAXHWBUF];
> >> + picture_t *picture;
> >> + picture_pool_t *pool;
> >> +
> >> + unsigned int front_buf;
> >> +
> >> + bool forced_drm_fourcc;
> >> + uint32_t drm_fourcc;
> >> + vlc_fourcc_t vlc_fourcc;
> >> +
> >> +/*
> >> + * modeset information
> >> + */
> >> + uint32_t crtc;
> >> + uint32_t plane_id;
> >> +
> >> +/*
> >> + * other generic stuff
> >> + */
> >> + int drm_fd;
> >> +};
> >> +
> >> +typedef struct {
> >> + vout_display_sys_t *p_voutsys;
> >> +} picture_sys_t;
> >> +
> >> +
> >> +static void DestroyFB(vout_display_sys_t const *sys, uint32_t const
> >> buf)
> >> +{
> >> + struct drm_mode_destroy_dumb destroy_req = { .handle =
> >> sys->handle[buf] };
> >> +
> >> + munmap(sys->map[buf], sys->size);
> >> + drmModeRmFB(sys->drm_fd, sys->fb[buf]);
> >> + drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_req);
> >> +}
> >> +
> >> +
> >> +#define ALIGN(v, a) (((v) + (a)-1) & ~((a)-1))
> >> +
> >> +static deviceRval CreateFB(vout_display_t *vd, const int buf)
> >> +{
> >> + vout_display_sys_t *sys = vd->sys;
> >> + struct drm_mode_create_dumb create_req = { .width = sys->width*2,
> >> + .height = sys->height,
> >> + .bpp = 16 };
> >> + struct drm_mode_destroy_dumb destroy_req;
> >> + struct drm_mode_map_dumb modify_req = {};
> >> + unsigned int tile_width = 512, tile_height = 16;
> >> + deviceRval ret;
> >> + int i;
> >> + uint32_t offsets[] = {0,0,0,0}, handles[] = {0,0,0,0},
> >> + pitches[] = {0,0,0,0};
> >> +
> >> + switch(sys->drm_fourcc) {
> >> +#ifdef DRM_FORMAT_P010
> >> + case DRM_FORMAT_P010:
> >> +#endif
> >> +#ifdef DRM_FORMAT_P012
> >> + case DRM_FORMAT_P012:
> >> +#endif
> >> +#ifdef DRM_FORMAT_P016
> >> + case DRM_FORMAT_P016:
> >> +#endif
> >> +#if defined(DRM_FORMAT_P010) || defined(DRM_FORMAT_P012) ||
> >> defined(DRM_FORMAT_P016)
> >> + sys->stride = ALIGN(sys->width*2, tile_width);
> >> + sys->offsets[1] = sys->stride*ALIGN(sys->height, tile_height);
> >> + create_req.height = ALIGN(sys->height*2, tile_height);
> >
> > Shouldn't it be 2*ALIGN(sys->height, tile_height)? You may have
> > misaligned buffers if the alignment doesn't add the same value.
> >> + break;
> >> +#endif
> >> + case DRM_FORMAT_NV12:
> >> + sys->stride = ALIGN(sys->width, tile_width);
> >> + sys->offsets[1] = sys->stride*ALIGN(sys->height, tile_height);
> >> + create_req.height = ALIGN(sys->height*2, tile_height);
> >> + break;
> >> + default:
> >> + create_req.height = ALIGN(sys->height*2, tile_height);
> >> +
> >> + /*
> >> + * width *4 so there's enough space for anything.
> >> + */
> >> + sys->stride = ALIGN(sys->width*4, tile_width);
> >
> > So 4*ALIGN(sys->width, tile_width)?
> >> + break;
> >> + }
> >> +
> >> + ret = drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_CREATE_DUMB,
> >> &create_req);
> >> + if (ret < 0) {
> >> + msg_Err(vd, "Cannot create dumb buffer");
> >> + return drvFail;
> >> + }
> >> +
> >> + sys->size = create_req.size;
> >> + sys->handle[buf] = create_req.handle;
> >> +
> >> + /*
> >> + * create framebuffer object for the dumb-buffer
> >> + * index 0 has to be filled in any case.
> >> + */
> >> + for (i = 0; i < 4 && (sys->offsets[i] || i < 1); i++) {
> >> + handles[i] = create_req.handle;
> >
> > instead of 4 use ARRAY_SIZE(handles)
> >
> >
> >
> >> + pitches[i] = sys->stride;
> >> + offsets[i] = sys->offsets[i];
> >> + }
> >> +
> >> + ret = drmModeAddFB2(sys->drm_fd, sys->width, sys->height,
> >> sys->drm_fourcc,
> >> + handles, pitches, offsets, &sys->fb[buf],
> >> 1<<1);
> >
> > If 1<<1 is a flag maybe it has a pretty name ?
> >
> >> +
> >> + if (ret) {
> >> + msg_Err(vd, "Cannot create frame buffer");
> >> + ret = drvFail;
> >> + goto err_destroy;
> >> + }
> >> +
> >> + modify_req.handle = sys->handle[buf];
> >> + ret = drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_MAP_DUMB, &modify_req);
> >> + if (ret) {
> >> + msg_Err(vd, "Cannot map dumb buffer");
> >> + ret = drvFail;
> >> + goto err_fb;
> >> + }
> >> +
> >> + sys->map[buf] = mmap(0, sys->size, PROT_READ | PROT_WRITE,
> >> MAP_SHARED,
> >> + sys->drm_fd, modify_req.offset);
> >> +
> >> + if (sys->map[buf] == MAP_FAILED) {
> >> + msg_Err(vd, "Cannot mmap dumb buffer");
> >> + ret = drvFail;
> >> + goto err_fb;
> >> + }
> >> + return drvSuccess;
> >> +
> >> +err_fb:
> >> + drmModeRmFB(sys->drm_fd, sys->fb[buf]);
> >> + sys->fb[buf] = 0;
> >> +
> >> +err_destroy:
> >> + memset(&destroy_req, 0, sizeof(destroy_req));
> >> + destroy_req.handle = sys->handle[buf];
> >> + drmIoctl(sys->drm_fd, DRM_IOCTL_MODE_DESTROY_DUMB, &destroy_req);
> >> + return ret;
> >> +}
> >> +
> >> +
> >> +static deviceRval FindCRTC(vout_display_t *vd, drmModeRes const *res,
> >> + drmModeConnector const *conn)
> >> +{
> >> + vout_display_sys_t *sys = vd->sys;
> >> + drmModeEncoder *enc;
> >> + int i, j;
> >> +
> >> + /*
> >> + * Try current encoder and CRTC combo
> >> + */
> >> + if (conn->encoder_id) {
> >> + enc = drmModeGetEncoder(sys->drm_fd, conn->encoder_id);
> >> + if (enc) {
> >> + if (enc->crtc_id) {
> >> + msg_Dbg(vd, "Got CRTC %d from current encoder",
> >> enc->crtc_id);
> >> +
> >> + sys->crtc = enc->crtc_id;
> >> + drmModeFreeEncoder(enc);
> >> + return drvSuccess;
> >> + }
> >> + drmModeFreeEncoder(enc);
> >> + }
> >> + }
> >> +
> >> + /*
> >> + * Iterate all available encoders to find CRTC
> >> + */
> >> + for (i = 0; i < conn->count_encoders; i++) {
> >> + enc = drmModeGetEncoder(sys->drm_fd, conn->encoders[i]);
> >> +
> >> + for (j = 0; enc && j < res->count_crtcs; ++j) {
> >> + if (ffs(enc->possible_crtcs) == j && res->crtcs[j]) {
> >> + msg_Dbg(vd, "Got CRTC %d", res->crtcs[j]);
> >> + sys->crtc = res->crtcs[j];
> >> + drmModeFreeEncoder(enc);
> >> + return drvSuccess;
> >> + }
> >> + }
> >> + drmModeFreeEncoder(enc);
> >> + }
> >> +
> >> + msg_Err(vd , "Cannot find CRTC for connector %d",
> >> conn->connector_id);
> >> + return drvTryNext;
> >> +}
> >> +
> >> +
> >> +static deviceRval SetupDevice(vout_display_t *vd, drmModeRes const
> >> *res,
> >> + drmModeConnector const *conn)
> >> +{
> >> + vout_display_sys_t *sys = vd->sys;
> >> + deviceRval ret;
> >> + int c, c2;
> >> +
> >> + if (conn->connection != DRM_MODE_CONNECTED || conn->count_modes
> >> == 0)
> >> + return drvTryNext;
> >> +
> >> + sys->width = conn->modes[0].hdisplay;
> >> + sys->height = conn->modes[0].vdisplay;
> >> + msg_Dbg(vd, "Mode resolution for connector %u is %ux%u",
> >> + conn->connector_id, sys->width, sys->height);
> >> +
> >> + ret = FindCRTC(vd, res, conn);
> >> + if (ret != drvSuccess) {
> >> + msg_Dbg(vd , "No valid CRTC for connector %d",
> >> conn->connector_id);
> >> + return ret;
> >> + }
> >> +
> >> + for (c = 0; c < MAXHWBUF; c++) {
> >> + ret = CreateFB(vd, c);
> >> + if (ret != drvSuccess) {
> >> + msg_Err(vd, "Cannot create framebuffer %d for connector
> >> %d", c,
> >> + conn->connector_id);
> >> + for (c2 = 0; c2 < c; c2++)
> >> + DestroyFB(sys, c2);
> >> +
> >> + return ret;
> >> + } else {
> >> + msg_Dbg(vd, "Created HW framebuffer %d/%d", c+1, MAXHWBUF);
> >> + }
> >> + }
> >> + return drvSuccess;
> >> +}
> >> +
> >> +
> >> +/** fourccmatching, matching drm to vlc fourccs and see if it was
> >> present
> >> + * in HW. Here really is two lists, one in RGB and second in YUV.
> >> They're
> >> + * listed in order of preference.
> >> + *
> >> + * fourccmatching::drm DRM fourcc code from drm_fourcc.h
> >> + * fourccmatching::vlc VLC fourcc code from vlc_fourcc.h under title
> >> 'Chromas'
> >> + * fourccmatching::plane_id from which plane this DRM fourcc was found
> >> + * fourccmatching::present if this mode was available in HW
> >> + * fourccmatching::isYUV as name suggest..
> >> + */
> >> +static struct
> >> +{
> >> + uint32_t drm;
> >> + vlc_fourcc_t vlc;
> >> + uint32_t plane_id;
> >> + bool present;
> >> + bool isYUV;
> >> +} fourccmatching[] = {
> >> + { .drm = DRM_FORMAT_XRGB8888, .vlc = VLC_CODEC_RGB32, .isYUV =
> >> false },
> >> + { .drm = DRM_FORMAT_RGB565, .vlc = VLC_CODEC_RGB16, .isYUV =
> >> false },
> >> +#if defined DRM_FORMAT_P010
> >> + { .drm = DRM_FORMAT_P010, .vlc = VLC_CODEC_P010, .isYUV = true },
> >> +#endif
> >> + { .drm = DRM_FORMAT_NV12, .vlc = VLC_CODEC_NV12, .isYUV = true },
> >> + { .drm = DRM_FORMAT_YUYV, .vlc = VLC_CODEC_YUYV, .isYUV = true },
> >> + { .drm = DRM_FORMAT_YVYU, .vlc = VLC_CODEC_YVYU, .isYUV = true },
> >> + { .drm = DRM_FORMAT_UYVY, .vlc = VLC_CODEC_UYVY, .isYUV = true },
> >> + { .drm = DRM_FORMAT_VYUY, .vlc = VLC_CODEC_VYUY, .isYUV = true },
> >> +};
> >> +
> >> +
> >> +static void CheckFourCCList(uint32_t drmfourcc, uint32_t plane_id)
> >> +{
> >> + unsigned i;
> >> +
> >> + for (i = 0; i < ARRAY_SIZE(fourccmatching); i++) {
> >> + if (fourccmatching[i].drm == drmfourcc) {
> >> + if (fourccmatching[i].present)
> >> + /* this drmfourcc already has a plane_id found where it
> >> + * could be used, we'll stay with earlier findings.
> >> + */
> >> + break;
> >> +
> >> + fourccmatching[i].present = true;
> >> + fourccmatching[i].plane_id = plane_id;
> >> + break;
> >> + }
> >> + }
> >> +}
> >> +
> >> +
> >> +static bool ChromaNegotiation(vout_display_t *vd)
> >> +{
> >> + vout_display_sys_t *sys = vd->sys;
> >> + unsigned i, c, propi;
> >> + uint32_t planetype;
> >> + const char types[][16] = { "OVERLAY", "PRIMARY", "CURSOR",
> >> "UNKNOWN" };
> >> + drmModePlaneRes *plane_res = NULL;
> >> + drmModePlane *plane;
> >> + drmModeObjectProperties *props;
> >> + drmModePropertyPtr pp;
> >> + bool YUVFormat;
> >> +
> >> + /*
> >> + * For convenience print out in debug prints all supported
> >> + * DRM modes so they can be seen if use verbose mode.
> >> + */
> >> + plane_res = drmModeGetPlaneResources(sys->drm_fd);
> >> + sys->plane_id = 0;
> >> +
> >> + if (plane_res != NULL && plane_res->count_planes > 0) {
> >> + msg_Dbg(vd, "List of DRM supported modes on this machine:");
> >> + for (c = 0; c < plane_res->count_planes; c++) {
> >> +
> >> + plane = drmModeGetPlane(sys->drm_fd, plane_res->planes[c]);
> >> + if (plane != NULL && plane->count_formats > 0) {
> >> + props = drmModeObjectGetProperties(sys->drm_fd,
> >> + plane->plane_id,
> >> + DRM_MODE_OBJECT_PLANE);
> >> +
> >> + planetype = 3;
> >> + pp = NULL;
> >> + for (propi = 0; propi < props->count_props; propi++) {
> >> + pp = drmModeGetProperty(sys->drm_fd,
> >> props->props[propi]);
> >> + if (strcmp(pp->name, "type") == 0) {
> >> + break;
> >> + }
> >> + drmModeFreeProperty(pp);
> >> + }
> >> +
> >> + if (pp != NULL) {
> >> + drmModeFreeProperty(pp);
> >> + planetype = props->prop_values[propi];
> >> + }
> >> +
> >> + for (i = 0; i < plane->count_formats; i++) {
> >> + CheckFourCCList(plane->formats[i],
> >> plane->plane_id);
> >> +
> >> + if (sys->forced_drm_fourcc && sys->plane_id == 0 &&
> >> + plane->formats[i] == sys->drm_fourcc) {
> >> + sys->plane_id = plane->plane_id;
> >> + }
> >> +
> >> + /*
> >> + * we don't advertise about cursor plane because
> >> + * of its special limitations.
> >> + */
> >> + if (planetype != DRM_PLANE_TYPE_CURSOR) {
> >> + msg_Dbg(vd, "plane id %d type %s pipe %c
> >> format %2d: %.4s",
> >> + plane->plane_id, types[planetype],
> >> + ('@'+ffs(plane->possible_crtcs)), i,
> >> + (char*)&plane->formats[i]);
> >> + }
> >> + }
> >> + drmModeFreePlane(plane);
> >> + } else {
> >> + msg_Err(vd, "Couldn't get list of DRM formats");
> >> + drmModeFreePlaneResources(plane_res);
> >> + return false;
> >> + }
> >> + }
> >> + drmModeFreePlaneResources(plane_res);
> >> + }
> >> +
> >> + if (sys->forced_drm_fourcc) {
> >> + for (c = i = 0; c < ARRAY_SIZE(fourccmatching); c++) {
> >> + if (fourccmatching[c].drm == sys->drm_fourcc) {
> >> + sys->vlc_fourcc = fourccmatching[c].vlc;
> >> + break;
> >> + }
> >> + }
> >> + if (sys->plane_id == 0) {
> >> + msg_Err(vd, "Forced DRM fourcc (%.4s) not available in
> >> kernel.",
> >> + (char*)&sys->drm_fourcc);
> >> + return false;
> >> + }
> >> + return true;
> >> + }
> >> +
> >> + /*
> >> + * favor yuv format according to YUVFormat flag.
> >> + * check for exact match first, then YUVFormat and then !YUVFormat
> >> + */
> >> + YUVFormat = vlc_fourcc_IsYUV(sys->vlc_fourcc);
> >
> > You may read this value right before you need it. In this first loop
> > it's not used.
> >> + for (c = i = 0; c < ARRAY_SIZE(fourccmatching); c++) {
> >> + if (fourccmatching[c].vlc == sys->vlc_fourcc) {
> >> + if (!sys->forced_drm_fourcc && fourccmatching[c].present) {
> >> + sys->drm_fourcc = fourccmatching[c].drm;
> >> + sys->plane_id = fourccmatching[c].plane_id;
> >> + }
> >> +
> >> + if (!sys->drm_fourcc) {
> >> + msg_Err(vd, "Forced VLC fourcc (%.4s) not matching
> >> anything available in kernel, please set manually",
> >> + (char*)&sys->vlc_fourcc);
> >> + return false;
> >> + }
> >> + return true;
> >> + }
> >> + }
> >> +
> >> + for (c = i = 0; c < ARRAY_SIZE(fourccmatching); c++) {
> >> + if (fourccmatching[c].isYUV == YUVFormat
> >> + && fourccmatching[c].present) {
> >> + if (!sys->forced_drm_fourcc) {
> >> + sys->drm_fourcc = fourccmatching[c].drm;
> >> + sys->plane_id = fourccmatching[c].plane_id;
> >> + }
> >> +
> >> + sys->vlc_fourcc = fourccmatching[c].vlc;
> >> + return true;
> >> + }
> >> + }
> >> +
> >> + for (i = 0; c < ARRAY_SIZE(fourccmatching); c++) {
> >> + if (!fourccmatching[c].isYUV != YUVFormat
> >> + && fourccmatching[c].present) {
> >> + if (!sys->forced_drm_fourcc) {
> >> + sys->drm_fourcc = fourccmatching[c].drm;
> >> + sys->plane_id = fourccmatching[c].plane_id;
> >> + }
> >> +
> >> + sys->vlc_fourcc = fourccmatching[c].vlc;
> >> + return true;
> >> + }
> >> + }
> >> +
> >> + return false;
> >> +}
> >> +
> >> +
> >> +static int OpenDisplay(vout_display_t *vd)
> >> +{
> >> + vout_display_sys_t *sys = vd->sys;
> >> + char *psz_device;
> >> + int ret;
> >> + uint64_t dumbRet;
> >> + drmModeConnector *conn;
> >> + drmModeRes *modeRes;
> >> + int c;
> >> + bool found_connector = false;
> >> +
> >> + /*
> >> + * Open framebuffer device
> >> + */
> >> + psz_device = var_InheritString(vd, KMS_VAR);
> >> + if (psz_device == NULL) {
> >> + msg_Err(vd, "Don't know which DRM device to open");
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + sys->drm_fd = vlc_open(psz_device, O_RDWR);
> >> + if (sys->drm_fd == -1) {
> >> + msg_Err(vd, "cannot open %s", psz_device);
> >> + free(psz_device);
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + ret = drmSetMaster(sys->drm_fd);
> >> + if (ret < 0) {
> >> + msg_Err(vd, "Cannot become DRM master on %s", psz_device);
> >> +
> >> + free(psz_device);
> >> + vlc_close(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + return VLC_EGENERIC;
> >> + }
> >> + free(psz_device);
> >> +
> >> + drmSetClientCap(sys->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
> >> +
> >> + if (!ChromaNegotiation(vd)) {
> >
> > No error message here ?
> > Maybe use a goto as the code is similar to the following error handling.
> >
> >> + drmDropMaster(sys->drm_fd);
> >> + vlc_close(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + msg_Dbg(vd, "Using VLC chroma '%.4s', DRM chroma '%.4s'",
> >> + (char*)&sys->vlc_fourcc, (char*)&sys->drm_fourcc);
> >> +
> >> + ret = drmGetCap(sys->drm_fd, DRM_CAP_DUMB_BUFFER, &dumbRet);
> >> + if (ret < 0 || !dumbRet) {
> >> + msg_Err(vd, "Device '%s' does not support dumb buffers",
> >> psz_device);
> >> + drmDropMaster(sys->drm_fd);
> >> + vlc_close(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + modeRes = drmModeGetResources(sys->drm_fd);
> >> + if (modeRes == NULL) {
> >> + msg_Err(vd, "Didn't get DRM resources");
> >> + vlc_close(sys->drm_fd);
> >> + drmDropMaster(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + for (c = 0; c < modeRes->count_connectors && sys->crtc == 0; c++) {
> >> +
> >> + conn = drmModeGetConnector(sys->drm_fd,
> >> modeRes->connectors[c]);
> >> + if (conn == NULL)
> >> + continue;
> >> +
> >> + found_connector = true;
> >> +
> >> + ret = SetupDevice(vd, modeRes, conn);
> >> + if (ret != drvSuccess) {
> >> + if (ret != drvTryNext) {
> >> + msg_Err(vd, "Cannot do setup for connector %u:%u", c,
> >> + modeRes->connectors[c]);
> >> +
> >> + drmModeFreeConnector(conn);
> >> + drmModeFreeResources(modeRes);
> >> + drmDropMaster(sys->drm_fd);
> >> + vlc_close(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + return VLC_EGENERIC;
> >> + }
> >> + drmModeFreeConnector(conn);
> >> + found_connector = false;
> >> + continue;
> >> + }
> >> + drmModeFreeConnector(conn);
> >> + }
> >> + drmModeFreeResources(modeRes);
> >> + if (!found_connector) {
> >> + drmModeFreeResources(modeRes);
> >> + drmDropMaster(sys->drm_fd);
> >> + vlc_close(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + return VLC_EGENERIC;
> >> + }
> >> + return VLC_SUCCESS;
> >> +}
> >> +
> >> +
> >> +static int Control(vout_display_t *vd, int query, va_list args)
> >> +{
> >> + VLC_UNUSED(vd);
> >> + VLC_UNUSED(query);
> >> + VLC_UNUSED(args);
> >> + return VLC_EGENERIC;
> >> +}
> >> +
> >> +
> >> +static void CustomDestroyPicture(picture_t *p_picture)
> >> +{
> >> + picture_sys_t *psys = (picture_sys_t*)p_picture->p_sys;
> >> + vout_display_sys_t *sys = (vout_display_sys_t *)psys->p_voutsys;
> >> + int c;
> >> +
> >> + for (c = 0; c < MAXHWBUF; c++)
> >> + DestroyFB(sys, c);
> >
> > Given the buffers/resources are not allocated when the pool is
> > created, they should not be released when the pool is released. In
> > this case when the module is released.
> >
> >> +
> >> + drmSetClientCap(sys->drm_fd, DRM_CLIENT_CAP_UNIVERSAL_PLANES, 0);
> >> + drmDropMaster(sys->drm_fd);
> >> + vlc_close(sys->drm_fd);
> >> + sys->drm_fd = 0;
> >> + free(p_picture->p_sys);
> >> + free(p_picture);
> >> +}
> >> +
> >> +
> >> +static picture_pool_t *Pool(vout_display_t *vd, unsigned count)
> >> +{
> >> + VLC_UNUSED(count);
> >> + vout_display_sys_t *sys = vd->sys;
> >> + picture_sys_t *psys;
> >> + picture_resource_t rsc;
> >> + int i;
> >> +
> >> + if (!sys->pool && !sys->picture) {
> >> + memset(&rsc, 0, sizeof(rsc));
> >> +
> >> + for (i = 0; i < PICTURE_PLANE_MAX; i++) {
> >> + rsc.p[i].p_pixels = sys->map[0]+sys->offsets[i];
> >> + rsc.p[i].i_lines = sys->height;
> >> + rsc.p[i].i_pitch = sys->stride;
> >> + }
> >> +
> >> + psys = calloc(1, sizeof(*psys));
> >> + if (psys == NULL)
> >> + return NULL;
> >> +
> >> + psys->p_voutsys = sys;
> >> + rsc.p_sys = psys;
> >> + rsc.pf_destroy = CustomDestroyPicture;
> >> +
> >> + sys->picture = picture_NewFromResource(&vd->fmt, &rsc);
> >> +
> >> + if (!sys->picture) {
> >> + free((void*)psys);
> >> + return NULL;
> >> + }
> >> +
> >> + sys->pool = picture_pool_New(1, &sys->picture);
> >> + if (!sys->pool)
> >> + picture_Release(sys->picture);
> >> + }
> >> +
> >> + return sys->pool;
> >> +}
> >> +
> >> +
> >> +static void Display(vout_display_t *vd, picture_t *picture,
> >> + subpicture_t *subpicture)
> >> +{
> >> + VLC_UNUSED(subpicture);
> >> + vout_display_sys_t *sys = vd->sys;
> >> + int i;
> >> +
> >> + if (drmModeSetPlane(sys->drm_fd, sys->plane_id, sys->crtc,
> >> + sys->fb[sys->front_buf], 1<<1,
> >> + 0, 0, sys->width, sys->height,
> >> + 0, 0, sys->width << 16, sys->height << 16))
> >> + {
> >> + msg_Err(vd, "Cannot do set plane for plane id %u, fb %x",
> >> + sys->plane_id,
> >> + sys->fb[sys->front_buf]);
> >> + } else {
> >> + sys->front_buf++;
> >> + sys->front_buf %= MAXHWBUF;
> >> +
> >> + for (i = 0; i < PICTURE_PLANE_MAX; i++)
> >> + sys->picture->p[i].p_pixels =
> >> + sys->map[sys->front_buf]+sys->offsets[i];
> >
> > Are you switching buffers in the single pool picture here ? IMO it
> > would be cleaner to use MAXHWBUF pictures in the pool and let the
> > picture pool do its job. You won't have to manage the front_buf yourself.
>
> While you're at it you might dump MAXHWBUF if the resources are usually
> not contrained. And allocate the amount of buffers requested in the
> Pool() call. This way you may benefit from direct rendering which is faster.
>
> If working in the DRM memory (is it GPU memory ?) is slow then you
> should set vd->info.is_slow = true. In this case you won't get direct
> rendering but you won't slow down decoding.
>
> Note that the core requests more buffers than it actually use most of
> the time. So if the amount of memory you can allocate is too small you
> should just allocate as much as you can and the core will allocate a
> separate memory pool for the decoder. But if that's CPU memory you're
> using it will just allocate more memory in the same place...
>
> >> + }
> >> + picture_Release(picture);
> >> +}
> >> +
> >> +
> >> +/**
> >> + * This function allocates and initializes a KMS vout method.
> >> + */
> >> +static int Open(vlc_object_t *object)
> >> +{
> >> + vout_display_t *vd = (vout_display_t *)object;
> >> + vout_display_sys_t *sys;
> >> + vlc_fourcc_t local_vlc_chroma;
> >> + uint32_t local_drm_chroma;
> >> + video_format_t fmt = {};
> >> + char *chroma;
> >> +
> >> + if (vout_display_IsWindowed(vd))
> >> + return VLC_EGENERIC;
> >> +
> >> + /*
> >> + * Allocate instance and initialize some members
> >> + */
> >> + vd->sys = sys = calloc(1, sizeof(*sys));
> >
> > Use vlc_obj_calloc().
> >
> >> + if (!sys)
> >> + return VLC_ENOMEM;
> >> +
> >> + sys->pool = NULL;
> >> + sys->drm_fourcc = 0;
> >> + sys->vlc_fourcc = 0;
> >
> > calloc already sets everything to 0
> >
> >> +
> >> + chroma = var_InheritString(vd, "kms-vlc-chroma");
> >> + if (chroma) {
> >> + local_vlc_chroma = vlc_fourcc_GetCodecFromString(VIDEO_ES,
> >> chroma);
> >> +
> >> + if (local_vlc_chroma) {
> >> + sys->vlc_fourcc = local_vlc_chroma;
> >> + msg_Dbg(vd, "Forcing VLC to use chroma '%4s'", chroma);
> >> + } else {
> >> + sys->vlc_fourcc = vd->fmt.i_chroma;
> >> + msg_Dbg(vd, "Chroma %4s invalid, using default", chroma);
> >> + }
> >> +
> >> + free(chroma);
> >> + chroma = NULL;
> >> + } else {
> >> + sys->vlc_fourcc = vd->fmt.i_chroma;
> >> + msg_Dbg(vd, "Chroma %4s invalid, using default", chroma);
> >> + }
> >> +
> >> + chroma = var_InheritString(vd, "kms-drm-chroma");
> >> + if (chroma) {
> >> + local_drm_chroma = VLC_FOURCC(chroma[0], chroma[1], chroma[2],
> >> + chroma[3]);
> >> +
> >> + if (local_drm_chroma) {
> >> + sys->forced_drm_fourcc = true;
> >> + sys->drm_fourcc = local_drm_chroma;
> >> + msg_Dbg(vd, "Setting DRM chroma to '%4s'", chroma);
> >> + }
> >> + else
> >> + msg_Dbg(vd, "Chroma %4s invalid, using default", chroma);
> >> +
> >> + free(chroma);
> >> + chroma = NULL;
> >> + }
> >> +
> >> + if (OpenDisplay(vd)) {
> >
> > != VLC_SUCCESS is more readable
> >
> >> + Close(VLC_OBJECT(vd));
> >> + return VLC_EGENERIC;
> >> + }
> >> +
> >> + video_format_ApplyRotation(&fmt, &vd->fmt);
> >> +
> >> + fmt.i_width = fmt.i_visible_width = sys->width;
> >> + fmt.i_height = fmt.i_visible_height = sys->height;
> >> + fmt.i_chroma = sys->vlc_fourcc;
> >> +
> >> + vd->fmt = fmt;
> >> + vd->pool = Pool;
> >> + vd->prepare = NULL;
> >> + vd->display = Display;
> >> + vd->control = Control;
> >> +
> >> + vout_window_ReportSize(vd->cfg->window, sys->width, sys->height);
> >> + return VLC_SUCCESS;
> >> +}
> >> +
> >> +
> >> +static void CloseDisplay(vout_display_t *vd)
> >> +{
> >> + vout_display_sys_t *sys = vd->sys;
> >> +
> >> + if (sys->pool)
> >> + picture_pool_Release(sys->pool);
> >> +
> >> + if (sys->drm_fd)
> >> + drmDropMaster(sys->drm_fd);
> >> +
> >> + free(sys);
> >> +}
> >> +
> >> +
> >> +/**
> >> + * Terminate an output method created by Open
> >> + */
> >> +static void Close(vlc_object_t *object)
> >> +{
> >> + vout_display_t *vd = (vout_display_t *)object;
> >> +
> >> + CloseDisplay(vd);
> >> +}
> >
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list