Review (part 4)
-
Missing include #include <pipewire/pipewire.h>
→ can be fixed withinclude_directories(${PipeWire_INCLUDE_DIRS})
-
open("/dev/dri/renderD128", O_RDWR)
→ does KWin always use this render node? I know I had to go through EGL to get same device used by the compositor in WebRTC using EglGetPlatformDisplay to import DMA-BUFs with same device they were produced. -
Shouldn't DMA_FORMAT_MOD_INVALID
be returned even if eglQueryDmaBufFormatsExt() call fails? At least that's what I do in WebRTC -
PipeWire version checks - I know usually PW server version = PW library version, but this is not necessarily true in case of Flatpak or similar -
You have to verify PW version (server, not client) to check whether you can use DMA-BUFs at all - has to be 0.3.24 -
You also want to check PW server version for SPA_POD_PROP_FLAG_DONT_FIXATE
-
-
You don't seem to be using DMA-BUF modifiers. Why do you query them if you always set plane.modifier = DRM_FORMAT_MOD_INVALID
. Wouldn't this fail if you settle on a certain modifier? Also, if you use a modifier and it fails to use it during import, there should be a way to re-negotiate stream and drop the modifier that you failed to use. NOTE: this requires PW server version 0.3.40. Reference in WebRTC. -
Shouldn't pw_context_connect()
use the node ID you get as a response from screencasting interface? -
Build failure:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp: In member function ‘void PipeWireRecordProduce::setupStream()’:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:315:66: error: taking address of temporary array
315 | qWarning() << "Could not allocate raw picture buffer" << av_err2str(ret);
| ^~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:321:55: error: taking address of temporary array
321 | qWarning() << "Could not open" << m_output << av_err2str(ret);
| ^~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:334:67: error: taking address of temporary array
334 | qWarning() << "Error occurred when passing the codec:" << av_err2str(ret);
| ^~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:340:64: error: taking address of temporary array
340 | qWarning() << "Error occurred when writing header:" << av_err2str(ret);
| ^~~~~~~~~~
In file included from /home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:34:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp: In member function ‘void PipeWireRecordProduce::updateTextureImage(const QImage&)’:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:461:64: error: taking address of temporary array
461 | qCDebug(PIPEWIRERECORD_LOGGING) << "sending frame" << i << av_ts2str(m_frame->m_avFrame->pts)
| ^~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:464:62: error: taking address of temporary array
464 | qCDebug(PIPEWIRERECORD_LOGGING) << "sent frames" << i << av_ts2str(m_frame->m_avFrame->pts) << t.elapsed();
| ^~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:466:64: error: taking address of temporary array
466 | qWarning() << "Error sending a frame for encoding:" << av_err2str(ret);
| ^~~~~~~~~~
In file included from /usr/include/qt5/QtCore/QLoggingCategory:1,
from /home/jgrulich/development/projects/kde/kpipewire/build/src/logging_record.h:6,
from /home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:10:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp: In function ‘void log_packet(const AVFormatContext*, const AVPacket*)’:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:479:13: error: taking address of temporary array
479 | av_ts2str(pkt->pts),
| ^~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:480:13: error: taking address of temporary array
480 | av_ts2timestr(pkt->pts, time_base),
| ^~~~~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:481:13: error: taking address of temporary array
481 | av_ts2str(pkt->dts),
| ^~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:482:13: error: taking address of temporary array
482 | av_ts2timestr(pkt->dts, time_base),
| ^~~~~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:483:13: error: taking address of temporary array
483 | av_ts2str(pkt->duration),
| ^~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:484:13: error: taking address of temporary array
484 | av_ts2timestr(pkt->duration, time_base),
| ^~~~~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp: In member function ‘virtual void PipeWireRecordWriteThread::run()’:
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:539:57: error: taking address of temporary array
539 | qWarning() << "Error encoding a frame: " << av_err2str(ret);
| ^~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:545:84: error: taking address of temporary array
545 | qCDebug(PIPEWIRERECORD_LOGGING) << "receiving packets" << i << m_active << av_ts2str(m_packet->pts) << (*m_avFormatContext->streams)->index;
| ^~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:551:67: error: taking address of temporary array
551 | qWarning() << "Error while writing output packet:" << av_err2str(ret);
| ^~~~~~~~~~
/home/jgrulich/development/projects/kde/kpipewire/src/pipewirerecord.cpp:557:52: error: taking address of temporary array
557 | qWarning() << "failed to write trailer" << av_err2str(ret);
All of my comments are based on my experience with WebRTC and don't necessarily mean I have correct implementation. I will happily fix stuff on my side if you prove me otherwise. Thanks.
Edited by Aleix Pol Gonzalez