A couple minor comments below.
On Wed, 2019-01-16 at 13:52 +0100, Lukáš Hrázký wrote:
> Adds an interface method to the FrameCapture class to get the device
> display info (device address and device display id) for each display
> of
> the graphics device that is captured.
>
> Also adds functions to the API implementing this functionality for
> X11
> in variants with and without DRM (the non-DRM version is rather
> limited
> and may not work for more complex setups) as well as some helper
> functions to make it easier for plugins to implement this and avoid
> code
> duplication.
>
> Implements the new interface method for the two built-in plugins
> (mjpeg-fallback and gst-plugin).
>
> Signed-off-by: Lukáš Hrázký <lhrazky@xxxxxxxxxx>
> ---
> configure.ac | 2 +
> include/spice-streaming-agent/Makefile.am | 2 +
> .../spice-streaming-agent/display-info.hpp | 52 +++
> .../spice-streaming-agent/frame-capture.hpp | 13 +
> .../x11-display-info.hpp | 57 ++++
> src/Makefile.am | 7 +
> src/display-info.cpp | 100 ++++++
> src/gst-plugin.cpp | 14 +
> src/mjpeg-fallback.cpp | 17 +-
> src/spice-streaming-agent.cpp | 13 +
> src/unittests/Makefile.am | 6 +
> src/utils.cpp | 46 +++
> src/utils.hpp | 4 +
> src/x11-display-info.cpp | 302
> ++++++++++++++++++
> 14 files changed, 633 insertions(+), 2 deletions(-)
> create mode 100644 include/spice-streaming-agent/display-info.hpp
> create mode 100644 include/spice-streaming-agent/x11-display-
> info.hpp
> create mode 100644 src/display-info.cpp
> create mode 100644 src/utils.cpp
> create mode 100644 src/x11-display-info.cpp
>
> diff --git a/configure.ac b/configure.ac
> index e66e6d8..fd18efe 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -34,8 +34,10 @@ SPICE_PROTOCOL_MIN_VER=0.12.14
> PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >=
> $SPICE_PROTOCOL_MIN_VER])
> AC_SUBST([SPICE_PROTOCOL_MIN_VER])
>
> +PKG_CHECK_MODULES(DRM, libdrm)
> PKG_CHECK_MODULES(X11, x11)
> PKG_CHECK_MODULES(XFIXES, xfixes)
> +PKG_CHECK_MODULES(XRANDR, xrandr)
>
> PKG_CHECK_MODULES(JPEG, libjpeg, , [
> AC_CHECK_LIB(jpeg, jpeg_destroy_decompress,
> diff --git a/include/spice-streaming-agent/Makefile.am
> b/include/spice-streaming-agent/Makefile.am
> index bcd679b..96c1a57 100644
> --- a/include/spice-streaming-agent/Makefile.am
> +++ b/include/spice-streaming-agent/Makefile.am
> @@ -1,8 +1,10 @@
> NULL =
> public_includedir = $(includedir)/spice-streaming-agent
> public_include_HEADERS = \
> + display-info.hpp \
> error.hpp \
> frame-capture.hpp \
> plugin.hpp \
> + x11-display-info.hpp \
> $(NULL)
>
> diff --git a/include/spice-streaming-agent/display-info.hpp
> b/include/spice-streaming-agent/display-info.hpp
> new file mode 100644
> index 0000000..f16212b
> --- /dev/null
> +++ b/include/spice-streaming-agent/display-info.hpp
> @@ -0,0 +1,52 @@
> +/* \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#ifndef SPICE_STREAMING_AGENT_DISPLAY_INFO_HPP
> +#define SPICE_STREAMING_AGENT_DISPLAY_INFO_HPP
> +
> +#include <vector>
> +#include <string>
> +
> +
> +namespace spice __attribute__ ((visibility ("default"))) {
> +namespace streaming_agent {
> +
> +/**
> + * Lists graphics cards listed in the DRM sybsystem in
> /sys/class/drm.
> + * Throws an instance of Error in case of an I/O error.
> + *
> + * @return a vector of paths of all graphics cards present in
> /sys/class/drm
> + */
> +std::vector<std::string> list_cards();
> +
> +/**
> + * Reads a single number in hex format from a file.
> + * Throws an instance of Error in case of an I/O or parsing error.
> + *
> + * @param path the path to the file
> + * @return the number parsed from the file
> + */
> +uint32_t read_hex_number_from_file(const std::string &path);
> +
> +/**
> + * Resolves any symlinks and then extracts the PCI path from the
> canonical path
> + * to a card. Returns the path in the following format:
> + * "pci/<DOMAIN>/<SLOT>.<FUNCTION>/.../<SLOT>.<FUNCTION>"
> + *
> + * <DOMAIN> is the PCI domain, followed by <SLOT>.<FUNCTION> of any
> PCI bridges
> + * in the chain leading to the device. The last <SLOT>.<FUNCTION> is
> the
> + * graphics device. All of <DOMAIN>, <SLOT>, <FUNCTION> are
> hexadecimal numbers
> + * with the following number of digits:
> + * <DOMAIN>: 4
> + * <SLOT>: 2
> + * <FUNCTION>: 1
> + *
> + * @param device_path the path to the card
> + * @return the device address
> + */
> +std::string get_device_address(const std::string &card_path);
> +
> +}} // namespace spice::streaming_agent
> +
> +#endif // SPICE_STREAMING_AGENT_DISPLAY_INFO_HPP
> diff --git a/include/spice-streaming-agent/frame-capture.hpp
> b/include/spice-streaming-agent/frame-capture.hpp
> index 51a2987..c244fb9 100644
> --- a/include/spice-streaming-agent/frame-capture.hpp
> +++ b/include/spice-streaming-agent/frame-capture.hpp
> @@ -6,7 +6,11 @@
> */
> #ifndef SPICE_STREAMING_AGENT_FRAME_CAPTURE_HPP
> #define SPICE_STREAMING_AGENT_FRAME_CAPTURE_HPP
> +
> +#include <cstdint>
> #include <cstdio>
> +#include <string>
> +#include <vector>
>
> #include <spice/enums.h>
>
> @@ -29,6 +33,13 @@ struct FrameInfo
> bool stream_start;
> };
>
> +struct DeviceDisplayInfo
> +{
> + uint32_t stream_id;
> + std::string device_address;
> + uint32_t device_display_id;
> +};
> +
> /*!
> * Pure base class implementing the frame capture
> */
> @@ -52,6 +63,8 @@ public:
> * Get video codec used to encode last frame
> */
> virtual SpiceVideoCodecType VideoCodecType() const = 0;
> +
> + virtual std::vector<DeviceDisplayInfo> get_device_display_info()
> const = 0;
> protected:
> FrameCapture() = default;
> FrameCapture(const FrameCapture&) = delete;
> diff --git a/include/spice-streaming-agent/x11-display-info.hpp
> b/include/spice-streaming-agent/x11-display-info.hpp
> new file mode 100644
> index 0000000..fa6afa7
> --- /dev/null
> +++ b/include/spice-streaming-agent/x11-display-info.hpp
> @@ -0,0 +1,57 @@
> +/* \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#ifndef SPICE_STREAMING_AGENT_X_DISPLAY_INFO_HPP
> +#define SPICE_STREAMING_AGENT_X_DISPLAY_INFO_HPP
> +
> +#include <spice-streaming-agent/frame-capture.hpp>
> +
> +#include <X11/Xlib.h>
> +
> +
> +namespace spice __attribute__ ((visibility ("default"))) {
> +namespace streaming_agent {
> +
> +/**
> + * Looks up device display info by listing the xrandr outputs of
> @display and
> + * comparing them to card output names found under the DRM subsystem
> in
> + * /sys/class/drm.
> + *
> + * Throws an Error in case of error or when the DRM outputs cannot
> be found.
> + *
> + * @param display the X display to find the info for
> + * @return a vector of DeviceDisplayInfo structs containing the
> information of
> + * all the outputs used by the display
> + */
> +std::vector<DeviceDisplayInfo> get_device_display_info_drm(Display
> *display);
> +
> +/**
> + * Attempts to get the device display info without using DRM. It
> looks up the
> + * first card in /sys/class/drm that doesn't have its outputs listed
> (assuming
> + * that if the card in use has DRM outputs, it would be found using
> the
> + * get_display_info function) and simply uses the xrandr output
> index as the
> + * device_display_id. This can obviously incorrectly match the
> device address
> + * and the device_display_ids of two unrelated devices.
> + *
> + * Unfortunately, without DRM, there is no way to match xrandr
> objects with the
> + * real device.
> + *
> + * @param display the X display to find the info for
> + * @return a vector of DeviceDisplayInfo structs containing the
> information of
> + * all the outputs used by the display
> + */
> +std::vector<DeviceDisplayInfo>
> get_device_display_info_no_drm(Display *display);
> +
> +/**
> + * Lists xrandr outputs and returns their names in a vector of
> strings.
> + *
> + * @param display the X display
> + * @param window the X root window
> + * @return A vector of xrandr output names
> + */
> +std::vector<std::string> get_xrandr_outputs(Display *display, Window
> window);
> +
> +}} // namespace spice::streaming_agent
> +
> +#endif // SPICE_STREAMING_AGENT_X_DISPLAY_INFO_HPP
> diff --git a/src/Makefile.am b/src/Makefile.am
> index bae3f9d..0133bf5 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -16,9 +16,11 @@ AM_CPPFLAGS = \
> -DSPICE_STREAMING_AGENT_PROGRAM \
> -I$(top_srcdir)/include \
> -DPLUGINSDIR=\"$(pkglibdir)/plugins\" \
> + $(DRM_CFLAGS) \
> $(SPICE_PROTOCOL_CFLAGS) \
> $(X11_CFLAGS) \
> $(XFIXES_CFLAGS) \
> + $(XRANDR_CFLAGS) \
> $(NULL)
>
> AM_CFLAGS = \
> @@ -50,8 +52,10 @@ spice_streaming_agent_LDADD = \
> -ldl \
> -lpthread \
> libstreaming-utils.a \
> + $(DRM_LIBS) \
> $(X11_LIBS) \
> $(XFIXES_LIBS) \
> + $(XRANDR_LIBS) \
> $(JPEG_LIBS) \
> $(NULL)
>
> @@ -61,6 +65,7 @@ spice_streaming_agent_SOURCES = \
> concrete-agent.hpp \
> cursor-updater.cpp \
> cursor-updater.hpp \
> + display-info.cpp \
> frame-log.cpp \
> frame-log.hpp \
> mjpeg-fallback.cpp \
> @@ -69,7 +74,9 @@ spice_streaming_agent_SOURCES = \
> jpeg.hpp \
> stream-port.cpp \
> stream-port.hpp \
> + utils.cpp \
> utils.hpp \
> + x11-display-info.cpp \
> $(NULL)
>
> if HAVE_GST
> diff --git a/src/display-info.cpp b/src/display-info.cpp
> new file mode 100644
> index 0000000..469d587
> --- /dev/null
> +++ b/src/display-info.cpp
> @@ -0,0 +1,100 @@
> +/* \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include <spice-streaming-agent/display-info.hpp>
> +
> +#include "utils.hpp"
> +#include <spice-streaming-agent/error.hpp>
> +
> +#include <algorithm>
> +#include <cstring>
> +#include <fstream>
> +#include <limits.h>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +namespace {
> +
> +std::string get_card_real_path(const std::string &card_path)
> +{
> + char real_path_buf[PATH_MAX];
> + if (realpath(card_path.c_str(), real_path_buf) == NULL) {
> + throw Error("Failed to realpath \"" + card_path + "\": " +
> strerror(errno));
I thinik that realpath() technically needs <cstdlib>. Presumably it's
pulled it from somewhere else, but just for completeness...
> + }
> +
> + return real_path_buf;
> +}
> +
> +} // namespace
> +
> +std::vector<std::string> list_cards()
> +{
> + std::string glob_path = "/sys/class/drm/card*";
> + auto globs = utils::glob(glob_path);
> +
> + globs.erase(std::remove_if(globs.begin(), globs.end(), [](const
> std::string &glob) {
> + // if the path contains "-", it is an output, not a
> card, filter it out
> + return glob.find("-") != glob.npos;
> + }),
> + globs.end()
> + );
> +
> + return globs;
> +}
> +
> +uint32_t read_hex_number_from_file(const std::string &path)
> +{
> + uint32_t res;
> + std::ifstream vf(path);
> +
> + if (vf.fail()) {
> + throw Error("Failed to open " + path + ": " +
> std::strerror(errno));
> + }
> +
> + if (!(vf >> std::hex >> res)) {
> + throw Error("Failed to read from " + path + ": " +
> std::strerror(errno));
> + }
> +
> + return res;
> +}
> +
> +std::string get_device_address(const std::string &card_path)
> +{
> + std::string real_path = get_card_real_path(card_path);
> +
> + // real_path is e.g.
> /sys/devices/pci0000:00/0000:00:02.0/drm/card0
> + std::string device_address = "pci/";
> +
> + const std::string path_prefix = "/sys/devices/pci";
> +
> + // if real_path doesn't start with path_prefix
> + if (real_path.compare(0, path_prefix.length(), path_prefix)) {
> + throw Error("Invalid device path \"" + real_path + "\"");
> + }
> +
> + // /sys/devices/pci0000:00/0000:00:02.0/drm/card0
> + // ^ ptr
> + const char *ptr = real_path.c_str() + path_prefix.length();
> +
> + // copy the domain
> + device_address += std::string(ptr, 4);
> +
> + // /sys/devices/pci0000:00/0000:00:02.0/drm/card0
> + // ^ ptr
> + ptr += 7;
> +
> + uint32_t domain, bus, slot, function, n;
> + while (sscanf(ptr, "/%x:%x:%x.%x%n", &domain, &bus, &slot,
> &function, &n) == 4) {
> + char pci_node[6];
> + snprintf(pci_node, 6, "/%02x.%01x", slot, function);
> + device_address += pci_node;
> + ptr += n;
> + }
> +
> + return device_address;
> +}
> +
> +}} // namespace spice::streaming_agent
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 097ef9d..3edf9f5 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -23,6 +23,8 @@
>
> #include <spice-streaming-agent/plugin.hpp>
> #include <spice-streaming-agent/frame-capture.hpp>
> +#include <spice-streaming-agent/x11-display-info.hpp>
> +
>
> #define gst_syslog(priority, str, ...) syslog(priority, "Gstreamer
> plugin: " str, ## __VA_ARGS__);
>
> @@ -76,6 +78,7 @@ public:
> SpiceVideoCodecType VideoCodecType() const override {
> return settings.codec;
> }
> + std::vector<DeviceDisplayInfo> get_device_display_info() const
> override;
> private:
> void free_sample();
> GstElement *get_encoder_plugin(const GstreamerEncoderSettings
> &settings, GstCapsUPtr &sink_caps);
> @@ -402,6 +405,17 @@ FrameInfo GstreamerFrameCapture::CaptureFrame()
> return info;
> }
>
> +std::vector<DeviceDisplayInfo>
> GstreamerFrameCapture::get_device_display_info() const
> +{
> + try {
> + return get_device_display_info_drm(dpy);
> + } catch (const std::exception &e) {
> + syslog(LOG_WARNING, "Failed to get device info using DRM:
> %s. Using no-DRM fallback.",
> + e.what());
> + return get_device_display_info_no_drm(dpy);
> + }
> +}
> +
> FrameCapture *GstreamerPlugin::CreateCapture()
> {
> return new GstreamerFrameCapture(settings);
> diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp
> index 8081007..09f3769 100644
> --- a/src/mjpeg-fallback.cpp
> +++ b/src/mjpeg-fallback.cpp
> @@ -7,6 +7,9 @@
> #include <config.h>
> #include "mjpeg-fallback.hpp"
>
> +#include "jpeg.hpp"
> +#include <spice-streaming-agent/x11-display-info.hpp>
> +
> #include <cstring>
> #include <exception>
> #include <stdexcept>
> @@ -15,8 +18,6 @@
> #include <syslog.h>
> #include <X11/Xlib.h>
>
> -#include "jpeg.hpp"
> -
> using namespace spice::streaming_agent;
>
> static inline uint64_t get_time()
> @@ -40,6 +41,7 @@ public:
> SpiceVideoCodecType VideoCodecType() const override {
> return SPICE_VIDEO_CODEC_TYPE_MJPEG;
> }
> + std::vector<DeviceDisplayInfo> get_device_display_info() const
> override;
> private:
> MjpegSettings settings;
> Display *dpy;
> @@ -137,6 +139,17 @@ FrameInfo MjpegFrameCapture::CaptureFrame()
> return info;
> }
>
> +std::vector<DeviceDisplayInfo>
> MjpegFrameCapture::get_device_display_info() const
> +{
> + try {
> + return get_device_display_info_drm(dpy);
> + } catch (const std::exception &e) {
> + syslog(LOG_WARNING, "Failed to get device info using DRM:
> %s. Using no-DRM fallback.",
> + e.what());
> + return get_device_display_info_no_drm(dpy);
> + }
> +}
> +
> FrameCapture *MjpegPlugin::CreateCapture()
> {
> return new MjpegFrameCapture(settings);
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-
> agent.cpp
> index 1eb8e1b..3024d98 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -217,6 +217,19 @@ do_capture(StreamPort &stream_port, FrameLog
> &frame_log)
> throw std::runtime_error("cannot find a suitable capture
> system");
> }
>
> + try {
> + std::vector<DeviceDisplayInfo> display_info = capture-
> >get_device_display_info();
> + syslog(LOG_DEBUG, "Got device info of %lu devices from
> the plugin", display_info.size());
> + for (const auto &info : display_info) {
> + syslog(LOG_DEBUG, " id %u: device address %s,
> device display id: %u",
> + info.id,
> + info.device_address.c_str(),
> + info.device_display_id);
> + }
> + } catch (const Error &e) {
> + syslog(LOG_ERR, "Error while getting device info: %s",
> e.what());
> + }
> +
> while (!quit_requested && streaming_requested) {
> if (++frame_count % 100 == 0) {
> syslog(LOG_DEBUG, "SENT %d frames", frame_count);
> diff --git a/src/unittests/Makefile.am b/src/unittests/Makefile.am
> index 1ae5a07..8ce1f7a 100644
> --- a/src/unittests/Makefile.am
> +++ b/src/unittests/Makefile.am
> @@ -5,6 +5,7 @@ AM_CPPFLAGS = \
> -I$(top_srcdir)/include \
> -I$(top_srcdir)/src \
> -I$(top_srcdir)/src/unittests \
> + $(DRM_CFLAGS) \
> $(SPICE_PROTOCOL_CFLAGS) \
> $(NULL)
>
> @@ -43,13 +44,18 @@ hexdump_LDADD = \
>
> test_mjpeg_fallback_SOURCES = \
> test-mjpeg-fallback.cpp \
> + ../display-info.cpp \
> ../jpeg.cpp \
> ../mjpeg-fallback.cpp \
> + ../utils.cpp \
> + ../x11-display-info.cpp \
> $(NULL)
>
> test_mjpeg_fallback_LDADD = \
> + $(DRM_LIBS) \
> $(X11_LIBS) \
> $(JPEG_LIBS) \
> + $(XRANDR_LIBS) \
> $(NULL)
>
> test_stream_port_SOURCES = \
> diff --git a/src/utils.cpp b/src/utils.cpp
> new file mode 100644
> index 0000000..b78c893
> --- /dev/null
> +++ b/src/utils.cpp
> @@ -0,0 +1,46 @@
> +/* \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include "utils.hpp"
> +
> +#include <spice-streaming-agent/error.hpp>
> +
> +#include <glob.h>
> +#include <string.h>
> +#include <stdexcept>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +namespace utils {
> +
> +std::vector<std::string> glob(const std::string& pattern)
> +{
> + glob_t glob_result{};
> +
> + std::vector<std::string> filenames;
> +
> + int ret = glob(pattern.c_str(), GLOB_ERR, NULL, &glob_result);
> +
> + if(ret != 0) {
> + globfree(&glob_result);
> +
> + if (ret == GLOB_NOMATCH) {
> + return filenames;
> + }
> +
> + throw Error("glob(" + pattern + ") failed with return value
> " + std::to_string(ret) +
> + ": " + strerror(errno));
> + }
> +
> + for(size_t i = 0; i < glob_result.gl_pathc; ++i) {
> + filenames.push_back(glob_result.gl_pathv[i]);
> + }
> +
> + globfree(&glob_result);
> +
> + return filenames;
> +}
> +
> +}}} // namespace spice::streaming_agent::utils
> diff --git a/src/utils.hpp b/src/utils.hpp
> index 64cb538..8c3b04e 100644
> --- a/src/utils.hpp
> +++ b/src/utils.hpp
> @@ -5,6 +5,8 @@
> #ifndef SPICE_STREAMING_AGENT_UTILS_HPP
> #define SPICE_STREAMING_AGENT_UTILS_HPP
>
> +#include <vector>
> +#include <string>
> #include <syslog.h>
>
>
> @@ -12,6 +14,8 @@ namespace spice {
> namespace streaming_agent {
> namespace utils {
>
> +std::vector<std::string> glob(const std::string& pattern);
> +
> template<class T>
> const T &syslog(const T &error) noexcept
> {
> diff --git a/src/x11-display-info.cpp b/src/x11-display-info.cpp
> new file mode 100644
> index 0000000..3890886
> --- /dev/null
> +++ b/src/x11-display-info.cpp
> @@ -0,0 +1,302 @@
> +/* \copyright
> + * Copyright 2018 Red Hat Inc. All rights reserved.
> + */
> +
> +#include <spice-streaming-agent/x11-display-info.hpp>
> +
> +#include <spice-streaming-agent/display-info.hpp>
> +#include "utils.hpp"
> +#include <spice-streaming-agent/error.hpp>
> +
> +#include <algorithm>
> +#include <cstring>
> +#include <map>
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <X11/extensions/Xrandr.h>
> +#include <xf86drm.h>
> +#include <xf86drmMode.h>
> +
> +
> +namespace spice {
> +namespace streaming_agent {
> +
> +namespace {
> +
> +constexpr uint32_t pci_vendor_id_redhat = 0x1b36;
> +constexpr uint32_t pci_device_id_qxl = 0x0100;
> +
> +struct OutputInfo {
> + std::string output_name;
> + std::string card_path;
> + uint32_t card_vendor_id;
> + uint32_t card_device_id;
> + uint32_t device_display_id;
> +};
> +
> +static const std::map<uint32_t, std::string>
> modesetting_output_names = {
> + {DRM_MODE_CONNECTOR_Unknown, "None"},
> + {DRM_MODE_CONNECTOR_VGA, "VGA"},
> + {DRM_MODE_CONNECTOR_DVII, "DVI-I"},
> + {DRM_MODE_CONNECTOR_DVID, "DVI-D"},
> + {DRM_MODE_CONNECTOR_DVIA, "DVI-A"},
> + {DRM_MODE_CONNECTOR_Composite, "Composite"},
> + {DRM_MODE_CONNECTOR_SVIDEO, "SVIDEO"},
> + {DRM_MODE_CONNECTOR_LVDS, "LVDS"},
> + {DRM_MODE_CONNECTOR_Component, "Component"},
> + {DRM_MODE_CONNECTOR_9PinDIN, "DIN"},
> + {DRM_MODE_CONNECTOR_DisplayPort, "DP"},
> + {DRM_MODE_CONNECTOR_HDMIA, "HDMI"},
> + {DRM_MODE_CONNECTOR_HDMIB, "HDMI-B"},
> + {DRM_MODE_CONNECTOR_TV, "TV"},
> + {DRM_MODE_CONNECTOR_eDP, "eDP"},
> + {DRM_MODE_CONNECTOR_VIRTUAL, "Virtual"},
> + {DRM_MODE_CONNECTOR_DSI, "DSI"},
> + {DRM_MODE_CONNECTOR_DPI, "DPI"},
> +};
> +
> +// Connector type names from qxl driver
> +static const std::map<uint32_t, std::string> qxl_output_names = {
> + {DRM_MODE_CONNECTOR_Unknown, "None"},
> + {DRM_MODE_CONNECTOR_VGA, "VGA"},
> + {DRM_MODE_CONNECTOR_DVII, "DVI"},
> + {DRM_MODE_CONNECTOR_DVID, "DVI"},
> + {DRM_MODE_CONNECTOR_DVIA, "DVI"},
> + {DRM_MODE_CONNECTOR_Composite, "Composite"},
> + {DRM_MODE_CONNECTOR_SVIDEO, "S-video"},
> + {DRM_MODE_CONNECTOR_LVDS, "LVDS"},
> + {DRM_MODE_CONNECTOR_Component, "CTV"},
> + {DRM_MODE_CONNECTOR_9PinDIN, "DIN"},
> + {DRM_MODE_CONNECTOR_DisplayPort, "DisplayPort"},
> + {DRM_MODE_CONNECTOR_HDMIA, "HDMI"},
> + {DRM_MODE_CONNECTOR_HDMIB, "HDMI"},
> + {DRM_MODE_CONNECTOR_TV, "TV"},
> + {DRM_MODE_CONNECTOR_eDP, "eDP"},
> + {DRM_MODE_CONNECTOR_VIRTUAL, "Virtual"},
> +};
> +
> +std::string get_drm_name(const std::map<uint32_t, std::string>
> &name_map,
> + uint32_t connector_type,
> + uint32_t connector_type_id)
> +{
> + auto name_it = name_map.find(connector_type);
> + if (name_it == name_map.end()) {
> + throw Error("Could not find DRM connector name for type " +
> std::to_string(connector_type));
> + }
> +
> + return name_it->second + "-" +
> std::to_string(connector_type_id);
> +}
> +
> +class DrmOutputGetter
> +{
> +public:
> + DrmOutputGetter(const char* card_path)
> + {
> + drm_fd = open(card_path, O_RDWR);
> + if (drm_fd < 0) {
> + throw Error(std::string("Unable to open file %s: ") +
> strerror(errno));
> + }
> +
> + drm_resources = drmModeGetResources(drm_fd);
> +
> + if (drm_resources == nullptr) {
> + close(drm_fd);
> + throw Error(std::string("Unable to get DRM resources for
> card ") + card_path);
> + }
> + }
> +
> + ~DrmOutputGetter()
> + {
> + drmModeFreeResources(drm_resources);
> + close(drm_fd);
> + }
> +
> + std::vector<std::string> get_output_names(const
> std::map<uint32_t, std::string> &names_map,
> + bool decrement =
> false)
> + {
> + std::vector<std::string> result;
> +
> + for (size_t i = 0; i < drm_resources->count_connectors; ++i)
> {
> + drmModeConnectorPtr conn = drmModeGetConnector(drm_fd,
> drm_resources->connectors[i]);
> +
> + if (conn == nullptr) {
> + throw Error(std::string("Unable to get DRM
> connector: ") + strerror(errno));
> + }
> +
> + uint32_t cti = conn->connector_type_id;
> + if (decrement) {
> + cti--;
> + }
> + result.push_back(get_drm_name(names_map, conn-
> >connector_type, cti));
> + }
> +
> + return result;
> + }
> +
> +private:
> + int drm_fd = -1;
> + drmModeResPtr drm_resources = nullptr;
> +};
> +
> +std::vector<OutputInfo> get_outputs()
> +{
> + std::vector<OutputInfo> result;
> +
> + for (uint8_t card_id = 0; card_id < 10; ++card_id) {
> + std::string card_name = "card" + std::to_string(card_id);
> +
> + char drm_path[64];
> + snprintf(drm_path, sizeof(drm_path), DRM_DEV_NAME,
> DRM_DIR_NAME, card_id);
> +
> + struct stat stat_buf;
> + if (stat(drm_path, &stat_buf) != 0) {
> + if (errno == ENOENT) {
> + // we're done searching
> + break;
> + }
> + throw Error(std::string("Error accessing DRM node for
> card ") + drm_path);
> + }
> +
> + std::string sys_path = "/sys/class/drm/" + card_name;
> +
> + uint32_t vendor_id = read_hex_number_from_file(sys_path +
> "/device/vendor");
> + uint32_t device_id = read_hex_number_from_file(sys_path +
> "/device/device");
> +
> + std::vector<std::string> outputs;
> +
> + if (vendor_id == pci_vendor_id_redhat && device_id ==
> pci_device_id_qxl) {
> + // to find out whether QXL output names need
> decrementing, look for
> + // an output called Virtual-0 in /sys/class/drm and if
> we find one,
> + // decrement the number in the output names
> + const auto globs = utils::glob("/sys/class/drm/card*-
> Virtual-0");
> + outputs =
> DrmOutputGetter(drm_path).get_output_names(qxl_output_names,
> globs.size() > 0);
> + } else {
> + outputs =
> DrmOutputGetter(drm_path).get_output_names(modesetting_output_names);
> + }
> +
> + for (uint32_t i = 0; i < outputs.size(); ++i) {
> + result.push_back({outputs[i],
> + sys_path,
> + vendor_id,
> + device_id,
> + i});
> + }
> + }
> +
> + return result;
> +}
> +
> +} // namespace
> +
> +std::vector<std::string> get_xrandr_outputs(Display *display, Window
> window)
> +{
> + XRRScreenResources *screen_resources =
> XRRGetScreenResources(display, window);
> + std::vector<std::string> result;
> + for (int i = 0; i < screen_resources->noutput; ++i) {
> + XRROutputInfo *output_info = XRRGetOutputInfo(display,
> + screen_resourc
> es,
> + screen_resourc
> es->outputs[i]);
> +
> + result.emplace_back(output_info->name);
> +
> + XRRFreeOutputInfo(output_info);
> + }
> +
> + XRRFreeScreenResources(screen_resources);
> + return result;
> +}
> +
> +std::vector<DeviceDisplayInfo> get_device_display_info_drm(Display
> *display)
> +{
> + auto outputs = get_outputs();
> +
> + // sorting by output name, may not be necessary (the list should
> be sorted out of glob)
> + std::sort(outputs.begin(), outputs.end(), [](const OutputInfo
> &oi1, const OutputInfo &oi2) {
> + return oi1.output_name > oi2.output_name;
> + }
> + );
> +
> + // Check if the output names of QXL cards start at Virtual-1 and
> if so,
> + // subtract one from each of them
> + for (auto &output : outputs) {
> + // if we find a QXL card
> + if (output.card_vendor_id == pci_vendor_id_redhat &&
> + output.card_device_id == pci_device_id_qxl)
> + {
> + // if the first name is Virtual-0, we know we are good,
> quit the loop
> + if (output.output_name == "Virtual-0") {
> + break;
> + }
> +
> + // if the name starts with "Virtual-"
> + if (!output.output_name.compare(0, 8, "Virtual-")) {
I strongly prefer "foo.compare() == 0" over "!foo.compare()"
> + // decrement the last digit by one
> + // fails if the number at the end is more than one
> digit,
> + // which would imply multiple QXL devices, an
> unsupported case
> + output.output_name[output.output_name.length() - 1]-
> -;
> + }
> + }
> + }
> +
> + std::map<std::string, OutputInfo> output_map;
> + for (const auto &output : outputs) {
> + output_map[output.output_name] = output;
> + }
> +
> + auto xrandr_outputs = get_xrandr_outputs(display,
> RootWindow(display, XDefaultScreen(display)));
> +
> + std::vector<DeviceDisplayInfo> result;
> +
> + for (uint32_t i = 0; i < xrandr_outputs.size(); ++i) {
> + const auto &output = xrandr_outputs[i];
Since you have variables named 'outputs' and 'xrandr_outputs' in this
function, I'd personally prefer to name this variable something like
'xoutput' instead of simply 'output' for clarity.
> + const auto it = output_map.find(output);
> + if (it == output_map.end()) {
> + throw Error("Could not find card for output " + output);
> + }
> +
> + std::string device_address = get_device_address(it-
> >second.card_path);
> +
> + result.push_back({i, device_address, it-
> >second.device_display_id});
> + }
> +
> + return result;
> +}
> +
> +std::vector<DeviceDisplayInfo>
> get_device_display_info_no_drm(Display *display)
> +{
> + // look for the first card that doesn't list its outputs (and
> therefore
> + // doesn't support DRM), that's the best we can do...
> + const auto globs = utils::glob("/sys/class/drm/card*");
> +
> + std::string found_card_path;
> + for (const auto &path : globs) {
> + std::string card_output = path.substr(path.rfind("/") + 1,
> path.npos);
> + auto dash_it = card_output.find("-");
> + // if this file is a card (and not an output)
> + if (dash_it == card_output.npos) {
> + // if we already have a found card, this means it
> doesn't have any outputs,
> + // we have found our card
> + if (!found_card_path.empty()) {
> + break;
> + }
> + found_card_path = card_output;
> + } else {
> + found_card_path = "";
> + }
> + }
> +
> + std::string device_address =
> get_device_address(found_card_path);
> +
> + auto xrandr_outputs = get_xrandr_outputs(display,
> RootWindow(display, XDefaultScreen(display)));
> +
> + std::vector<DeviceDisplayInfo> result;
> +
> + for (uint32_t i = 0; i < xrandr_outputs.size(); ++i) {
> + result.push_back({i, device_address, i});
> + }
> +
> + return result;
> +}
> +
> +}} // namespace spice::streaming_agent
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel