Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of selected bugfixes from the 1.18.6 release to fix the "Flicker" problem #2

Open
wants to merge 6 commits into
base: qgroundcontrol
Choose a base branch
from
Open

Backport of selected bugfixes from the 1.18.6 release to fix the "Flicker" problem #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
291d401
qtitem: fix leak of caps
ystreet Feb 3, 2020
1db83ab
qmlglsink: propagate the context up the the application
ystreet Feb 4, 2020
bb5f86d
qt: don't always activate/deactivate our GstGLContext
ystreet Feb 25, 2020
7fd5ed2
qmlglsink: fix crash when created/destroyed in quick succession
breboulet Oct 16, 2020
908d21a
qtitem: don't potentially leak a large number of buffers
ystreet Jun 3, 2021
074cb98
qmlglsink: Keep old buffers around a bit longer if they were bound by…
sdroege Nov 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions ext/qt/gstqsgtexture.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ GstQSGTexture::GstQSGTexture ()

gst_video_info_init (&this->v_info);
this->buffer_ = NULL;
this->buffer_was_bound = FALSE;
this->qt_context_ = NULL;
this->sync_buffer_ = gst_buffer_new ();
this->dummy_tex_id_ = 0;
Expand All @@ -56,6 +57,7 @@ GstQSGTexture::~GstQSGTexture ()
{
gst_buffer_replace (&this->buffer_, NULL);
gst_buffer_replace (&this->sync_buffer_, NULL);
this->buffer_was_bound = FALSE;
if (this->dummy_tex_id_ && QOpenGLContext::currentContext ()) {
QOpenGLContext::currentContext ()->functions ()->glDeleteTextures (1,
&this->dummy_tex_id_);
Expand All @@ -80,11 +82,26 @@ GstQSGTexture::setBuffer (GstBuffer * buffer)
if (!gst_buffer_replace (&this->buffer_, buffer))
return FALSE;

this->buffer_was_bound = FALSE;
this->qt_context_ = gst_gl_context_get_current ();

return TRUE;
}

/* only called from the streaming thread with scene graph thread blocked */
GstBuffer *
GstQSGTexture::getBuffer (gboolean * was_bound)
{
GstBuffer *buffer = NULL;

if (this->buffer_)
buffer = gst_buffer_ref (this->buffer_);
if (was_bound)
*was_bound = this->buffer_was_bound;

return buffer;
}

/* only called from qt's scene graph render thread */
void
GstQSGTexture::bind ()
Expand All @@ -99,8 +116,6 @@ GstQSGTexture::bind ()
if (!this->qt_context_)
return;

gst_gl_context_activate (this->qt_context_, TRUE);

if (!this->buffer_)
goto out;
if (GST_VIDEO_INFO_FORMAT (&this->v_info) == GST_VIDEO_FORMAT_UNKNOWN)
Expand Down Expand Up @@ -144,6 +159,8 @@ GstQSGTexture::bind ()
* to use the dummy texture */
use_dummy_tex = FALSE;

this->buffer_was_bound = TRUE;

out:
if (G_UNLIKELY (use_dummy_tex)) {
QOpenGLContext *qglcontext = QOpenGLContext::currentContext ();
Expand Down Expand Up @@ -174,8 +191,6 @@ GstQSGTexture::bind ()

funcs->glBindTexture (GL_TEXTURE_2D, this->dummy_tex_id_);
}

gst_gl_context_activate (this->qt_context_, FALSE);
}

/* can be called from any thread */
Expand Down
2 changes: 2 additions & 0 deletions ext/qt/gstqsgtexture.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class GstQSGTexture : public QSGTexture, protected QOpenGLFunctions

void setCaps (GstCaps * caps);
gboolean setBuffer (GstBuffer * buffer);
GstBuffer * getBuffer (gboolean * was_bound);

/* QSGTexture */
void bind ();
Expand All @@ -48,6 +49,7 @@ class GstQSGTexture : public QSGTexture, protected QOpenGLFunctions

private:
GstBuffer * buffer_;
gboolean buffer_was_bound;
GstBuffer * sync_buffer_;
GstGLContext * qt_context_;
GstMemory * mem_;
Expand Down
7 changes: 7 additions & 0 deletions ext/qt/gstqtsink.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,13 @@ gst_qt_sink_change_state (GstElement * element, GstStateChange transition)
(NULL));
return GST_STATE_CHANGE_FAILURE;
}

GST_OBJECT_LOCK (qt_sink->display);
gst_gl_display_add_context (qt_sink->display, qt_sink->context);
GST_OBJECT_UNLOCK (qt_sink->display);

gst_gl_element_propagate_display_context (GST_ELEMENT (qt_sink), qt_sink->display);

break;
case GST_STATE_CHANGE_READY_TO_PAUSED:
break;
Expand Down
85 changes: 80 additions & 5 deletions ext/qt/qtitem.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

#include <QtCore/QRunnable>
#include <QtCore/QMutexLocker>
#include <QtCore/QPointer>
#include <QtGui/QGuiApplication>
#include <QtQuick/QQuickWindow>
#include <QtQuick/QSGSimpleTextureNode>
Expand Down Expand Up @@ -82,6 +83,14 @@ struct _QtGLVideoItemPrivate
QOpenGLContext *qt_context;
GstGLContext *other_context;
GstGLContext *context;

/* buffers with textures that were bound by QML */
GQueue bound_buffers;
/* buffers that were previously bound but in the meantime a new one was
* bound so this one is most likely not used anymore
* FIXME: Ideally we would use fences for this but there seems to be no
* way to reliably "try wait" on a fence */
GQueue potentially_unbound_buffers;
};

class InitializeSceneGraph : public QRunnable
Expand All @@ -91,7 +100,7 @@ class InitializeSceneGraph : public QRunnable
void run();

private:
QtGLVideoItem *item_;
QPointer<QtGLVideoItem> item_;
};

InitializeSceneGraph::InitializeSceneGraph(QtGLVideoItem *item) :
Expand All @@ -101,7 +110,8 @@ InitializeSceneGraph::InitializeSceneGraph(QtGLVideoItem *item) :

void InitializeSceneGraph::run()
{
item_->onSceneGraphInitialized();
if(item_)
item_->onSceneGraphInitialized();
}

QtGLVideoItem::QtGLVideoItem()
Expand Down Expand Up @@ -135,6 +145,8 @@ QtGLVideoItem::QtGLVideoItem()

QtGLVideoItem::~QtGLVideoItem()
{
GstBuffer *tmp_buffer;

/* Before destroying the priv info, make sure
* no qmlglsink's will call in again, and that
* any ongoing calls are done by invalidating the proxy
Expand All @@ -150,6 +162,19 @@ QtGLVideoItem::~QtGLVideoItem()
gst_object_unref(this->priv->other_context);
if (this->priv->display)
gst_object_unref(this->priv->display);

while ((tmp_buffer = (GstBuffer*) g_queue_pop_head (&this->priv->potentially_unbound_buffers))) {
GST_TRACE ("old buffer %p should be unbound now, unreffing", tmp_buffer);
gst_buffer_unref (tmp_buffer);
}
while ((tmp_buffer = (GstBuffer*) g_queue_pop_head (&this->priv->bound_buffers))) {
GST_TRACE ("old buffer %p should be unbound now, unreffing", tmp_buffer);
gst_buffer_unref (tmp_buffer);
}

gst_buffer_replace (&this->priv->buffer, NULL);

gst_caps_replace (&this->priv->caps, NULL);
g_free (this->priv);
this->priv = NULL;
}
Expand Down Expand Up @@ -192,6 +217,9 @@ QSGNode *
QtGLVideoItem::updatePaintNode(QSGNode * oldNode,
UpdatePaintNodeData * updatePaintNodeData)
{
GstBuffer *old_buffer;
gboolean was_bound = FALSE;

if (!m_openGlContextInitialized) {
return oldNode;
}
Expand All @@ -201,7 +229,9 @@ QtGLVideoItem::updatePaintNode(QSGNode * oldNode,
GstQSGTexture *tex;

g_mutex_lock (&this->priv->lock);
gst_gl_context_activate (this->priv->other_context, TRUE);

if (gst_gl_context_get_current() == NULL)
gst_gl_context_activate (this->priv->other_context, TRUE);

GST_TRACE ("%p updatePaintNode", this);

Expand All @@ -217,6 +247,38 @@ QtGLVideoItem::updatePaintNode(QSGNode * oldNode,
}

tex = static_cast<GstQSGTexture *> (texNode->texture());

if ((old_buffer = tex->getBuffer(&was_bound))) {
if (old_buffer == this->priv->buffer) {
/* same buffer */
gst_buffer_unref (old_buffer);
} else if (!was_bound) {
GST_TRACE ("old buffer %p was not bound yet, unreffing", old_buffer);
gst_buffer_unref (old_buffer);
} else {
GstBuffer *tmp_buffer;

GST_TRACE ("old buffer %p was bound, queueing up for later", old_buffer);
/* Unref all buffers that were previously not bound anymore. At least
* one more buffer was bound in the meantime so this one is most likely
* not in use anymore. */
while ((tmp_buffer = (GstBuffer*) g_queue_pop_head (&this->priv->potentially_unbound_buffers))) {
GST_TRACE ("old buffer %p should be unbound now, unreffing", tmp_buffer);
gst_buffer_unref (tmp_buffer);
}

/* Move previous bound buffers to the next queue. We now know that
* another buffer was bound in the meantime and will free them on
* the next iteration above. */
while ((tmp_buffer = (GstBuffer*) g_queue_pop_head (&this->priv->bound_buffers))) {
GST_TRACE ("old buffer %p is potentially unbound now", tmp_buffer);
g_queue_push_tail (&this->priv->potentially_unbound_buffers, tmp_buffer);
}
g_queue_push_tail (&this->priv->bound_buffers, old_buffer);
}
old_buffer = NULL;
}

tex->setCaps (this->priv->caps);
tex->setBuffer (this->priv->buffer);
texNode->markDirty(QSGNode::DirtyMaterial);
Expand All @@ -240,7 +302,6 @@ QtGLVideoItem::updatePaintNode(QSGNode * oldNode,

texNode->setRect (QRectF (result.x, result.y, result.w, result.h));

gst_gl_context_activate (this->priv->other_context, FALSE);
g_mutex_unlock (&this->priv->lock);

return texNode;
Expand All @@ -249,12 +310,23 @@ QtGLVideoItem::updatePaintNode(QSGNode * oldNode,
static void
_reset (QtGLVideoItem * qt_item)
{
GstBuffer *tmp_buffer;

gst_buffer_replace (&qt_item->priv->buffer, NULL);

gst_caps_replace (&qt_item->priv->caps, NULL);

qt_item->priv->negotiated = FALSE;
qt_item->priv->initted = FALSE;

while ((tmp_buffer = (GstBuffer*) g_queue_pop_head (&qt_item->priv->potentially_unbound_buffers))) {
GST_TRACE ("old buffer %p should be unbound now, unreffing", tmp_buffer);
gst_buffer_unref (tmp_buffer);
}
while ((tmp_buffer = (GstBuffer*) g_queue_pop_head (&qt_item->priv->bound_buffers))) {
GST_TRACE ("old buffer %p should be unbound now, unreffing", tmp_buffer);
gst_buffer_unref (tmp_buffer);
}
}

void
Expand Down Expand Up @@ -289,7 +361,10 @@ QtGLVideoItem::onSceneGraphInitialized ()
QWindow* window = this->window();
#endif

GST_DEBUG ("scene graph initialization with Qt GL context %p",
if (this->window() == NULL)
return;

GST_DEBUG ("%p scene graph initialization with Qt GL context %p", this,
this->window()->openglContext ());

if (this->priv->qt_context == this->window()->openglContext ())
Expand Down