GNOME Bugzilla – Bug 744816
VideoNode::setCurrentFrame() material assert fails after updating QStandardItemModel
Last modified: 2017-12-13 14:26:14 UTC
void VideoNode::setCurrentFrame(GstBuffer* buffer) { Q_ASSERT (m_materialType == MaterialTypeVideo); } fails after a QStandardItemModel::setData() is called on a role that should not affect the surface. The QStandardModel is simple: QHash<int,QByteArray> VideoSceneModel::roleNames() const { QHash<int, QByteArray> retVal; retVal[VideoSceneModelRole_openGLsurface] = "role_videoSceneModelOpenGLSurface"; retVal[VideoSceneModelRole_X] = "role_videoSceneModelX"; } I can set the x position manually in QML just fine. The video plays fine. However when we bind the x position in XML to the model role, updating the x position is triggering an event that causes the video node to be destroyed somewhere. i.e. // set X position from 0 to 500 QModelIndex modelIndex = index( 0, 0 ); if( modelIndex.isValid() ) { setData( modelIndex, 500, VideoSceneModelRole_X ); } Which is emitting some sort of change signal that ends up in: QSGNode* QtQuick2VideoSinkDelegate::updateNode(QSGNode *node, const QRectF & targetArea) GST_TRACE_OBJECT(m_sink, "updateNode called"); bool sgnodeFormatChanged = false; VideoNode *vnode = dynamic_cast<VideoNode*>(node); if (!vnode) { GST_INFO_OBJECT(m_sink, "creating new VideoNode"); vnode = new VideoNode; } vnode is null, causing a new VideoNode to be created. This in turn sets a black surface. Since the video is still playing, the next void VideoNode::setCurrentFrame(GstBuffer* buffer) { Q_ASSERT (m_materialType == MaterialTypeVideo); } fails. Interestingly, if you stick a breakpoint on g_signal_emitv(values, signal.id(), detail, &returnValue); in signal.cpp, the call sometimes work fine. The video changex to x=500 and everything keeps goign. this seems like it could be race condition/execution order related here is the assert failure on the material stack trace: 0 __GI_raise 56 0x7ffff4b42e37 1 __GI_abort 89 0x7ffff4b44528 2 QMessageLogger::fatal(char const*, ...) const /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff5486b76 3 qt_assert(char const*, char const*, int) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff54817ab 4 VideoNode::setCurrentFrame videonode.cpp 49 0x7fffd3270791 5 QtQuick2VideoSinkDelegate::updateNode qtquick2videosinkdelegate.cpp 98 0x7fffd3271c9f 6 gst_qt_quick2_video_sink_update_node gstqtquick2videosink.cpp 265 0x7fffd3272a32 7 g_cclosure_user_marshal_POINTER__POINTER_DOUBLE_DOUBLE_DOUBLE_DOUBLE gstqtvideosinkmarshal.c 221 0x7fffd326d5a2 8 g_closure_invoke /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0 0x7ffff432f285 9 ?? /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0 0x7ffff4340e62 10 g_signal_emitv /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0 0x7ffff4348541 11 QGlib::Private::emit signal.cpp 190 0x7ffff48ebac3 12 QGlib::Private::EmitImpl<void* (void*, double, double, double, double)>::emit(void*, char const*, QGlib::Quark, void* const&, double const&, double const&, double const&, double const&) emitimpl.h 78 0x7ffff79a1ae9 13 QGlib::emit<void*, void*, double, double, double, double> emitimpl.h 113 0x7ffff79a1659 14 QGst::Quick::VideoItem::updatePaintNode videoitem.cpp 113 0x7ffff79a0c26 15 QQuickWindowPrivate::updateDirtyNode(QQuickItem*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72ee3e6 16 QQuickWindowPrivate::updateDirtyNodes() /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72ef7c3 17 QQuickWindowPrivate::syncSceneGraph() /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72f0ba9 18 ?? /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72bedd5 19 ?? /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72bf4ad 20 QApplicationPrivate::notify_helper(QObject*, QEvent*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Widgets.so.5 0x7ffff6a228f4 21 QApplication::notify(QObject*, QEvent*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Widgets.so.5 0x7ffff6a26506 22 QCoreApplication::notifyInternal(QObject*, QEvent*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff56ccc84 23 QTimerInfoList::activateTimers() /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff57269c4 24 ?? /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff5726c6d 25 g_main_context_dispatch /lib/x86_64-linux-gnu/libglib-2.0.so.0 0x7ffff4059ecd 26 ?? /lib/x86_64-linux-gnu/libglib-2.0.so.0 0x7ffff405a1b0 27 g_main_context_iteration /lib/x86_64-linux-gnu/libglib-2.0.so.0 0x7ffff405a25c 28 QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff57275b3 29 QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff56caeab 30 QCoreApplication::exec() /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff56cfdc5 31 main main.cpp 72 0x405230 here is the stack trace that causes the material to be set to black after a new vnode is created (which is likely more useful): 0 VideoNode::setMaterialTypeSolidBlack videonode.cpp 40 0x7fffd32706a8 1 VideoNode::VideoNode videonode.cpp 28 0x7fffd3270617 2 QtQuick2VideoSinkDelegate::updateNode qtquick2videosinkdelegate.cpp 34 0x7fffd3271651 3 gst_qt_quick2_video_sink_update_node gstqtquick2videosink.cpp 265 0x7fffd3272a32 4 g_cclosure_user_marshal_POINTER__POINTER_DOUBLE_DOUBLE_DOUBLE_DOUBLE gstqtvideosinkmarshal.c 221 0x7fffd326d5a2 5 g_closure_invoke /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0 0x7ffff432f285 6 ?? /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0 0x7ffff4340e62 7 g_signal_emitv /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0 0x7ffff4348541 8 QGlib::Private::emit signal.cpp 190 0x7ffff48ebac3 9 QGlib::Private::EmitImpl<void* (void*, double, double, double, double)>::emit(void*, char const*, QGlib::Quark, void* const&, double const&, double const&, double const&, double const&) emitimpl.h 78 0x7ffff79a1ae9 10 QGlib::emit<void*, void*, double, double, double, double> emitimpl.h 113 0x7ffff79a1659 11 QGst::Quick::VideoItem::updatePaintNode videoitem.cpp 113 0x7ffff79a0c26 12 QQuickWindowPrivate::updateDirtyNode(QQuickItem*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72ee3e6 13 QQuickWindowPrivate::updateDirtyNodes() /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72ef7c3 14 QQuickWindowPrivate::syncSceneGraph() /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72f0ba9 15 ?? /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72bedd5 16 ?? /home/dev/Qt/5.4/gcc_64/lib/libQt5Quick.so.5 0x7ffff72bf4ad 17 QApplicationPrivate::notify_helper(QObject*, QEvent*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Widgets.so.5 0x7ffff6a228f4 18 QApplication::notify(QObject*, QEvent*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Widgets.so.5 0x7ffff6a26506 19 QCoreApplication::notifyInternal(QObject*, QEvent*) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff56ccc84 20 QTimerInfoList::activateTimers() /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff57269c4 21 ?? /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff5726c6d 22 g_main_context_dispatch /lib/x86_64-linux-gnu/libglib-2.0.so.0 0x7ffff4059ecd 23 ?? /lib/x86_64-linux-gnu/libglib-2.0.so.0 0x7ffff405a1b0 24 g_main_context_iteration /lib/x86_64-linux-gnu/libglib-2.0.so.0 0x7ffff405a25c 25 QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff57275b3 26 QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff56caeab 27 QCoreApplication::exec() /home/dev/Qt/5.4/gcc_64/lib/libQt5Core.so.5 0x7ffff56cfdc5 28 main main.cpp 72 0x405230
to clarify, the VideoSceneModelRole_openGLsurface role has a QGst::Quick::VideoSurface* in it.
"I can set the x position manually in QML just fine. The video plays fine. However when we bind the x position in XML to the model role, updating the x position is triggering an event that causes the video node to be destroyed somewhere. i.e." typo, I can set the x position manually in C++ just fine. When we bind x to a QML object, changing the x value triggers a dataChanged() event for the VideoSceneModelRole_openGLsurface role as well. If you manually supress signals in QStandardItemModel::setData(), i.e. blockSignals( true ); and emitDataChanged() manually without the VideoSceneModelRole_openGLsurface, we can work around the problem. To boil down the issue, the QGst::Quick::VideoSurface object (usually) crashes when it attempts to handle a dataChanged() event.
Created attachment 347541 [details] [review] Fix This fixes the crash, if somebody can accept the fix, that would be great. Also it will need a release eventually.
(In reply to Aleix Pol from comment #3) > Created attachment 347541 [details] [review] [review] > Fix > > This fixes the crash, if somebody can accept the fix, that would be great. > Also it will need a release eventually. Were you able to trigger the crash? Any chance you have a small program that triggers it? (AKA can we have a test for the bug, and then show the fix fixes it?)
Created attachment 347591 [details] [review] Version of the patch with autotest Now with a test case
The second patch looks the same as the first to me.
Created attachment 347615 [details] [review] Fix, patch with autotest You were right, silly git.
The tests didn't run correctly for me... But I think my test environment is broken. Will try to replicate over the weekend.
bump
I have made mistakes with git, and have confused myself. So I have two repositories, because at first I didn't have the permissions to, and then I didn't know the right policies to commit to the freedesktop repository So the other repository is at https://github.com/detrout/qt-gstreamer The have drifted a bit. What was the commit you based your fix on? did you do make test? how many of the tests passed for you?
I'm using git://anongit.freedesktop.org/gstreamer/qt-gstreamer commit 0384481b2f670b3db1a147a1ac0c5ea24c66554e Author: Diane Trout <diane@ghic.org> Date: Sat Mar 21 13:08:29 2015 -0700 gst_message_new_application fails when passed a NULL structure
bump?
That commit builds for you? I'm trying that commit and it won't build with out this patch. Well some of the tests wont build. I used cmake -DQT_VERSION=5 -DQTGSTREAMER_TESTS=ON I did see your test case fails at least some of the time without the fix and seems to work with the fix added. Diane index 2d66ba1..7ed2bdf 100644 --- a/src/QGst/message.h +++ b/src/QGst/message.h @@ -205,7 +205,7 @@ class QTGSTREAMER_EXPORT ApplicationMessage : public Message QGST_WRAPPER_FAKE_SUBCLASS(Application, Message) public: static ApplicationMessagePtr create(const ObjectPtr & source, - const Structure & structure); + const Structure & structure=Structure()); }; /*! \headerfile message.h <QGst/Message> @@ -216,7 +216,7 @@ class QTGSTREAMER_EXPORT ElementMessage : public Message QGST_WRAPPER_FAKE_SUBCLASS(Element, Message) public: static ElementMessagePtr create(const ObjectPtr & source, - const Structure & structure); + const Structure & structure=Structure()); }; //maybe do: SEGMENT_START (internal)
Correct. tests/auto/refpointertest.cpp won't build for me either.
Ok I pushed a fix for refpointer test, and now all the tests build I'm now trying to run the reproducer... I tend to do "mkdir build ; cmake -D<stuff> .." to build cmake projects in a separate directory so in build/tests/auto I had to symlink the test qml file for it to be found ln -s ../../../tests/auto/videoitemtest.qml Might it be possible to use the cmake source directory variable to add a path to find the qml file? I needed to install qml-module-qtquick2 and qml-module-qtquick2 to satisfy the qml file dependencies. Now I'm trying to figure out what the environment variable settings are necessary to get the test to run It looks like it works with this command when I'm in build/tests/auto: QML2_IMPORT_PATH=../../src/qml/quick2/ GST_PLUGIN_PATH=../../elements/gstqtvideosink/ ./qtquick2test ********* Start testing of QtQuick2Test ********* Config: Using QtTest library 5.7.1, Qt 5.7.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by GCC 6.3.0 20170124) PASS : QtQuick2Test::initTestCase() PASS : QtQuick2Test::testLaunch() PASS : QtQuick2Test::cleanupTestCase() Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 95ms ********* Finished testing of QtQuick2Test ********* And it fails when I remove the fix. unfortunately make test fails because some of the above variables aren't being set. Do you think you could figure out how to add them to the test runners environment?
Created attachment 348649 [details] [review] New version of the patch that doesn't require an install There you go :)
Thanks that worked great!. commit ea0d273008316e875a2f6496c0f5f3fa1fcfb6b8 Author: Aleix Pol <aleixpol@kde.org> Date: Fri Mar 10 01:50:33 2017 +0100 Fix crash when the VideoItem moves in the SceneGraph When the item is recreated it should set the format on the new node, otherwise we get the crash. BUG: 744816 Since you're still using it what do you think about renaming QGst/Memory to QGst/QGstMemory? its a problem with case sensitive file systems described in bug 763201
We don't use QGst/Memory, so that wouldn't be an issue for us.
Is it possible to get a release with this fix in? it's been over half a year...
QtGStreamer is basically unmaintained. If someone wants to take up maintainership of it, please let us know.