[vlc-commits] [Git][videolan/vlc][master] qt: fix video surface initialization and de-initialization ordering issues
Steve Lhomme (@robUx4)
gitlab at videolan.org
Sat Aug 10 10:04:32 UTC 2024
Steve Lhomme pushed to branch master at VideoLAN / VLC
Commits:
f2b0471c by Fatih Uzunoglu at 2024-08-10T09:49:55+00:00
qt: fix video surface initialization and de-initialization ordering issues
I wanted to initialize video surface provider and
window handler within `commonSetupVoutWindow()`, since
it is where the video window is set and video surface
provider and window handler only makes sense when
there is a valid vout window. However, for some reason,
it did not work so it is a TODO for now.
Currently, since there are a lot of signal connections,
changes are reported even when there is no valid vout
window. For example, `CompositorVideo::onSurfacePositionChanged()`
may be called when there is no vout window or when it
is already invalidated.
- - - - -
6 changed files:
- modules/gui/qt/maininterface/compositor.cpp
- modules/gui/qt/maininterface/mainctx.hpp
- modules/gui/qt/maininterface/videosurface.cpp
- modules/gui/qt/maininterface/videosurface.hpp
- modules/gui/qt/player/qml/PIPPlayer.qml
- modules/gui/qt/player/qml/Player.qml
Changes:
=====================================
modules/gui/qt/maininterface/compositor.cpp
=====================================
@@ -200,12 +200,23 @@ void CompositorVideo::commonSetupVoutWindow(vlc_window_t* p_wnd, VoutDestroyCb d
p_wnd->sys = this;
p_wnd->ops = &ops;
p_wnd->info.has_double_click = true;
+
+ // These need to be connected here, since the compositor might not be ready when
+ // these signals are emitted. VOut window might not be set, or worse, compositor's
+ // internal preparations might not be completed yet:
+ connect(m_videoSurfaceProvider.get(), &VideoSurfaceProvider::surfacePositionChanged,
+ this, &CompositorVideo::onSurfacePositionChanged, Qt::UniqueConnection);
+ connect(m_videoSurfaceProvider.get(), &VideoSurfaceProvider::surfaceSizeChanged,
+ this, &CompositorVideo::onSurfaceSizeChanged, Qt::UniqueConnection);
}
void CompositorVideo::windowDestroy()
{
if (m_destroyCb)
m_destroyCb(m_wnd);
+
+ m_videoSurfaceProvider.reset();
+ m_videoWindowHandler.reset();
}
void CompositorVideo::windowResize(unsigned width, unsigned height)
@@ -252,10 +263,6 @@ bool CompositorVideo::commonGUICreateImpl(QWindow* window, CompositorVideo::Flag
if (flags & CompositorVideo::CAN_SHOW_PIP)
{
m_mainCtx->setCanShowVideoPIP(true);
- connect(m_videoSurfaceProvider.get(), &VideoSurfaceProvider::surfacePositionChanged,
- this, &CompositorVideo::onSurfacePositionChanged);
- connect(m_videoSurfaceProvider.get(), &VideoSurfaceProvider::surfaceSizeChanged,
- this, &CompositorVideo::onSurfaceSizeChanged);
}
if (flags & CompositorVideo::HAS_ACRYLIC)
{
@@ -316,8 +323,6 @@ void CompositorVideo::commonGUIDestroy()
void CompositorVideo::commonIntfDestroy()
{
- m_videoWindowHandler.reset();
- m_videoSurfaceProvider.reset();
unloadGUI();
}
=====================================
modules/gui/qt/maininterface/mainctx.hpp
=====================================
@@ -45,6 +45,7 @@ Q_MOC_INCLUDE( "dialogs/toolbar/controlbar_profile_model.hpp" )
Q_MOC_INCLUDE( "util/csdbuttonmodel.hpp" )
Q_MOC_INCLUDE( "playlist/playlist_controller.hpp" )
Q_MOC_INCLUDE( "maininterface/mainctx_submodels.hpp" )
+Q_MOC_INCLUDE( "maininterface/videosurface.hpp" )
class CSDButtonModel;
class QSettings;
@@ -127,6 +128,7 @@ class MainCtx : public QObject
Q_PROPERTY(bool useGlobalShortcuts READ getUseGlobalShortcuts WRITE setUseGlobalShortcuts NOTIFY useGlobalShortcutsChanged FINAL)
Q_PROPERTY(int maxVolume READ maxVolume NOTIFY maxVolumeChanged FINAL)
Q_PROPERTY(float safeArea READ safeArea NOTIFY safeAreaChanged FINAL)
+ Q_PROPERTY(VideoSurfaceProvider* videoSurfaceProvider READ getVideoSurfaceProvider WRITE setVideoSurfaceProvider NOTIFY hasEmbededVideoChanged FINAL)
Q_PROPERTY(CSDButtonModel *csdButtonModel READ csdButtonModel CONSTANT FINAL)
=====================================
modules/gui/qt/maininterface/videosurface.cpp
=====================================
@@ -209,18 +209,14 @@ VideoSurface::VideoSurface(QQuickItem* parent)
setAcceptHoverEvents(true);
setAcceptedMouseButtons(Qt::AllButtons);
setFlag(ItemAcceptsInputMethod, true);
- setFlag(ItemHasContents, true);
-}
-MainCtx* VideoSurface::getCtx()
-{
- return m_ctx;
-}
+ {
+ connect(this, &QQuickItem::widthChanged, this, &VideoSurface::updateSurfaceSize);
+ connect(this, &QQuickItem::heightChanged, this, &VideoSurface::updateSurfaceSize);
-void VideoSurface::setCtx(MainCtx* ctx)
-{
- m_ctx = ctx;
- emit ctxChanged(ctx);
+ connect(this, &QQuickItem::xChanged, this, &VideoSurface::updateSurfacePosition);
+ connect(this, &QQuickItem::yChanged, this, &VideoSurface::updateSurfacePosition);
+ }
}
int VideoSurface::qtMouseButton2VLC( Qt::MouseButton qtButton )
@@ -306,12 +302,6 @@ void VideoSurface::keyPressEvent(QKeyEvent* event)
event->ignore();
}
-void VideoSurface::geometryChange(const QRectF& newGeometry, const QRectF& oldGeometry)
-{
- QQuickItem::geometryChange(newGeometry, oldGeometry);
- onSurfaceSizeChanged();
-}
-
#if QT_CONFIG(wheelevent)
void VideoSurface::wheelEvent(QWheelEvent *event)
{
@@ -330,97 +320,75 @@ void VideoSurface::setCursorShape(Qt::CursorShape shape)
setCursor(shape);
}
-QSGNode*VideoSurface::updatePaintNode(QSGNode* oldNode, QQuickItem::UpdatePaintNodeData* data)
+void VideoSurface::updatePolish()
{
- const auto node = ViewBlockingRectangle::updatePaintNode(oldNode, data);
+ QQuickItem::updatePolish();
- if (m_provider == nullptr)
+ if (m_sizeDirty)
{
- if (m_ctx == nullptr)
- return node;
- m_provider = m_ctx->getVideoSurfaceProvider();
- if (!m_provider)
- return node;
-
- //forward signal to the provider
- connect(this, &VideoSurface::mouseMoved, m_provider, &VideoSurfaceProvider::onMouseMoved);
- connect(this, &VideoSurface::mousePressed, m_provider, &VideoSurfaceProvider::onMousePressed);
- connect(this, &VideoSurface::mouseDblClicked, m_provider, &VideoSurfaceProvider::onMouseDoubleClick);
- connect(this, &VideoSurface::mouseReleased, m_provider, &VideoSurfaceProvider::onMouseReleased);
- connect(this, &VideoSurface::mouseWheeled, m_provider, &VideoSurfaceProvider::onMouseWheeled);
- connect(this, &VideoSurface::keyPressed, m_provider, &VideoSurfaceProvider::onKeyPressed);
- connect(this, &VideoSurface::surfaceSizeChanged, m_provider, &VideoSurfaceProvider::onSurfaceSizeChanged);
- connect(this, &VideoSurface::surfacePositionChanged, m_provider, &VideoSurfaceProvider::surfacePositionChanged);
-
- connect(m_provider, &VideoSurfaceProvider::hasVideoEmbedChanged, this, &VideoSurface::onProviderVideoChanged);
-
+ emit surfaceSizeChanged(size() * window()->effectiveDevicePixelRatio());
+ m_sizeDirty = false;
}
- updatePositionAndSize();
- return node;
-}
-
-void VideoSurface::componentComplete()
-{
- ViewBlockingRectangle::componentComplete();
- connect(this, &QQuickItem::xChanged, this, &VideoSurface::onSurfacePositionChanged);
- connect(this, &QQuickItem::yChanged, this, &VideoSurface::onSurfacePositionChanged);
- connect(this, &QQuickItem::widthChanged, this, &VideoSurface::onSurfaceSizeChanged);
- connect(this, &QQuickItem::heightChanged, this, &VideoSurface::onSurfaceSizeChanged);
- connect(this, &VideoSurface::enabledChanged, this, &VideoSurface::updatePositionAndSize);
+ if (m_positionDirty)
+ {
+ QPointF scenePosition = this->mapToScene(QPointF(0,0));
- updatePositionAndSize();
+ emit surfacePositionChanged(scenePosition * window()->effectiveDevicePixelRatio());
+ m_positionDirty = false;
+ }
}
-void VideoSurface::onProviderVideoChanged(bool hasVideo)
+void VideoSurface::updateSurfacePosition()
{
- if (!hasVideo)
- return;
- updatePositionAndSize();
+ m_positionDirty = true;
+ polish();
}
-static qreal dprForWindow(QQuickWindow* quickWindow)
+void VideoSurface::updateSurfaceSize()
{
- if (!quickWindow)
- return 1.0;
-
- QWindow* window = QQuickRenderControl::renderWindowFor(quickWindow);
- if (!window)
- window = quickWindow;
-
- return window->devicePixelRatio();
+ m_sizeDirty = true;
+ polish();
}
-void VideoSurface::onSurfaceSizeChanged()
+void VideoSurface::updateSurfacePositionAndSize()
{
- if (!isEnabled())
- return;
-
- qreal dpr = dprForWindow(window());
-
- emit surfaceSizeChanged(size() * dpr);
+ updateSurfacePosition();
+ updateSurfaceSize();
}
-void VideoSurface::onSurfacePositionChanged()
+void VideoSurface::setVideoSurfaceProvider(VideoSurfaceProvider *newVideoSurfaceProvider)
{
- if (!isEnabled())
+ if (m_provider == newVideoSurfaceProvider)
return;
- qreal dpr = dprForWindow(window());
+ if (m_provider)
+ {
+ disconnect(m_provider, nullptr, this, nullptr);
+ }
- QPointF scenePosition = this->mapToScene(QPointF(0,0));
+ m_provider = newVideoSurfaceProvider;
- emit surfacePositionChanged(scenePosition * dpr);
-}
+ if (m_provider)
+ {
+ connect(this, &VideoSurface::mouseMoved, m_provider, &VideoSurfaceProvider::onMouseMoved);
+ connect(this, &VideoSurface::mousePressed, m_provider, &VideoSurfaceProvider::onMousePressed);
+ connect(this, &VideoSurface::mouseDblClicked, m_provider, &VideoSurfaceProvider::onMouseDoubleClick);
+ connect(this, &VideoSurface::mouseReleased, m_provider, &VideoSurfaceProvider::onMouseReleased);
+ connect(this, &VideoSurface::mouseWheeled, m_provider, &VideoSurfaceProvider::onMouseWheeled);
+ connect(this, &VideoSurface::keyPressed, m_provider, &VideoSurfaceProvider::onKeyPressed);
+ connect(this, &VideoSurface::surfaceSizeChanged, m_provider, &VideoSurfaceProvider::onSurfaceSizeChanged);
+ connect(this, &VideoSurface::surfacePositionChanged, m_provider, &VideoSurfaceProvider::surfacePositionChanged);
-void VideoSurface::updatePositionAndSize()
-{
- if (!isEnabled())
- return;
+ connect(m_provider, &VideoSurfaceProvider::videoEnabledChanged, this, &VideoSurface::updateSurfacePositionAndSize);
- qreal dpr = dprForWindow(window());
+ setFlag(ItemHasContents, true);
+ updateSurfacePositionAndSize(); // Polish is queued anyway, updatePolish() should be called when the initial size is set.
+ }
+ else
+ {
+ setFlag(ItemHasContents, false);
+ }
- emit surfaceSizeChanged(size() * dpr);
- QPointF scenePosition = this->mapToScene(QPointF(0, 0));
- emit surfacePositionChanged(scenePosition * dpr);
+ emit videoSurfaceProviderChanged();
}
=====================================
modules/gui/qt/maininterface/videosurface.hpp
=====================================
@@ -98,14 +98,14 @@ protected:
class VideoSurface : public ViewBlockingRectangle
{
Q_OBJECT
- Q_PROPERTY(MainCtx* ctx READ getCtx WRITE setCtx NOTIFY ctxChanged FINAL)
+ Q_PROPERTY(VideoSurfaceProvider* videoSurfaceProvider READ videoSurfaceProvider WRITE setVideoSurfaceProvider NOTIFY videoSurfaceProviderChanged FINAL)
Q_PROPERTY(Qt::CursorShape cursorShape READ getCursorShape WRITE setCursorShape RESET unsetCursor FINAL)
public:
VideoSurface( QQuickItem* parent = nullptr );
- MainCtx* getCtx();
- void setCtx(MainCtx* ctx);
+ VideoSurfaceProvider* videoSurfaceProvider() const { return m_provider; };
+ void setVideoSurfaceProvider(VideoSurfaceProvider *newVideoSurfaceProvider);
protected:
int qtMouseButton2VLC( Qt::MouseButton qtButton );
@@ -120,18 +120,12 @@ protected:
void wheelEvent(QWheelEvent *event) override;
#endif
- void geometryChange(const QRectF &newGeometry,
- const QRectF &oldGeometry) override;
-
Qt::CursorShape getCursorShape() const;
void setCursorShape(Qt::CursorShape);
- QSGNode* updatePaintNode(QSGNode *, QQuickItem::UpdatePaintNodeData *) override;
-
- void componentComplete() override;
+ void updatePolish() override;
signals:
- void ctxChanged(MainCtx*);
void surfaceSizeChanged(QSizeF);
void surfacePositionChanged(QPointF);
@@ -142,18 +136,20 @@ signals:
void keyPressed(int key, Qt::KeyboardModifiers modifier);
void mouseWheeled(const QWheelEvent& event);
+ void videoSurfaceProviderChanged();
+
protected slots:
- void onProviderVideoChanged(bool);
- void onSurfaceSizeChanged();
- void onSurfacePositionChanged();
- void updatePositionAndSize();
+ void updateSurfacePosition();
+ void updateSurfaceSize();
+ void updateSurfacePositionAndSize();
private:
- MainCtx* m_ctx = nullptr;
-
QPointF m_oldHoverPos;
QPointer<VideoSurfaceProvider> m_provider;
+
+ bool m_sizeDirty = false;
+ bool m_positionDirty = false;
};
#endif // VIDEOSURFACE_HPP
=====================================
modules/gui/qt/player/qml/PIPPlayer.qml
=====================================
@@ -31,8 +31,8 @@ T.Control {
height: VLCStyle.dp(180, VLCStyle.scale)
//VideoSurface x,y won't update
- onXChanged: videoSurface.onSurfacePositionChanged()
- onYChanged: videoSurface.onSurfacePositionChanged()
+ onXChanged: videoSurface.updateSurfacePosition()
+ onYChanged: videoSurface.updateSurfacePosition()
objectName: "pip window"
@@ -93,7 +93,7 @@ T.Control {
background: VideoSurface {
id: videoSurface
- ctx: MainCtx
+ videoSurfaceProvider: MainCtx.videoSurfaceProvider
}
contentItem: Rectangle {
=====================================
modules/gui/qt/player/qml/Player.qml
=====================================
@@ -191,9 +191,10 @@ FocusScope {
VideoSurface {
id: videoSurface
- ctx: MainCtx
+ videoSurfaceProvider: MainCtx.videoSurfaceProvider
+
visible: rootPlayer.hasEmbededVideo
- enabled: rootPlayer.hasEmbededVideo
+
anchors.fill: parent
anchors.topMargin: rootPlayer._controlsUnderVideo ? topBar.height : 0
anchors.bottomMargin: rootPlayer._controlsUnderVideo ? controlBar.height : 0
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/f2b0471cb9e27b18fb525f665b63011223e9bdec
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/f2b0471cb9e27b18fb525f665b63011223e9bdec
You're receiving this email because of your account on code.videolan.org.
VideoLAN code repository instance
More information about the vlc-commits
mailing list