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

DRMPRIME: Check for FFMPEG support #23138

Merged
merged 1 commit into from
Apr 26, 2023
Merged

Conversation

sundermann
Copy link
Contributor

@sundermann sundermann commented Apr 13, 2023

Description

libdrm prime is not available on webOS but is enabled by default

Motivation and context

How has this been tested?

LG OLED77C28 (webOS 7)

What is the effect on users?

Disabling a broken option in player settings (enabling leads to crash on start of a video)

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@fuzzard
Copy link
Contributor

fuzzard commented Apr 13, 2023

Might be time to not include https://github.com/xbmc/xbmc/blob/master/cmake/platform/linux/wayland.cmake at

include(${CMAKE_SOURCE_DIR}/cmake/platform/${CORE_SYSTEM_NAME}/wayland.cmake)

Theres not that much in wayland.cmake.

Does webos use GL or GLES, or is it possible to use either?
Does webos use VAAPI?

The rest (PLATFORM_REQUIRED_DEPS except libDRM of course) can just be duplicated imo

@sundermann
Copy link
Contributor Author

There is only GLES and no VAAPI.

@fuzzard
Copy link
Contributor

fuzzard commented Apr 13, 2023

something like the following. (assuming you need generate-wayland-extra-protocols?)

list(APPEND PLATFORM_REQUIRED_DEPS OpenGLES EGL WaylandProtocols>=1.7 Waylandpp>=0.2.2  Xkbcommon>=0.4.1 WaylandProtocolsWebOS)
list(APPEND PLATFORM_GLOBAL_TARGET_DEPS generate-wayland-extra-protocols generate-wayland-webos-protocols)

set(WAYLAND_EXTRA_PROTOCOL_GENERATED_DIR "${CMAKE_CURRENT_BINARY_DIR}")
# for wayland-extra-protocols.hpp
include_directories("${WAYLAND_EXTRA_PROTOCOL_GENERATED_DIR}")

# add wayland as platform, as we require it.
# saves reworking other assumptions for linux windowing as the platform name.
list(APPEND CORE_PLATFORM_NAME_LC wayland)

list(APPEND ARCH_DEFINES -DTARGET_WEBOS)
set(TARGET_WEBOS TRUE)
set(PREFER_TOOLCHAIN_PATH ${TOOLCHAIN}/${HOST}/sysroot)

You may want to alter the minimum versions of the various required deps to suit as well, because you no longer have to match the generic linux versions.

@sundermann sundermann force-pushed the webos-remove-libdrm branch 2 times, most recently from 212c28b to e7e284d Compare April 13, 2023 12:05
@sundermann sundermann requested a review from yol as a code owner April 13, 2023 12:05
@sundermann
Copy link
Contributor Author

It seems there was a problem with my build setup locally and it seemingly build. LibDRM seems to be used in a lot of places and removing it using ifdefs completely seems like a worse option than just disabling the DRM Prime renderer which is crashing on webOS when enabled.

@sundermann sundermann changed the title webos: Remove libdrm webos: Disable DRMPRIME Apr 13, 2023
@fuzzard fuzzard closed this Apr 14, 2023
@fuzzard fuzzard reopened this Apr 14, 2023
@fuzzard
Copy link
Contributor

fuzzard commented Apr 14, 2023

@lrusak i know the ifdeffery will send you crazy. Any ideas on a better architectural way to handle this?

@sundermann
Copy link
Contributor Author

What about creating a static query function for platforms?

@sundermann sundermann changed the title webos: Disable DRMPRIME DRMPRIME: Check for FFMPEG support Apr 14, 2023
Copy link
Contributor

@lrusak lrusak 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, I can test it later.

@sundermann sundermann force-pushed the webos-remove-libdrm branch 3 times, most recently from 776abe3 to 4c457c8 Compare April 14, 2023 22:21
Copy link
Contributor

@lrusak lrusak left a comment

Choose a reason for hiding this comment

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

I tested this and it seems to work ok. I get the settings available when libdrm is present in FFmpeg. I just made a couple small comments that I would like to see addressed before merging.

@sundermann
Copy link
Contributor Author

I have applied your suggestions

@wsnipex
Copy link
Member

wsnipex commented Apr 22, 2023

jenkins build this please

@wsnipex wsnipex merged commit 46ec553 into xbmc:master Apr 26, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants