From c39e5411cb5bf3e1e09f1e0bf9cf3a6387022e75 Mon Sep 17 00:00:00 2001 From: Yoann Lopes Date: Sat, 10 Dec 2016 15:58:58 +0200 Subject: GStreamer: fix a memory leak in the media player session A gchar* was never freed. Change-Id: Ib28fee3d13839bf15a9216bdcd5da68b6d1f904c Reviewed-by: Christian Stromme --- src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp index 09b74148e..67bfa628f 100644 --- a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp +++ b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp @@ -1894,12 +1894,14 @@ void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find, gpoint { QGstreamerPlayerSession* session = (QGstreamerPlayerSession*)userData; - const gchar *uri = 0; + gchar *strval = 0; #if GST_CHECK_VERSION(1,0,0) - g_object_get(G_OBJECT(session->m_playbin), "current-uri", &uri, NULL); + g_object_get(G_OBJECT(session->m_playbin), "current-uri", &strval, NULL); #else - g_object_get(G_OBJECT(session->m_playbin), "uri", &uri, NULL); + g_object_get(G_OBJECT(session->m_playbin), "uri", &strval, NULL); #endif + const QString uri(QString::fromUtf8(strval)); + g_free(strval); guint64 length = gst_type_find_get_length(find); if (!length) @@ -1910,7 +1912,7 @@ void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find, gpoint while (length > 0) { const guint8 *data = gst_type_find_peek(find, 0, length); if (data) { - session->m_isPlaylist = (QPlaylistFileParser::findPlaylistType(QString::fromUtf8(uri), 0, data, length) != QPlaylistFileParser::UNKNOWN); + session->m_isPlaylist = (QPlaylistFileParser::findPlaylistType(uri, 0, data, length) != QPlaylistFileParser::UNKNOWN); return; } length >>= 1; // for HTTP files length is not available, -- cgit v1.2.3 From 2c80a54fd0962704a0651f07a83952b17888fc04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Str=C3=B8mme?= Date: Wed, 14 Dec 2016 12:27:28 +0100 Subject: GStreamer: Only register one typefind function Each time a GStreamer session was created it would register the same callback function, but each time with a new user data, which in our case was a pointer to the session. The pointer would then be used without verification of its validity. The problem with this is that the typefind function is static and used for all sessions, making multiple sessions impossible. The documentation for gst_type_find_register() also clearly specifies that the supplied user data needs to valid as long as the plugin is loaded, which of course was not the case. With this change only one callback function will be registered, and each session will register itself with a typefind delegate, that will keep track of sessions that are still alive. Note: This only makes sure that the typefind function is called on a valid session, it does not make sure that the type actually belong to the session, which is a separate issue. Task-number: QTBUG-50460 Change-Id: I20eeabfe4e85839e8845003298d6f64705c2e15e Reviewed-by: Yoann Lopes --- .../mediaplayer/qgstreamerplayersession.cpp | 64 ++++++++++++++++++---- .../mediaplayer/qgstreamerplayersession.h | 4 +- 2 files changed, 56 insertions(+), 12 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp index 67bfa628f..76a56aca7 100644 --- a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp +++ b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.cpp @@ -61,12 +61,59 @@ #include #include #include +#include +#include //#define DEBUG_PLAYBIN //#define DEBUG_VO_BIN_DUMP QT_BEGIN_NAMESPACE +class TypefindDelegator +{ +public: + TypefindDelegator() + { + Q_ASSERT(gst_type_find_register(0, "playlist", GST_RANK_MARGINAL, notifySessions, 0, 0, 0, 0) == TRUE); + } + + void add(QGstreamerPlayerSession *session) + { + QMutexLocker locker(&m_mtx); + m_sessions.append(session); + } + + void remove(QGstreamerPlayerSession *session) + { + QMutexLocker locker(&m_mtx); + const int idx = m_sessions.indexOf(session); + if (idx != -1) + m_sessions.remove(idx); + } + +private: + static void notifySessions(GstTypeFind *find, gpointer /* unused */) + { + QMutexLocker locker(&m_mtx); + SessionList::const_iterator it = m_sessions.constBegin(); + SessionList::const_iterator end = m_sessions.constEnd(); + + while (it != end) { + (*it)->playlistTypeFindFunction(find); + ++it; + } + } + + typedef QVector SessionList; + static SessionList m_sessions; + static QMutex m_mtx; +}; + +TypefindDelegator::SessionList TypefindDelegator::m_sessions; +QMutex TypefindDelegator::m_mtx; + +Q_GLOBAL_STATIC(TypefindDelegator, g_typeRegister); + static bool usePlaybinVolume() { static enum { Yes, No, Unknown } status = Unknown; @@ -147,10 +194,7 @@ QGstreamerPlayerSession::QGstreamerPlayerSession(QObject *parent) m_isLiveSource(false), m_isPlaylist(false) { - gboolean result = gst_type_find_register(0, "playlist", GST_RANK_MARGINAL, playlistTypeFindFunction, 0, 0, this, 0); - Q_ASSERT(result == TRUE); - Q_UNUSED(result); - + g_typeRegister->add(this); m_playbin = gst_element_factory_make(QT_GSTREAMER_PLAYBIN_ELEMENT_NAME, NULL); if (m_playbin) { //GST_PLAY_FLAG_NATIVE_VIDEO omits configuration of ffmpegcolorspace and videoscale, @@ -251,6 +295,8 @@ QGstreamerPlayerSession::QGstreamerPlayerSession(QObject *parent) QGstreamerPlayerSession::~QGstreamerPlayerSession() { + g_typeRegister->remove(this); + if (m_playbin) { stop(); @@ -1890,15 +1936,13 @@ void QGstreamerPlayerSession::resumeVideoProbes() m_videoProbe->stopFlushing(); } -void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find, gpointer userData) +void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find) { - QGstreamerPlayerSession* session = (QGstreamerPlayerSession*)userData; - gchar *strval = 0; #if GST_CHECK_VERSION(1,0,0) - g_object_get(G_OBJECT(session->m_playbin), "current-uri", &strval, NULL); + g_object_get(G_OBJECT(m_playbin), "current-uri", &strval, NULL); #else - g_object_get(G_OBJECT(session->m_playbin), "uri", &strval, NULL); + g_object_get(G_OBJECT(m_playbin), "uri", &strval, NULL); #endif const QString uri(QString::fromUtf8(strval)); g_free(strval); @@ -1912,7 +1956,7 @@ void QGstreamerPlayerSession::playlistTypeFindFunction(GstTypeFind *find, gpoint while (length > 0) { const guint8 *data = gst_type_find_peek(find, 0, length); if (data) { - session->m_isPlaylist = (QPlaylistFileParser::findPlaylistType(uri, 0, data, length) != QPlaylistFileParser::UNKNOWN); + m_isPlaylist = (QPlaylistFileParser::findPlaylistType(uri, 0, data, length) != QPlaylistFileParser::UNKNOWN); return; } length >>= 1; // for HTTP files length is not available, diff --git a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h index e5d5498c7..fdfb416bd 100644 --- a/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h +++ b/src/plugins/gstreamer/mediaplayer/qgstreamerplayersession.h @@ -131,6 +131,8 @@ public: void endOfMediaReset(); + void playlistTypeFindFunction(GstTypeFind *find); + public slots: void loadFromUri(const QNetworkRequest &url); void loadFromStream(const QNetworkRequest &url, QIODevice *stream); @@ -192,8 +194,6 @@ private: void flushVideoProbes(); void resumeVideoProbes(); - static void playlistTypeFindFunction(GstTypeFind *find, gpointer userData); - QNetworkRequest m_request; QMediaPlayer::State m_state; QMediaPlayer::State m_pendingState; -- cgit v1.2.3 From 38ebe4372ca7017b4ed7f477cc6532efcf5fdceb Mon Sep 17 00:00:00 2001 From: Tasuku Suzuki Date: Wed, 14 Dec 2016 23:00:37 +0900 Subject: return better images in metadata in gstreamer cover art image uses "preview image". But "image" is better for that. "preview image" will be used for thumbnail image. Change-Id: Ic01f878f146b0369eb84e6b153fa68fbc6c54e57 Reviewed-by: Yoann Lopes --- src/plugins/gstreamer/mediaplayer/qgstreamermetadataprovider.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/gstreamer/mediaplayer/qgstreamermetadataprovider.cpp b/src/plugins/gstreamer/mediaplayer/qgstreamermetadataprovider.cpp index 01103d659..b9e29245f 100644 --- a/src/plugins/gstreamer/mediaplayer/qgstreamermetadataprovider.cpp +++ b/src/plugins/gstreamer/mediaplayer/qgstreamermetadataprovider.cpp @@ -93,7 +93,8 @@ static const QGstreamerMetaDataKeyLookup *qt_gstreamerMetaDataKeys() //metadataKeys->insert(0, QMediaMetaData::CoverArtUrlSmall); //metadataKeys->insert(0, QMediaMetaData::CoverArtUrlLarge); - metadataKeys->insert(GST_TAG_PREVIEW_IMAGE, QMediaMetaData::CoverArtImage); + metadataKeys->insert(GST_TAG_PREVIEW_IMAGE, QMediaMetaData::ThumbnailImage); + metadataKeys->insert(GST_TAG_IMAGE, QMediaMetaData::CoverArtImage); // Image/Video metadataKeys->insert("resolution", QMediaMetaData::Resolution); -- cgit v1.2.3 From fc50058455536b857d101166f8c003e83aed4f99 Mon Sep 17 00:00:00 2001 From: Maurice Kalinowski Date: Tue, 20 Dec 2016 15:41:15 +0100 Subject: winrt: Fix video playback Rendering should only be activated once the video frame size is known. Otherwise the texture is created with an invalid size and rendering will be stopped. This happens for some codecs where MF_MEDIA_ENGINE_EVENT_LOADEDMETADATA is not known instantly after loading video files. Task-number: QTBUG-57707 Change-Id: Ic2347c8606239770e129544fd228b96929ed6198 Reviewed-by: Oliver Wolff --- src/plugins/winrt/qwinrtmediaplayercontrol.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/winrt/qwinrtmediaplayercontrol.cpp b/src/plugins/winrt/qwinrtmediaplayercontrol.cpp index 5720488f2..a4df6306f 100644 --- a/src/plugins/winrt/qwinrtmediaplayercontrol.cpp +++ b/src/plugins/winrt/qwinrtmediaplayercontrol.cpp @@ -267,7 +267,8 @@ public: } if (d->videoRenderer) - d->videoRenderer->setActive(d->state == QMediaPlayer::PlayingState); + d->videoRenderer->setActive(d->state == QMediaPlayer::PlayingState && + d->videoRenderer->size().isValid()); const QMediaPlayer::MediaStatus oldMediaStatus = d->mediaStatus; const QMediaPlayer::State oldState = d->state; -- cgit v1.2.3 From b4211c1c233933d109bc71b640013e14afc58d4a Mon Sep 17 00:00:00 2001 From: Maurice Kalinowski Date: Tue, 20 Dec 2016 15:45:28 +0100 Subject: winrt: Fix pause/play for videos This is partially a revert of bff19dbe6733d245. While that commit fixed switching cameras it did not respect pausing a video, where setActive is called from a different thread than in the camera case. Using shutdown introduced a wait, which in that case deadlocked. Instead revert to request an interrupt, but when restarting make sure that the render thread is on hold before start is invoked. Task-number: QTBUG-53722 Change-Id: Id5a722911bb2c30509fabb73bec925cadbf77628 Reviewed-by: Oliver Wolff --- src/plugins/winrt/qwinrtabstractvideorenderercontrol.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/winrt/qwinrtabstractvideorenderercontrol.cpp b/src/plugins/winrt/qwinrtabstractvideorenderercontrol.cpp index 561d9e03c..b3fd11111 100644 --- a/src/plugins/winrt/qwinrtabstractvideorenderercontrol.cpp +++ b/src/plugins/winrt/qwinrtabstractvideorenderercontrol.cpp @@ -356,6 +356,13 @@ void QWinRTAbstractVideoRendererControl::setActive(bool active) if (!d->surface) return; + // This only happens for quick restart scenarios, for instance + // when switching cameras. + if (d->renderThread.isRunning() && d->renderThread.isInterruptionRequested()) { + CriticalSectionLocker lock(&d->mutex); + d->renderThread.wait(); + } + if (!d->surface->isActive()) d->surface->start(d->format); @@ -363,7 +370,8 @@ void QWinRTAbstractVideoRendererControl::setActive(bool active) return; } - shutdown(); + d->renderThread.requestInterruption(); + if (d->surface && d->surface->isActive()) d->surface->stop(); } -- cgit v1.2.3 From c1164f874a21959d03893f62db8f8e2def44122d Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Wed, 11 Jan 2017 14:23:36 +0100 Subject: Only update the texture if the m_glResources are valid Change-Id: Ifa0c768e2f0299d31d3d52db975c896bdb2aab5e Reviewed-by: Yoann Lopes --- src/plugins/common/evr/evrd3dpresentengine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/common/evr/evrd3dpresentengine.cpp b/src/plugins/common/evr/evrd3dpresentengine.cpp index 9718c78b5..ae3d69fc2 100644 --- a/src/plugins/common/evr/evrd3dpresentengine.cpp +++ b/src/plugins/common/evr/evrd3dpresentengine.cpp @@ -255,7 +255,7 @@ QVariant IMFSampleVideoBuffer::handle() const if (handleType() != GLTextureHandle) return handle; - if (m_textureUpdated || m_engine->updateTexture(m_surface)) { + if (m_engine->m_glResources && (m_textureUpdated || m_engine->updateTexture(m_surface))) { m_textureUpdated = true; handle = QVariant::fromValue(m_engine->m_glResources->glTexture); } -- cgit v1.2.3