[neon/extras/phonon-backend-vlc/Neon/release] debian/patches: import code changes from 0.11 source branch
Harald Sitter
null at kde.org
Tue Mar 23 11:03:08 GMT 2021
Git commit b1e79fa776ea7fda15d3db9e4717b909530da711 by Harald Sitter.
Committed on 23/03/2021 at 11:01.
Pushed by sitter into branch 'Neon/release'.
import code changes from 0.11 source branch
A +59 -0 debian/patches/0001-fix-surface-painting-format-setup.patch
D +0 -37 debian/patches/0001-line-calc-variants.patch
A +110 -0 debian/patches/0002-stop-using-our-helper-function-to-calculate-pitch-an.patch
M +2 -1 debian/patches/series
https://invent.kde.org/neon/extras/phonon-backend-vlc/commit/b1e79fa776ea7fda15d3db9e4717b909530da711
diff --git a/debian/patches/0001-fix-surface-painting-format-setup.patch b/debian/patches/0001-fix-surface-painting-format-setup.patch
new file mode 100644
index 0000000..3777506
--- /dev/null
+++ b/debian/patches/0001-fix-surface-painting-format-setup.patch
@@ -0,0 +1,59 @@
+From f263063d1a9c748f92fa8cb021d8d8a85bbace42 Mon Sep 17 00:00:00 2001
+From: Harald Sitter <sitter at kde.org>
+Date: Fri, 19 Mar 2021 16:06:19 +0100
+Subject: [PATCH 1/2] fix surface painting format setup
+
+originally the format callback was not locked, but since we intercept
+qwidget paints that meant we'd be able to receive paints before the
+callback/setup had run. this could then result in a race condition where
+if the user was quick enough, or supposedly the system under sufficient
+load, that paints would be done on random blobs of invalid data. in
+particular when the paint would happen right as the other thread was in
+the format setup callback.
+
+to deal with this simply lock the format callback function and ensure we
+only paint when the frame has been constructed with actual data (frame
+is non-null after the callback since we feed it the plane as data,
+making it non-null)
+
+BUG: 434506
+---
+ src/video/videowidget.cpp | 8 +++++++-
+ 1 file changed, 7 insertions(+), 1 deletion(-)
+
+diff --git a/src/video/videowidget.cpp b/src/video/videowidget.cpp
+index bc03f8e..25543fa 100644
+--- a/src/video/videowidget.cpp
++++ b/src/video/videowidget.cpp
+@@ -3,7 +3,7 @@
+ Copyright (C) 2008 Lukas Durfina <lukas.durfina at gmail.com>
+ Copyright (C) 2009 Fathi Boudra <fabo at kde.org>
+ Copyright (C) 2009-2011 vlc-phonon AUTHORS <kde-multimedia at kde.org>
+- Copyright (C) 2011-2019 Harald Sitter <sitter at kde.org>
++ Copyright (C) 2011-2021 Harald Sitter <sitter at kde.org>
+
+ This library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+@@ -48,6 +48,11 @@ public:
+ // Plus VLC can actually skip frames as necessary.
+ QMutexLocker lock(&m_mutex);
+ Q_UNUSED(event);
++
++ if (m_frame.isNull()) {
++ return;
++ }
++
+ QPainter painter(widget);
+ // When using OpenGL for the QPaintEngine drawing the same QImage twice
+ // does not actually result in a texture change for one reason or another.
+@@ -90,6 +95,7 @@ private:
+ unsigned *pitches,
+ unsigned *lines)
+ {
++ QMutexLocker lock(&m_mutex);
+ qstrcpy(chroma, "RV32");
+ unsigned bufferSize = setPitchAndLines(vlc_fourcc_GetChromaDescription(VLC_CODEC_RGB32),
+ *width, *height,
+--
+2.25.1
+
diff --git a/debian/patches/0001-line-calc-variants.patch b/debian/patches/0001-line-calc-variants.patch
deleted file mode 100644
index 92e5d72..0000000
--- a/debian/patches/0001-line-calc-variants.patch
+++ /dev/null
@@ -1,37 +0,0 @@
-From dcfb93446b5d8d508d079f99db6832bcef2f7b5e Mon Sep 17 00:00:00 2001
-From: Harald Sitter <sitter at kde.org>
-Date: Fri, 19 Mar 2021 16:06:19 +0100
-Subject: [PATCH] line calc variants
-
-temporary patch
----
- src/video/videowidget.cpp | 6 ++++++
- 1 file changed, 6 insertions(+)
-
-diff --git a/src/video/videowidget.cpp b/src/video/videowidget.cpp
-index bc03f8e..0616e19 100644
---- a/src/video/videowidget.cpp
-+++ b/src/video/videowidget.cpp
-@@ -48,6 +48,11 @@ public:
- // Plus VLC can actually skip frames as necessary.
- QMutexLocker lock(&m_mutex);
- Q_UNUSED(event);
-+
-+ if (m_plane.isEmpty() || m_frame.isNull()) {
-+ return;
-+ }
-+
- QPainter painter(widget);
- // When using OpenGL for the QPaintEngine drawing the same QImage twice
- // does not actually result in a texture change for one reason or another.
-@@ -90,6 +95,7 @@ private:
- unsigned *pitches,
- unsigned *lines)
- {
-+ QMutexLocker lock(&m_mutex);
- qstrcpy(chroma, "RV32");
- unsigned bufferSize = setPitchAndLines(vlc_fourcc_GetChromaDescription(VLC_CODEC_RGB32),
- *width, *height,
---
-2.25.1
-
diff --git a/debian/patches/0002-stop-using-our-helper-function-to-calculate-pitch-an.patch b/debian/patches/0002-stop-using-our-helper-function-to-calculate-pitch-an.patch
new file mode 100644
index 0000000..09d8ac9
--- /dev/null
+++ b/debian/patches/0002-stop-using-our-helper-function-to-calculate-pitch-an.patch
@@ -0,0 +1,110 @@
+From 1fe55de0df172829722bab864f05351f244c116e Mon Sep 17 00:00:00 2001
+From: Harald Sitter <sitter at kde.org>
+Date: Tue, 23 Mar 2021 11:19:44 +0100
+Subject: [PATCH 2/2] stop using our helper function to calculate pitch and
+ lines
+
+the surface painter was more inefficient than it needed to be by
+manually calculating a pitch and stride but then constructing a qimage
+anyway. after some research it seems that we can simply construct our
+qimage and pass its picture values to VLC with the assumption that VLC
+will adjust the data as it gets passed into vmem output (i.e. our data
+blob). this then also allow us to get rid of the intermediate plane
+bytearray and simply use the memory the qimage allocates anyway.
+
+CCBUG: 434506
+---
+ src/video/videowidget.cpp | 48 +++++++++++++++++++++++++++------------
+ 1 file changed, 33 insertions(+), 15 deletions(-)
+
+diff --git a/src/video/videowidget.cpp b/src/video/videowidget.cpp
+index 25543fa..35d9025 100644
+--- a/src/video/videowidget.cpp
++++ b/src/video/videowidget.cpp
+@@ -59,10 +59,9 @@ public:
+ // So we simply create new images for every event. This is plenty cheap
+ // as the QImage only points to the plane data (it can't even make it
+ // properly shared as it does not know that the data belongs to a QBA).
+- painter.drawImage(drawFrameRect(),
+- QImage(reinterpret_cast<const uchar *>(m_plane.constData()),
+- m_frame.width(), m_frame.height(),
+- m_frame.bytesPerLine(), m_frame.format()));
++ // TODO: investigate if this is still necessary. This was added for gwenview, but with Qt 5.15 the problem
++ // can't be produced.
++ painter.drawImage(drawFrameRect(), QImage(m_frame));
+ event->accept();
+ }
+
+@@ -72,7 +71,7 @@ private:
+ virtual void *lockCallback(void **planes)
+ {
+ m_mutex.lock();
+- planes[0] = (void *) m_plane.data();
++ planes[0] = (void *) m_frame.bits();
+ return 0;
+ }
+
+@@ -96,21 +95,41 @@ private:
+ unsigned *lines)
+ {
+ QMutexLocker lock(&m_mutex);
++ // Surface rendering is a fallback system used when no efficient rendering implementation is available.
++ // As such we only support RGB32 for simplicity reasons and this will almost always mean software scaling.
++ // And since scaling is unavoidable anyway we take the canonical frame size and then scale it on our end via
++ // QPainter, again, greater simplicity at likely no real extra cost since this is all super inefficient anyway.
++ // Also, since aspect ratio can be change mid-playback by the user, doing the scaling on our end means we
++ // don't need to restart the entire player to retrigger format calculation.
++ // With all that in mind we simply use the canonical size and feed VLC the QImage's pitch and lines as
++ // effectively the VLC vout is the QImage so its constraints matter.
++
++ // per https://wiki.videolan.org/Hacker_Guide/Video_Filters/#Pitch.2C_visible_pitch.2C_planes_et_al.
++ // it would seem that we can use either real or visible pitches and lines as VLC generally will iterate the
++ // smallest value when moving data between two entities. i.e. since QImage will at most paint NxM anyway,
++ // we may just go with its values as calculating the real pitch/line of the VLC picture_t for RV32 wouldn't
++ // change the maximum pitch/lines we can paint on the output side.
++
+ qstrcpy(chroma, "RV32");
+- unsigned bufferSize = setPitchAndLines(vlc_fourcc_GetChromaDescription(VLC_CODEC_RGB32),
+- *width, *height,
+- pitches, lines);
+- m_plane.resize(bufferSize);
+- m_frame = QImage(reinterpret_cast<const uchar *>(m_plane.constData()),
+- *width, *height, pitches[0], QImage::Format_RGB32);
+- return bufferSize;
++ m_frame = QImage(*width, *height, QImage::Format_RGB32);
++ Q_ASSERT(!m_frame.isNull()); // ctor may construct null if allocation fails
++ m_frame.fill(0);
++ pitches[0] = m_frame.bytesPerLine();
++ lines[0] = m_frame.sizeInBytes() / m_frame.bytesPerLine();
++
++ return m_frame.sizeInBytes();
+ }
+
+ virtual void formatCleanUpCallback()
+ {
+ // Lazy delete the object to avoid callbacks from VLC after deletion.
+- if (!widget)
++ if (!widget) {
++ // The widget member is set to null by the widget destructor, so when this condition is true the
++ // widget had already been destroyed and we can't possibly receive a paint event anymore, meaning
++ // we need no lock here. If it were any other way we'd have trouble with synchronizing deletion
++ // without deleting a locked mutex.
+ delete this;
++ }
+ }
+
+ QRect scaleToAspect(QRect srcRect, int w, int h) const
+@@ -171,9 +190,8 @@ private:
+ return drawFrameRect;
+ }
+
++ // Could ReadWriteLock two frames so VLC can write while we paint.
+ QImage m_frame;
+- // We need an idependent plane as QImage needs to be forced to use the right stride/pitch.
+- QByteArray m_plane;
+ QMutex m_mutex;
+ };
+
+--
+2.25.1
+
diff --git a/debian/patches/series b/debian/patches/series
index aa6f55a..bc9cc47 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1,2 @@
-0001-line-calc-variants.patch
+0001-fix-surface-painting-format-setup.patch
+0002-stop-using-our-helper-function-to-calculate-pitch-an.patch
More information about the Neon-commits
mailing list