From 59f20906d9d6d7bc33069fc0960c59edf0e232e1 Mon Sep 17 00:00:00 2001 From: Maurice Kalinowski Date: Tue, 28 Jun 2016 10:16:40 +0200 Subject: winrt: Do not assert when missing region of interest Asserting is too heavy and can cause false positive on devices like the Lumia 535, where focus is supported, but not setting a focus point. Later on we check for existence of the control during the initialization, so it is safe to continue here. Task-number: QTBUG-54278 Change-Id: Ie68ff754d742888bdd6f4047e07d207707c90c91 Reviewed-by: Friedemann Kleint Reviewed-by: Oliver Wolff --- src/plugins/winrt/qwinrtcameracontrol.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/winrt/qwinrtcameracontrol.cpp b/src/plugins/winrt/qwinrtcameracontrol.cpp index f11a975b2..73ae4b067 100644 --- a/src/plugins/winrt/qwinrtcameracontrol.cpp +++ b/src/plugins/winrt/qwinrtcameracontrol.cpp @@ -942,7 +942,8 @@ HRESULT QWinRTCameraControl::initialize() Q_ASSERT_SUCCEEDED(hr); if (isFocusSupported) { hr = advancedVideoDeviceController->get_RegionsOfInterestControl(&d->regionsOfInterestControl); - Q_ASSERT_SUCCEEDED(hr); + if (FAILED(hr)) + qCDebug(lcMMCamera) << "Focus supported, but no control for regions of interest available"; hr = initializeFocus(); Q_ASSERT_SUCCEEDED(hr); } else { -- cgit v1.2.3 From fa91fc211c788305de930d355c1c81725f829789 Mon Sep 17 00:00:00 2001 From: Dan Cape Date: Wed, 29 Jun 2016 12:13:01 -0400 Subject: QNX: Reset playback position before stopping media MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change ensures that the next media file to be played back will start playing at position 0. Without this change, it would start playback at the last position of the previous media file. In the play() function, when attempting to play a video, we first go into the loading state which just defers playing until later. Since we set the state to “Playing”, we now accept events from mm-renderer. It sends us a position update which ends up being from the previous video. Since we’re in “Playing” state, we trust this event is right and set our internal m_position variable to that value. Once the media has loaded, play() is called again and then we hit the call setPositionInternal() which seeks the media to our internal m_position (the position we got from mm-renderer). To resolve this, we now will call setPosition in the stop function that will cause mm-renderer to seek to 0 just before it stops playback (as recommended in documentation) so that any events we get from mm-renderer when we move to the “Playing” state will be related to the current media. Change-Id: I8ae35e57dd690bbae1fb996c1feb15ee4addab55 Reviewed-by: James McDonnell Reviewed-by: Rafael Roquetto --- src/plugins/qnx/mediaplayer/mmrenderermediaplayercontrol.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/qnx/mediaplayer/mmrenderermediaplayercontrol.cpp b/src/plugins/qnx/mediaplayer/mmrenderermediaplayercontrol.cpp index 1cccbfa01..569070a6a 100644 --- a/src/plugins/qnx/mediaplayer/mmrenderermediaplayercontrol.cpp +++ b/src/plugins/qnx/mediaplayer/mmrenderermediaplayercontrol.cpp @@ -340,6 +340,8 @@ void MmRendererMediaPlayerControl::setState(QMediaPlayer::State state) void MmRendererMediaPlayerControl::stopInternal(StopCommand stopCommand) { + setPosition(0); + if (m_state != QMediaPlayer::StoppedState) { if (stopCommand == StopMmRenderer) { @@ -349,11 +351,6 @@ void MmRendererMediaPlayerControl::stopInternal(StopCommand stopCommand) setState(QMediaPlayer::StoppedState); } - - if (m_position != 0) { - m_position = 0; - emit positionChanged(0); - } } void MmRendererMediaPlayerControl::setVolume(int volume) -- cgit v1.2.3 From 7fb7f278fff2c407f8c0fe8165c60a934f95cd91 Mon Sep 17 00:00:00 2001 From: Yoann Lopes Date: Tue, 5 Jul 2016 14:07:28 +0200 Subject: DirectShow: fix deadlock in VideoSurfaceFilter When the DirectShow graph is done being built, there's no need to wake up any blocking calls on the main thread since setting a source is done asynchronously anyway. This could cause synchronous operations (like stopping or changing the video output) to end prematurely if called while a source was still being loaded, in turn causing the deadlock. Task-number: QTBUG-54504 Change-Id: I4f534e637bfca6d3020a3bc28725c8c7042941d5 Reviewed-by: Andreas Holzammer --- src/plugins/directshow/player/directshowplayerservice.cpp | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/directshow/player/directshowplayerservice.cpp b/src/plugins/directshow/player/directshowplayerservice.cpp index 57f100b9c..f7510316a 100644 --- a/src/plugins/directshow/player/directshowplayerservice.cpp +++ b/src/plugins/directshow/player/directshowplayerservice.cpp @@ -535,8 +535,6 @@ void DirectShowPlayerService::doRender(QMutexLocker *locker) m_executedTasks |= Render; } - - m_loop->wake(); } void DirectShowPlayerService::doFinalizeLoad(QMutexLocker *locker) -- cgit v1.2.3 From 443ed0be093b2d8e78d111a59eca97f2eba5b0fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Str=C3=B8mme?= Date: Mon, 4 Jul 2016 17:32:24 +0200 Subject: Android: Fix attach- and detachFromGLContext() This fixes a regression caused by ba8127639857232d8a. The change assumed that the AndroidSurfaceTexture class could cope with changing textures, but since the callback from the SurfaceTextureListener was tied to the initial, invalid, texture handle, it would only work as long as there were only one texture; all textures would register the callback to the same invalid handle. This change maps the callback directly to the android texture object, instead of the texture handle. Task-number: QTBUG-54340 Change-Id: I39568d0f97fa6b9cb1182efaca568b16a26f0d09 Reviewed-by: Yoann Lopes --- .../multimedia/QtSurfaceTextureListener.java | 10 +++--- .../src/wrappers/jni/androidsurfacetexture.cpp | 42 +++++++++++++++------- .../src/wrappers/jni/androidsurfacetexture.h | 4 +-- 3 files changed, 37 insertions(+), 19 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/android/jar/src/org/qtproject/qt5/android/multimedia/QtSurfaceTextureListener.java b/src/plugins/android/jar/src/org/qtproject/qt5/android/multimedia/QtSurfaceTextureListener.java index bd076bd43..a9ec4b5ae 100644 --- a/src/plugins/android/jar/src/org/qtproject/qt5/android/multimedia/QtSurfaceTextureListener.java +++ b/src/plugins/android/jar/src/org/qtproject/qt5/android/multimedia/QtSurfaceTextureListener.java @@ -37,18 +37,18 @@ import android.graphics.SurfaceTexture; public class QtSurfaceTextureListener implements SurfaceTexture.OnFrameAvailableListener { - private final int texID; + private final long m_id; - public QtSurfaceTextureListener(int texName) + public QtSurfaceTextureListener(long id) { - texID = texName; + m_id = id; } @Override public void onFrameAvailable(SurfaceTexture surfaceTexture) { - notifyFrameAvailable(texID); + notifyFrameAvailable(m_id); } - private static native void notifyFrameAvailable(int id); + private static native void notifyFrameAvailable(long id); } diff --git a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp index 2cea896e1..bc7187e9c 100644 --- a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp +++ b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.cpp @@ -34,24 +34,32 @@ #include "androidsurfacetexture.h" #include #include +#include QT_BEGIN_NAMESPACE static const char QtSurfaceTextureListenerClassName[] = "org/qtproject/qt5/android/multimedia/QtSurfaceTextureListener"; -static QMap g_objectMap; +typedef QVector SurfaceTextures; +Q_GLOBAL_STATIC(SurfaceTextures, g_surfaceTextures); +Q_GLOBAL_STATIC(QMutex, g_textureMutex); // native method for QtSurfaceTexture.java -static void notifyFrameAvailable(JNIEnv* , jobject, int id) +static void notifyFrameAvailable(JNIEnv* , jobject, jlong id) { - AndroidSurfaceTexture *obj = g_objectMap.value(id, 0); + const QMutexLocker lock(g_textureMutex); + const int idx = g_surfaceTextures->indexOf(id); + if (idx == -1) + return; + + AndroidSurfaceTexture *obj = reinterpret_cast(g_surfaceTextures->at(idx)); if (obj) Q_EMIT obj->frameAvailable(); } AndroidSurfaceTexture::AndroidSurfaceTexture(unsigned int texName) : QObject() - , m_texID(int(texName)) { + Q_STATIC_ASSERT(sizeof (jlong) >= sizeof (void *)); // API level 11 or higher is required if (QtAndroidPrivate::androidSdkVersion() < 11) { qWarning("Camera preview and video playback require Android 3.0 (API level 11) or later."); @@ -67,13 +75,13 @@ AndroidSurfaceTexture::AndroidSurfaceTexture(unsigned int texName) env->ExceptionClear(); } - if (m_surfaceTexture.isValid()) - g_objectMap.insert(int(texName), this); + if (!m_surfaceTexture.isValid()) + return; - QJNIObjectPrivate listener(QtSurfaceTextureListenerClassName, "(I)V", jint(texName)); - m_surfaceTexture.callMethod("setOnFrameAvailableListener", - "(Landroid/graphics/SurfaceTexture$OnFrameAvailableListener;)V", - listener.object()); + const QMutexLocker lock(g_textureMutex); + g_surfaceTextures->append(jlong(this)); + QJNIObjectPrivate listener(QtSurfaceTextureListenerClassName, "(J)V", jlong(this)); + setOnFrameAvailableListener(listener); } AndroidSurfaceTexture::~AndroidSurfaceTexture() @@ -83,7 +91,10 @@ AndroidSurfaceTexture::~AndroidSurfaceTexture() if (m_surfaceTexture.isValid()) { release(); - g_objectMap.remove(m_texID); + const QMutexLocker lock(g_textureMutex); + const int idx = g_surfaceTextures->indexOf(jlong(this)); + if (idx != -1) + g_surfaceTextures->remove(idx); } } @@ -172,7 +183,7 @@ bool AndroidSurfaceTexture::initJNI(JNIEnv *env) env); static const JNINativeMethod methods[] = { - {"notifyFrameAvailable", "(I)V", (void *)notifyFrameAvailable} + {"notifyFrameAvailable", "(J)V", (void *)notifyFrameAvailable} }; if (clazz && env->RegisterNatives(clazz, @@ -184,4 +195,11 @@ bool AndroidSurfaceTexture::initJNI(JNIEnv *env) return true; } +void AndroidSurfaceTexture::setOnFrameAvailableListener(const QJNIObjectPrivate &listener) +{ + m_surfaceTexture.callMethod("setOnFrameAvailableListener", + "(Landroid/graphics/SurfaceTexture$OnFrameAvailableListener;)V", + listener.object()); +} + QT_END_NAMESPACE diff --git a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h index a08483e5e..3c41bf51d 100644 --- a/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h +++ b/src/plugins/android/src/wrappers/jni/androidsurfacetexture.h @@ -48,7 +48,6 @@ public: explicit AndroidSurfaceTexture(unsigned int texName); ~AndroidSurfaceTexture(); - int textureID() const { return m_texID; } jobject surfaceTexture(); jobject surface(); jobject surfaceHolder(); @@ -67,7 +66,8 @@ Q_SIGNALS: void frameAvailable(); private: - int m_texID; + void setOnFrameAvailableListener(const QJNIObjectPrivate &listener); + QJNIObjectPrivate m_surfaceTexture; QJNIObjectPrivate m_surface; QJNIObjectPrivate m_surfaceHolder; -- cgit v1.2.3 From fd89c0946a378812121615ab6b8a3ad1ae93d4fb Mon Sep 17 00:00:00 2001 From: Yoann Lopes Date: Tue, 5 Jul 2016 13:21:31 +0200 Subject: DirectShow: correctly clear surface in EVR presenter The surface was never cleared in the EVR presenter. It could lead to situations where the presenter would use a destroyed surface. Change-Id: If2223f09f6f8c20c06345bed40803da10dcf4ae3 Reviewed-by: Christian Stromme --- .../player/directshowvideorenderercontrol.cpp | 24 +++++++++++++++++++--- .../player/directshowvideorenderercontrol.h | 6 ++++++ 2 files changed, 27 insertions(+), 3 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/directshow/player/directshowvideorenderercontrol.cpp b/src/plugins/directshow/player/directshowvideorenderercontrol.cpp index 027d2ce55..b5dbfef9e 100644 --- a/src/plugins/directshow/player/directshowvideorenderercontrol.cpp +++ b/src/plugins/directshow/player/directshowvideorenderercontrol.cpp @@ -46,11 +46,20 @@ DirectShowVideoRendererControl::DirectShowVideoRendererControl(DirectShowEventLo , m_loop(loop) , m_surface(0) , m_filter(0) +#ifdef HAVE_EVR + , m_evrPresenter(0) +#endif { } DirectShowVideoRendererControl::~DirectShowVideoRendererControl() { +#ifdef HAVE_EVR + if (m_evrPresenter) { + m_evrPresenter->setSurface(Q_NULLPTR); + m_evrPresenter->Release(); + } +#endif if (m_filter) m_filter->Release(); } @@ -65,6 +74,14 @@ void DirectShowVideoRendererControl::setSurface(QAbstractVideoSurface *surface) if (m_surface == surface) return; +#ifdef HAVE_EVR + if (m_evrPresenter) { + m_evrPresenter->setSurface(Q_NULLPTR); + m_evrPresenter->Release(); + m_evrPresenter = 0; + } +#endif + if (m_filter) { m_filter->Release(); m_filter = 0; @@ -75,12 +92,13 @@ void DirectShowVideoRendererControl::setSurface(QAbstractVideoSurface *surface) if (m_surface) { #ifdef HAVE_EVR m_filter = com_new(clsid_EnhancedVideoRenderer); - EVRCustomPresenter *evrPresenter = new EVRCustomPresenter(m_surface); - if (!evrPresenter->isValid() || !qt_evr_setCustomPresenter(m_filter, evrPresenter)) { + m_evrPresenter = new EVRCustomPresenter(m_surface); + if (!m_evrPresenter->isValid() || !qt_evr_setCustomPresenter(m_filter, m_evrPresenter)) { m_filter->Release(); m_filter = 0; + m_evrPresenter->Release(); + m_evrPresenter = 0; } - evrPresenter->Release(); if (!m_filter) #endif diff --git a/src/plugins/directshow/player/directshowvideorenderercontrol.h b/src/plugins/directshow/player/directshowvideorenderercontrol.h index d08d124ca..e3d0a5f23 100644 --- a/src/plugins/directshow/player/directshowvideorenderercontrol.h +++ b/src/plugins/directshow/player/directshowvideorenderercontrol.h @@ -39,6 +39,9 @@ #include "qvideorenderercontrol.h" class DirectShowEventLoop; +#ifdef HAVE_EVR +class EVRCustomPresenter; +#endif QT_USE_NAMESPACE @@ -61,6 +64,9 @@ private: DirectShowEventLoop *m_loop; QAbstractVideoSurface *m_surface; IBaseFilter *m_filter; +#ifdef HAVE_EVR + EVRCustomPresenter *m_evrPresenter; +#endif }; #endif -- cgit v1.2.3 From 278fd530f0ad883f88e15d3403f1ab63fbdeb131 Mon Sep 17 00:00:00 2001 From: Pavel Golikov Date: Wed, 6 Jul 2016 15:22:31 +0300 Subject: Android: Fix OpenGL texture name leak This fixes texture name leak when changing media player's source. Texture name shold be deleted by OpenGLResourcesDeleter class, but when player's source is changed OpenGLResourcesDeleter's texture name is reassigned with new one without old name deletion. This change deletes OpenGLResourcesDeleter's current texture name when new name is assigned. Task-number: QTBUG-54340 Change-Id: I22bbd60b4462b0034fd115f0582ea43b9bcaee4b Reviewed-by: Yoann Lopes --- src/plugins/android/src/common/qandroidvideooutput.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/android/src/common/qandroidvideooutput.cpp b/src/plugins/android/src/common/qandroidvideooutput.cpp index f69be679a..f462a28d5 100644 --- a/src/plugins/android/src/common/qandroidvideooutput.cpp +++ b/src/plugins/android/src/common/qandroidvideooutput.cpp @@ -141,7 +141,13 @@ public: delete m_program; } - void setTexture(quint32 id) { m_textureID = id; } + void setTexture(quint32 id) { + if (m_textureID) + glDeleteTextures(1, &m_textureID); + + m_textureID = id; + } + void setFbo(QOpenGLFramebufferObject *fbo) { m_fbo = fbo; } void setShaderProgram(QOpenGLShaderProgram *prog) { m_program = prog; } -- cgit v1.2.3 From 17674ddc637a059c3052953c1ec8a80a7d078293 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Wed, 29 Jun 2016 14:50:34 +0200 Subject: AVFMediaAssetWriter - fix atomics use 1. No need in two different atomics (m_stopped/m_aborted) - the single one 'm_state' with states (Idle/Active/Aborted) should be enough. 2. QAtomicInt::load/store actually have relaxed memory ordering semantics, (not like std::atomic with sequential ordering as the default one) which is not always appropriate - replace with loadAquire/storeRelease instead. Change-Id: I4ce8c9ca7556de3d2c7e369b8a05276b2870460c Reviewed-by: Yoann Lopes --- .../avfoundation/camera/avfmediaassetwriter.h | 6 +-- .../avfoundation/camera/avfmediaassetwriter.mm | 53 +++++++++++++--------- 2 files changed, 32 insertions(+), 27 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/avfoundation/camera/avfmediaassetwriter.h b/src/plugins/avfoundation/camera/avfmediaassetwriter.h index 9b2479909..77e2a2f32 100644 --- a/src/plugins/avfoundation/camera/avfmediaassetwriter.h +++ b/src/plugins/avfoundation/camera/avfmediaassetwriter.h @@ -47,13 +47,10 @@ QT_BEGIN_NAMESPACE class AVFMediaRecorderControlIOS; class AVFCameraService; -typedef QAtomicInteger AVFAtomicBool; typedef QAtomicInteger AVFAtomicInt64; QT_END_NAMESPACE -// TODO: any reasonable error handling requires smart pointers, otherwise it's getting crappy immediately. - @interface QT_MANGLE_NAMESPACE(AVFMediaAssetWriter) : NSObject { @@ -78,9 +75,8 @@ QT_END_NAMESPACE QT_PREPEND_NAMESPACE(AVFMediaRecorderControlIOS) *m_delegate; bool m_setStartTime; - QT_PREPEND_NAMESPACE(AVFAtomicBool) m_stopped; - QT_PREPEND_NAMESPACE(AVFAtomicBool) m_aborted; + QT_PREPEND_NAMESPACE(QAtomicInt) m_state; QT_PREPEND_NAMESPACE(QMutex) m_writerMutex; @public QT_PREPEND_NAMESPACE(AVFAtomicInt64) m_durationInMs; diff --git a/src/plugins/avfoundation/camera/avfmediaassetwriter.mm b/src/plugins/avfoundation/camera/avfmediaassetwriter.mm index 59c1d1aa2..4e227114b 100644 --- a/src/plugins/avfoundation/camera/avfmediaassetwriter.mm +++ b/src/plugins/avfoundation/camera/avfmediaassetwriter.mm @@ -40,7 +40,6 @@ #include "avfcameradebug.h" #include "avfmediacontainercontrol.h" -//#include #include #include @@ -68,6 +67,13 @@ bool qt_camera_service_isValid(AVFCameraService *service) return true; } +enum WriterState +{ + WriterStateIdle, + WriterStateActive, + WriterStateAborted +}; + } // unnamed namespace @interface QT_MANGLE_NAMESPACE(AVFMediaAssetWriter) (PrivateAPI) @@ -92,8 +98,7 @@ bool qt_camera_service_isValid(AVFCameraService *service) m_delegate = delegate; m_setStartTime = true; - m_stopped.store(true); - m_aborted.store(false); + m_state.store(WriterStateIdle); m_startTime = kCMTimeInvalid; m_lastTimeStamp = kCMTimeInvalid; m_durationInMs.store(0); @@ -167,15 +172,22 @@ bool qt_camera_service_isValid(AVFCameraService *service) - (void)start { - // To be executed on a writer's queue. + // 'start' is executed on a special writer's queue. + // We lock a mutex for the whole function body + // - render control probably was deleted + // 'immediately' after submitting 'start' + // and before 'start' is actually executed ... + // So if we're starting, render control can not + // abort us 'in the middle'. const QMutexLocker lock(&m_writerMutex); - if (m_aborted.load()) + if (m_state.load() == WriterStateAborted) return; [self setQueues]; m_setStartTime = true; - m_stopped.store(false); + m_state.storeRelease(WriterStateActive); + [m_assetWriter startWriting]; AVCaptureSession *session = m_service->session()->captureSession(); if (!session.running) @@ -187,26 +199,23 @@ bool qt_camera_service_isValid(AVFCameraService *service) // To be executed on a writer's queue. { const QMutexLocker lock(&m_writerMutex); - if (m_aborted.load()) - return; - - if (m_stopped.load()) + if (m_state.load() != WriterStateActive) return; - - m_stopped.store(true); } [m_assetWriter finishWritingWithCompletionHandler:^{ // This block is async, so by the time it's executed, // it's possible that render control was deleted already ... + // We lock so that nobody can call 'abort' in the middle ... const QMutexLocker lock(&m_writerMutex); - if (m_aborted.load()) + if (m_state.load() != WriterStateActive) return; AVCaptureSession *session = m_service->session()->captureSession(); [session stopRunning]; [session removeOutput:m_audioOutput]; [session removeInput:m_audioInput]; + m_state.storeRelease(WriterStateIdle); QMetaObject::invokeMethod(m_delegate, "assetWriterFinished", Qt::QueuedConnection); }]; } @@ -216,9 +225,10 @@ bool qt_camera_service_isValid(AVFCameraService *service) // To be executed on any thread (presumably, it's the main thread), // prevents writer from accessing any shared object. const QMutexLocker lock(&m_writerMutex); - m_aborted.store(true); - if (m_stopped.load()) + if (m_state.fetchAndStoreRelease(WriterStateAborted) != WriterStateActive) { + // Not recording, nothing to stop. return; + } [m_assetWriter finishWritingWithCompletionHandler:^{ }]; @@ -231,7 +241,7 @@ bool qt_camera_service_isValid(AVFCameraService *service) Q_ASSERT(sampleBuffer); const QMutexLocker lock(&m_writerMutex); - if (m_aborted.load() || m_stopped.load()) + if (m_state.load() != WriterStateActive) return; QMetaObject::invokeMethod(m_delegate, "assetWriterStarted", Qt::QueuedConnection); @@ -248,7 +258,7 @@ bool qt_camera_service_isValid(AVFCameraService *service) Q_ASSERT(sampleBuffer); // This code is executed only on a writer's queue. - if (!m_aborted.load() && !m_stopped.load()) { + if (m_state.loadAcquire() == WriterStateActive) { if (m_setStartTime) [self setStartTimeFrom:sampleBuffer]; @@ -264,11 +274,10 @@ bool qt_camera_service_isValid(AVFCameraService *service) - (void)writeAudioSampleBuffer:(CMSampleBufferRef)sampleBuffer { - // This code is executed only on a writer's queue. - // it does not touch any shared/external data. Q_ASSERT(sampleBuffer); - if (!m_aborted.load() && !m_stopped.load()) { + // This code is executed only on a writer's queue. + if (m_state.loadAcquire() == WriterStateActive) { if (m_setStartTime) [self setStartTimeFrom:sampleBuffer]; @@ -290,7 +299,7 @@ bool qt_camera_service_isValid(AVFCameraService *service) // This method can be called on either video or audio queue, // never on a writer's queue, it needs access to a shared data, so // lock is required. - if (m_stopped.load()) + if (m_state.loadAcquire() != WriterStateActive) return; if (!CMSampleBufferDataIsReady(sampleBuffer)) { @@ -302,7 +311,7 @@ bool qt_camera_service_isValid(AVFCameraService *service) if (captureOutput != m_audioOutput.data()) { const QMutexLocker lock(&m_writerMutex); - if (m_aborted.load() || m_stopped.load()) { + if (m_state.load() != WriterStateActive) { CFRelease(sampleBuffer); return; } -- cgit v1.2.3 From 6f28963c50261fffdabd48be0b0dfe20480277bb Mon Sep 17 00:00:00 2001 From: BogDan Vatra Date: Wed, 20 Jul 2016 15:47:58 +0300 Subject: Make sure JNI_OnLoad is not called more than once Since Android 5.0 Google introduce a nasty bug[1] which calls JNI_OnLoad more than once. Basically every time when a library is loaded JNI_OnLoad is called if found, but it calls *again* JNI_OnLoad of its .so dependencies! [1] Workaround https://code.google.com/p/android/issues/detail?id=215069 Change-Id: I81b4a94beedaad299267ac6deab2f9c3a1693a62 Reviewed-by: Eskil Abrahamsen Blomfeldt --- src/plugins/android/src/qandroidmediaserviceplugin.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/plugins') diff --git a/src/plugins/android/src/qandroidmediaserviceplugin.cpp b/src/plugins/android/src/qandroidmediaserviceplugin.cpp index bf89badb3..91381b76d 100644 --- a/src/plugins/android/src/qandroidmediaserviceplugin.cpp +++ b/src/plugins/android/src/qandroidmediaserviceplugin.cpp @@ -145,6 +145,11 @@ QT_END_NAMESPACE #ifndef Q_OS_ANDROID_NO_SDK Q_DECL_EXPORT jint JNICALL JNI_OnLoad(JavaVM *vm, void * /*reserved*/) { + static bool initialized = false; + if (initialized) + return JNI_VERSION_1_6; + initialized = true; + QT_USE_NAMESPACE typedef union { JNIEnv *nativeEnvironment; -- cgit v1.2.3 From ab940d8fe9affd2ae1bb9cdf0e3f9d41fa49a662 Mon Sep 17 00:00:00 2001 From: Jesus Fernandez Date: Mon, 23 May 2016 17:28:46 +0200 Subject: Resource leak fixed CameraBinV4LImageProcessing::setParameter was leaking the resource fd in some cases in the switch using return instead of break. Change-Id: Ie56eaf4cf1d1b7531094c321f49a818632985628 Reviewed-by: Yoann Lopes --- src/plugins/gstreamer/camerabin/camerabinv4limageprocessing.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/plugins') diff --git a/src/plugins/gstreamer/camerabin/camerabinv4limageprocessing.cpp b/src/plugins/gstreamer/camerabin/camerabinv4limageprocessing.cpp index bf51cbfd0..913c4548d 100644 --- a/src/plugins/gstreamer/camerabin/camerabinv4limageprocessing.cpp +++ b/src/plugins/gstreamer/camerabin/camerabinv4limageprocessing.cpp @@ -194,8 +194,10 @@ void CameraBinV4LImageProcessing::setParameter( const QCameraImageProcessing::WhiteBalanceMode m = value.value(); if (m != QCameraImageProcessing::WhiteBalanceAuto - && m != QCameraImageProcessing::WhiteBalanceManual) + && m != QCameraImageProcessing::WhiteBalanceManual) { + qt_safe_close(fd); return; + } control.value = (m == QCameraImageProcessing::WhiteBalanceAuto) ? true : false; } @@ -214,6 +216,7 @@ void CameraBinV4LImageProcessing::setParameter( break; default: + qt_safe_close(fd); return; } -- cgit v1.2.3 From 7788feea5d55418ab61681e122c1d2d29f7bddce Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Fri, 22 Jul 2016 14:29:45 +0200 Subject: AVFMediaAssetWriter - fix race conditions Apple recommends starting/setting up a session (AVCaptureSession) on a special queue and in the past we did this on the writer's queue. Unfortunately, we also can access the same session from a different thread and this results in race conditions and different weird crashes, for example, we're calling start/stopRunning from the writer's queue, while the session is being configured (in between beginConfiguration/commitConfiguration on the recorder control's thread). So we have to limit access to the session by the control's thread. Apple docs say we have to ensure all appendSampleBuffer calls done _before_ finishWriting. We ensure this by dispatching_sync an empty block on the writer's queue after store-release of m_state. We also do the same with video queue to ensure it does not try to access viewfinder's data after stop/abort executed. All these changes also make lock/mutex unneeded. Task-number: QTBUG-54890 Change-Id: I38e86c879b6b62306bdfbeade65405d6ac3be9f3 Reviewed-by: Yoann Lopes --- .../avfoundation/camera/avfmediaassetwriter.h | 13 ++-- .../avfoundation/camera/avfmediaassetwriter.mm | 91 ++++++++++++---------- .../camera/avfmediarecordercontrol_ios.h | 2 - .../camera/avfmediarecordercontrol_ios.mm | 26 +++---- 4 files changed, 67 insertions(+), 65 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/avfoundation/camera/avfmediaassetwriter.h b/src/plugins/avfoundation/camera/avfmediaassetwriter.h index 77e2a2f32..c36caee8b 100644 --- a/src/plugins/avfoundation/camera/avfmediaassetwriter.h +++ b/src/plugins/avfoundation/camera/avfmediaassetwriter.h @@ -38,7 +38,6 @@ #include #include -#include #include @@ -63,12 +62,12 @@ QT_END_NAMESPACE QT_PREPEND_NAMESPACE(AVFScopedPointer) m_audioWriterInput; AVCaptureDevice *m_audioCaptureDevice; + // Queue to write sample buffers: + QT_PREPEND_NAMESPACE(AVFScopedPointer) m_writerQueue; // High priority serial queue for video output: QT_PREPEND_NAMESPACE(AVFScopedPointer) m_videoQueue; // Serial queue for audio output: QT_PREPEND_NAMESPACE(AVFScopedPointer) m_audioQueue; - // Queue to write sample buffers: - QT_PREPEND_NAMESPACE(AVFScopedPointer) m_writerQueue; QT_PREPEND_NAMESPACE(AVFScopedPointer) m_assetWriter; @@ -77,7 +76,6 @@ QT_END_NAMESPACE bool m_setStartTime; QT_PREPEND_NAMESPACE(QAtomicInt) m_state; - QT_PREPEND_NAMESPACE(QMutex) m_writerMutex; @public QT_PREPEND_NAMESPACE(AVFAtomicInt64) m_durationInMs; @private @@ -88,8 +86,7 @@ QT_END_NAMESPACE NSDictionary *m_videoSettings; } -- (id)initWithQueue:(dispatch_queue_t)writerQueue - delegate:(QT_PREPEND_NAMESPACE(AVFMediaRecorderControlIOS) *)delegate; +- (id)initWithDelegate:(QT_PREPEND_NAMESPACE(AVFMediaRecorderControlIOS) *)delegate; - (bool)setupWithFileURL:(NSURL *)fileURL cameraService:(QT_PREPEND_NAMESPACE(AVFCameraService) *)service @@ -97,10 +94,10 @@ QT_END_NAMESPACE videoSettings:(NSDictionary *)videoSettings transform:(CGAffineTransform)transform; +// This to be called from the recorder control's thread: - (void)start; - (void)stop; -// This to be called if control's dtor gets called, -// on the control's thread. +// This to be called from the recorder control's dtor: - (void)abort; @end diff --git a/src/plugins/avfoundation/camera/avfmediaassetwriter.mm b/src/plugins/avfoundation/camera/avfmediaassetwriter.mm index 4e227114b..464d90db0 100644 --- a/src/plugins/avfoundation/camera/avfmediaassetwriter.mm +++ b/src/plugins/avfoundation/camera/avfmediaassetwriter.mm @@ -85,17 +85,11 @@ enum WriterState @implementation QT_MANGLE_NAMESPACE(AVFMediaAssetWriter) -- (id)initWithQueue:(dispatch_queue_t)writerQueue - delegate:(AVFMediaRecorderControlIOS *)delegate +- (id)initWithDelegate:(AVFMediaRecorderControlIOS *)delegate { - Q_ASSERT(writerQueue); Q_ASSERT(delegate); if (self = [super init]) { - // "Shared" queue: - dispatch_retain(writerQueue); - m_writerQueue.reset(writerQueue); - m_delegate = delegate; m_setStartTime = true; m_state.store(WriterStateIdle); @@ -126,6 +120,12 @@ enum WriterState m_audioSettings = audioSettings; m_videoSettings = videoSettings; + m_writerQueue.reset(dispatch_queue_create("asset-writer-queue", DISPATCH_QUEUE_SERIAL)); + if (!m_writerQueue) { + qDebugCamera() << Q_FUNC_INFO << "failed to create an asset writer's queue"; + return false; + } + m_videoQueue.reset(dispatch_queue_create("video-output-queue", DISPATCH_QUEUE_SERIAL)); if (!m_videoQueue) { qDebugCamera() << Q_FUNC_INFO << "failed to create video queue"; @@ -172,20 +172,10 @@ enum WriterState - (void)start { - // 'start' is executed on a special writer's queue. - // We lock a mutex for the whole function body - // - render control probably was deleted - // 'immediately' after submitting 'start' - // and before 'start' is actually executed ... - // So if we're starting, render control can not - // abort us 'in the middle'. - const QMutexLocker lock(&m_writerMutex); - if (m_state.load() == WriterStateAborted) - return; - [self setQueues]; m_setStartTime = true; + m_state.storeRelease(WriterStateActive); [m_assetWriter startWriting]; @@ -196,40 +186,65 @@ enum WriterState - (void)stop { - // To be executed on a writer's queue. - { - const QMutexLocker lock(&m_writerMutex); - if (m_state.load() != WriterStateActive) + if (m_state.loadAcquire() != WriterStateActive) return; - } + if ([m_assetWriter status] != AVAssetWriterStatusWriting) + return; + + // Do this here so that - + // 1. '-abort' should not try calling finishWriting again and + // 2. async block (see below) will know if recorder control was deleted + // before the block's execution: + m_state.storeRelease(WriterStateIdle); + // Now, since we have to ensure no sample buffers are + // appended after a call to finishWriting, we must + // ensure writer's queue sees this change in m_state + // _before_ we call finishWriting: + dispatch_sync(m_writerQueue, ^{}); + // Done, but now we also want to prevent video queue + // from updating our viewfinder: + dispatch_sync(m_videoQueue, ^{}); + + // Now we're safe to stop: [m_assetWriter finishWritingWithCompletionHandler:^{ // This block is async, so by the time it's executed, // it's possible that render control was deleted already ... - // We lock so that nobody can call 'abort' in the middle ... - const QMutexLocker lock(&m_writerMutex); - if (m_state.load() != WriterStateActive) + if (m_state.loadAcquire() == WriterStateAborted) return; AVCaptureSession *session = m_service->session()->captureSession(); - [session stopRunning]; + if (session.running) + [session stopRunning]; [session removeOutput:m_audioOutput]; [session removeInput:m_audioInput]; - m_state.storeRelease(WriterStateIdle); QMetaObject::invokeMethod(m_delegate, "assetWriterFinished", Qt::QueuedConnection); }]; } - (void)abort { - // To be executed on any thread (presumably, it's the main thread), - // prevents writer from accessing any shared object. - const QMutexLocker lock(&m_writerMutex); + // -abort is to be called from recorder control's dtor. + if (m_state.fetchAndStoreRelease(WriterStateAborted) != WriterStateActive) { // Not recording, nothing to stop. return; } + // From Apple's docs: + // "To guarantee that all sample buffers are successfully written, + // you must ensure that all calls to appendSampleBuffer: and + // appendPixelBuffer:withPresentationTime: have returned before + // invoking this method." + // + // The only way we can ensure this is: + dispatch_sync(m_writerQueue, ^{}); + // At this point next block (if any) on the writer's queue + // will see m_state preventing it from any further processing. + dispatch_sync(m_videoQueue, ^{}); + // After this point video queue will not try to modify our + // viewfider, so we're safe to delete now. + [m_assetWriter finishWritingWithCompletionHandler:^{ }]; } @@ -240,13 +255,12 @@ enum WriterState Q_ASSERT(m_setStartTime); Q_ASSERT(sampleBuffer); - const QMutexLocker lock(&m_writerMutex); - if (m_state.load() != WriterStateActive) + if (m_state.loadAcquire() != WriterStateActive) return; QMetaObject::invokeMethod(m_delegate, "assetWriterStarted", Qt::QueuedConnection); - m_durationInMs.store(0); + m_durationInMs.storeRelease(0); m_startTime = CMSampleBufferGetPresentationTimeStamp(sampleBuffer); m_lastTimeStamp = m_startTime; [m_assetWriter startSessionAtSourceTime:m_startTime]; @@ -255,9 +269,9 @@ enum WriterState - (void)writeVideoSampleBuffer:(CMSampleBufferRef)sampleBuffer { + // This code is executed only on a writer's queue. Q_ASSERT(sampleBuffer); - // This code is executed only on a writer's queue. if (m_state.loadAcquire() == WriterStateActive) { if (m_setStartTime) [self setStartTimeFrom:sampleBuffer]; @@ -268,7 +282,6 @@ enum WriterState } } - CFRelease(sampleBuffer); } @@ -296,9 +309,6 @@ enum WriterState { Q_UNUSED(connection) - // This method can be called on either video or audio queue, - // never on a writer's queue, it needs access to a shared data, so - // lock is required. if (m_state.loadAcquire() != WriterStateActive) return; @@ -310,7 +320,6 @@ enum WriterState CFRetain(sampleBuffer); if (captureOutput != m_audioOutput.data()) { - const QMutexLocker lock(&m_writerMutex); if (m_state.load() != WriterStateActive) { CFRelease(sampleBuffer); return; @@ -450,7 +459,7 @@ enum WriterState if (!CMTimeCompare(duration, kCMTimeInvalid)) return; - m_durationInMs.store(CMTimeGetSeconds(duration) * 1000); + m_durationInMs.storeRelease(CMTimeGetSeconds(duration) * 1000); m_lastTimeStamp = newTimeStamp; } } diff --git a/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.h b/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.h index 33cb08804..589810b07 100644 --- a/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.h +++ b/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.h @@ -91,8 +91,6 @@ private: void stopWriter(); AVFCameraService *m_service; - - AVFScopedPointer m_writerQueue; AVFScopedPointer m_writer; QUrl m_outputLocation; diff --git a/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.mm b/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.mm index e65c98257..292b449d2 100644 --- a/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.mm +++ b/src/plugins/avfoundation/camera/avfmediarecordercontrol_ios.mm @@ -86,13 +86,7 @@ AVFMediaRecorderControlIOS::AVFMediaRecorderControlIOS(AVFCameraService *service { Q_ASSERT(service); - m_writerQueue.reset(dispatch_queue_create("asset-writer-queue", DISPATCH_QUEUE_SERIAL)); - if (!m_writerQueue) { - qDebugCamera() << Q_FUNC_INFO << "failed to create an asset writer's queue"; - return; - } - - m_writer.reset([[QT_MANGLE_NAMESPACE(AVFMediaAssetWriter) alloc] initWithQueue:m_writerQueue delegate:this]); + m_writer.reset([[QT_MANGLE_NAMESPACE(AVFMediaAssetWriter) alloc] initWithDelegate:this]); if (!m_writer) { qDebugCamera() << Q_FUNC_INFO << "failed to create an asset writer"; return; @@ -289,9 +283,15 @@ void AVFMediaRecorderControlIOS::setState(QMediaRecorder::State state) Q_EMIT stateChanged(m_state); Q_EMIT statusChanged(m_lastStatus); - dispatch_async(m_writerQueue, ^{ - [m_writer start]; - }); + // Apple recommends to call startRunning and do all + // setup on a special queue, and that's what we had + // initially (dispatch_async to writerQueue). Unfortunately, + // writer's queue is not the only queue/thread that can + // access/modify the session, and as a result we have + // all possible data/race-conditions with Obj-C exceptions + // at best and something worse in general. + // Now we try to only modify session on the same thread. + [m_writer start]; } else { [session startRunning]; Q_EMIT error(QMediaRecorder::FormatError, tr("Failed to start recording")); @@ -318,7 +318,7 @@ void AVFMediaRecorderControlIOS::setMuted(bool muted) void AVFMediaRecorderControlIOS::setVolume(qreal volume) { - Q_UNUSED(volume); + Q_UNUSED(volume) qDebugCamera() << Q_FUNC_INFO << "not implemented"; } @@ -403,9 +403,7 @@ void AVFMediaRecorderControlIOS::stopWriter() Q_EMIT stateChanged(m_state); Q_EMIT statusChanged(m_lastStatus); - dispatch_async(m_writerQueue, ^{ - [m_writer stop]; - }); + [m_writer stop]; } } -- cgit v1.2.3 From d7d31d63db5f0029a4a5e24d998601baee8bade0 Mon Sep 17 00:00:00 2001 From: Anatoly Stolbov Date: Wed, 27 Jul 2016 11:01:51 +0300 Subject: Android camera: use closest viewfinder resolution For some cameras difference between preview aspect rate and capture aspect rate is more than 0.01. Therefore it is better to use preview size with closest aspect rate. Task-number: QTBUG-50813 Change-Id: I1284c8ec2be1aa160a656e396a52960fa06aaa56 Reviewed-by: Yoann Lopes --- .../android/src/mediacapture/qandroidcamerasession.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'src/plugins') diff --git a/src/plugins/android/src/mediacapture/qandroidcamerasession.cpp b/src/plugins/android/src/mediacapture/qandroidcamerasession.cpp index 7b065c8c4..df9f0367b 100644 --- a/src/plugins/android/src/mediacapture/qandroidcamerasession.cpp +++ b/src/plugins/android/src/mediacapture/qandroidcamerasession.cpp @@ -261,17 +261,28 @@ void QAndroidCameraSession::adjustViewfinderSize(const QSize &captureSize, bool // search for viewfinder resolution with the same aspect ratio const qreal aspectRatio = qreal(captureSize.width()) / qreal(captureSize.height()); QList previewSizes = m_camera->getSupportedPreviewSizes(); + qreal minAspectDiff = 1; + QSize closestResolution; for (int i = previewSizes.count() - 1; i >= 0; --i) { const QSize &size = previewSizes.at(i); - if (qAbs(aspectRatio - (qreal(size.width()) / size.height())) < 0.01) { + const qreal sizeAspect = qreal(size.width()) / size.height(); + if (qFuzzyCompare(aspectRatio, sizeAspect)) { adjustedViewfinderResolution = size; break; + } else if (minAspectDiff > qAbs(sizeAspect - aspectRatio)) { + closestResolution = size; + minAspectDiff = qAbs(sizeAspect - aspectRatio); } } if (!adjustedViewfinderResolution.isValid()) { qWarning("Cannot find a viewfinder resolution matching the capture aspect ratio."); - return; + if (closestResolution.isValid()) { + adjustedViewfinderResolution = closestResolution; + qWarning("Using closest viewfinder resolution."); + } else { + return; + } } } -- cgit v1.2.3