Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vo: hwdec: drmprime: add separate hwdecs for v4l2request #14690

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

philipl
Copy link
Member

@philipl philipl commented Aug 18, 2024

With all the machinery in place, we can now add the v4l2request hwdecs with a different hw device type, and a different initialisation path. This applies to both the drmprime and drmprime_overlay hwdecs.

At the moment, the device initialisation is done in the bare minimum way, but it can be extended to take a device path (for example) if that makes sense as we better understand what meaningful configuration will be.

This change is based on the code proposed by Jonas Karlman jonas@kwiboo.se in #14511

@philipl
Copy link
Member Author

philipl commented Aug 18, 2024

@Kwiboo I suggest we close #14511 and track the final support code in this PR which we'll merge once the ffmpeg changes are finalised.

Copy link

github-actions bot commented Aug 18, 2024

Download the artifacts for this pull request:

Windows
macOS

@diederikdehaas
Copy link
Contributor

It's (very) likely just a dumb error on my part, but I'm getting an error when compiling it (on an arm64 device with Debian Trixie/Sid):

/usr/bin/ld: libmpv.so.2.3.0.p/video_out_gpu_hwdec.c.o:(.data.rel.ro+0x20): undefined reference to `ra_hwdec_v4l2request'
/usr/bin/ld: libmpv.so.2.3.0.p/video_out_gpu_hwdec.c.o:(.data.rel.ro+0x28): undefined reference to `ra_hwdec_v4l2request_overlay'

Parts of my build log (hopefully relevant):

make[1]: Entering directory '/home/diederik/dev/debian/salsa/multimedia-team/mpv'
dh_auto_configure -- \
	-Dlibmpv=true \
	-Dbuild-date=false \
	-Dcdda=enabled \
	-Ddvdnav=enabled \
	-Dsdl2=enabled \
	-Dv4l2request=enabled \
	-Davfoundation=disabled \
	-Degl-angle-win32=disabled \
	-Dgl-cocoa=disabled \
	-Dgl-win32=disabled \
	-Dios-gl=disabled \
	-Duwp=disabled \
	-Dvaapi-win32=disabled \
	-Dwasapi=disabled \
	-Dwin32-threads=disabled \
	-Ddvbin=enabled
	cd obj-aarch64-linux-gnu && DEB_PYTHON_INSTALL_LAYOUT=deb LC_ALL=C.UTF-8 meson setup .. --wrap-mode=nodownload --buildtype=plain --prefix=/usr --sysconfdir=/etc --localstatedir=/var --libdir=lib/aarch64-linux-gnu -Dpython.bytecompile=-1 -Dlibmpv=true -Dbuild-date=false -Dcdda=enabled -Ddvdnav=enabled -Dsdl2=enabled -Dv4l2request=enabled -Davfoundation=disabled -Degl-angle-win32=disabled -Dgl-cocoa=disabled -Dgl-win32=disabled -Dios-gl=disabled -Duwp=disabled -Dvaapi-win32=disabled -Dwasapi=disabled -Dwin32-threads=disabled -Ddvbin=enabled
The Meson build system
Version: 1.5.1
Source dir: /home/diederik/dev/debian/salsa/multimedia-team/mpv
Build dir: /home/diederik/dev/debian/salsa/multimedia-team/mpv/obj-aarch64-linux-gnu
Build type: native build
Project name: mpv
Project version: 0.38.0
C compiler for the host machine: cc (gcc 14.2.0 "cc (Debian 14.2.0-1) 14.2.0")
C linker for the host machine: cc ld.bfd 2.43
Host machine cpu family: aarch64
Host machine cpu: aarch64
...
Message: List of enabled features: alsa av-channel-layout avif-muxer caca cdda cplugins cuda-hwaccel cuda-interop dmabuf-interop-gl dmabuf-wayland drm dvbin dvdnav egl egl-drm egl-wayland egl-x11 ffmpeg ffnvcodec gbm gl glibc-thread-name glob glob-posix gpl iconv jack javascript jpeg jpegxl lavu-uuid lcms2 libarchive libass libavdevice libbluray libdl libplacebo linux-fstatfs lua52 memfd-create pipewire posix posix-shm ppoll pthread-condattr-setclock pulse rubberband rubberband-3 sdl2 sdl2-audio sdl2-gamepad sdl2-video sixel sndio sndio-1-9 uchardet v4l2request vaapi vaapi-drm vaapi-wayland vaapi-x11 vdpau vector vk-khr-display vt.h vulkan vulkan-interop wayland wayland-protocols-1-27 wayland-protocols-1-31 wayland-protocols-1-32 x11 xv zimg zimg-st428 zlib
Fetching value of define "MPV_CLIENT_API_VERSION" : (((2) << 16) | (3) | 0UL) 
Compiler for C supports link arguments -Wl,-Bsymbolic: YES 
Build targets in project: 46

mpv 0.38.0

    d3d11             : NO
    javascript        : YES
    libmpv            : YES
    lua               : YES
    opengl            : YES
    v4l2request       : YES
    vulkan            : YES
    wayland           : YES
    x11               : YES

  User defined options
    buildtype         : plain
    libdir            : lib/aarch64-linux-gnu
    localstatedir     : /var
    prefix            : /usr
    sysconfdir        : /etc
    wrap_mode         : nodownload
    python.bytecompile: -1
    avfoundation      : disabled
    build-date        : false
    cdda              : enabled
    dvbin             : enabled
    dvdnav            : enabled
    egl-angle-win32   : disabled
    gl-cocoa          : disabled
    gl-win32          : disabled
    ios-gl            : disabled
    libmpv            : true
    sdl2              : enabled
    uwp               : disabled
    v4l2request       : enabled
    vaapi-win32       : disabled
    wasapi            : disabled
    win32-threads     : disabled

Found ninja-1.12.1 at /usr/bin/ninja

What am I missing?
Note: I was successful in building PR 14511

https://salsa.debian.org/diederik/mpv/-/commits/v4l2request-hwdevice is the source of my build

@philipl
Copy link
Member Author

philipl commented Aug 18, 2024

@diederikdehaas yeah. I need to change how I did that conditional build logic.

Copy link
Contributor

@Kwiboo Kwiboo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AV_HWDEVICE_TYPE_V4L2REQUEST is a enum value so it cannot be used with #ifdef, an #if for HAVE_V4L2REQUEST should work.

Should also be possible to change the v4l2request feature to auto as it check if the AV_HWDEVICE_TYPE_V4L2REQUEST enum value exists.

meson_options.txt Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime.c Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime.c Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime_overlay.c Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime_overlay.c Outdated Show resolved Hide resolved
We will probably adjust this to look for a specific libavutil version after
v4l2request support is merged upstream, but this check is fine for now.
Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look ok, not tested though. I would wait for ffmpeg side of changes to be merged and after that we can retest and merge this guy.

meson.build Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime.c Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime.c Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime.c Outdated Show resolved Hide resolved
video/out/hwdec/hwdec_drmprime_overlay.c Outdated Show resolved Hide resolved
@diederikdehaas
Copy link
Contributor

I would wait for ffmpeg side of changes to be merged and after that we can retest and merge this guy.

FWIW: I've verified that on my amd64 PC, which does not have the ffmpeg changes, the build 'still' succeeds, but just without v4l2request support:

Build targets in project: 46

mpv 0.38.0

    d3d11             : NO
    javascript        : YES
    libmpv            : YES
    lua               : YES
    opengl            : YES
    v4l2request       : NO
    vulkan            : YES
    wayland           : YES
    x11               : YES

And that is confirmed by a successful CI build here: https://salsa.debian.org/diederik/mpv/-/pipelines/718879
(before autodetection I enabled it explicitly and then the CI build failed)

Whereas on my arm64 device which does have ffmpeg changes, the build succeeds as well, but also has this line:

    v4l2request       : YES

You can still choose to wait ofc.

Now on to runtime testing it...

@diederikdehaas
Copy link
Contributor

Did you try with --vo=dmabuf-wayland? Any difference?

With my/Debian based 0.38 + this patch set I noticed the following on my PineTab2 with Sway:

  1. With videos where v4l2request is used, average CPU usage went from ~20% to ~10%
  2. If I then request an OSD overlay (?) like with O or I and then press that again to make it disappear again, part of it remains on screen, which seems in the area of the black bars above (and probably below) the 'actual' video. When I go to normal screen size (and then full screen again), those left-overs disappear again
  3. With some of my test videos, v4l2request doesn't work (same as with previous iterations of this patch set). When I use this (extra) parameter, I don't get a video at all, while without it I do see a video but this is then using CPU based rendering.

This could ofc already be fixed in master, but I wanted to do my initial tests as close as possible to my previous ones.

@Dudemanguy
Copy link
Member

The second one has been reported in #13318. I was never personally able to reproduce but it's at least unrelated to the hwdec.

With all the machinery in place, we can now add the v4l2request hwdecs with a
different hw device type, and a different initialisation path. This applies to
both the drmprime and drmprime_overlay hwdecs.

At the moment, the device initialisation is done in the bare minimum way, but
it can be extended to take a device path (for example) if that makes sense as
we better understand what meaningful configuration will be.

Co-authored-by: Jonas Karlman <jonas@kwiboo.se>
@philipl
Copy link
Member Author

philipl commented Aug 19, 2024

Look ok, not tested though. I would wait for ffmpeg side of changes to be merged and after that we can retest and merge this guy.

That's fine. I'll leave this here until it's in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants