[Merge] lp:~phablet-team/qtubuntu-camera/fix-trust-prompt-bugs into lp:qtubuntu-camera/stable

Florian Boucault florian.boucault at canonical.com
Fri Aug 21 20:08:45 UTC 2015



Diff comments:

> 
> === modified file 'src/aalmediarecordercontrol.cpp'
> --- src/aalmediarecordercontrol.cpp	2015-01-26 16:21:39 +0000
> +++ src/aalmediarecordercontrol.cpp	2015-08-21 19:32:33 +0000
> @@ -158,76 +159,35 @@
>  /*!
>   * \brief Starts the main microphone reader/writer loop in AudioCapture (run)
>   */
> -void AalMediaRecorderControl::onStartThread()
> +void AalMediaRecorderControl::startAudioCaptureThread()
>  {
> -    qDebug() << "Starting microphone reader/writer worker thread";
> -    // Start the microphone read/write worker thread
> -    m_workerThread->start();
> -    Q_EMIT startWorkerThread();
> +    qDebug() << "Starting microphone reader/writer thread";
> +    // Start the microphone read/write thread
> +    m_audioCaptureThread.start();
> +    Q_EMIT audioCaptureThreadStarted();
>  }
>  
>  /*!
>   * \brief AalMediaRecorderControl::init makes sure the mediarecorder is
>   * initialized
>   */
> -void AalMediaRecorderControl::initRecorder()
> +bool AalMediaRecorderControl::initRecorder()
>  {
>      if (m_mediaRecorder == 0) {
>          m_mediaRecorder = android_media_new_recorder();
> -
> -        m_audioCapture = new AudioCapture(m_mediaRecorder);
> -        m_workerThread = new QThread;
> -
> -        if (m_audioCapture == 0) {
> -            qWarning() << "Unable to create new audio capture, audio recording won't function";
> -            Q_EMIT error(RECORDER_INITIALIZATION_ERROR, "Unable to create new audio capture, audio recording won't function");
> -        }
> -        else
> -        {
> -            bool ret = false;
> -
> -            // Make sure that m_audioCapture is executed within the m_workerThread affinity
> -            m_audioCapture->moveToThread(m_workerThread);
> -
> -            // Finished signal is for when the workerThread is completed. Important to connect this so that
> -            // resources are cleaned up in the proper order and not leaked
> -            ret = connect(m_audioCapture, SIGNAL(finished()), m_workerThread, SLOT(quit()));
> -            if (!ret)
> -                qWarning() << "Failed to connect quit() to the m_audioCapture finished signal";
> -            ret = connect(m_audioCapture, SIGNAL(finished()), m_audioCapture, SLOT(deleteLater()));
> -            if (!ret)
> -                qWarning() << "Failed to connect deleteLater() to the m_audioCapture finished signal";
> -            // Clean up the worker thread after we finish recording

I'm pretty sure it's cleaned up properly. Also it's easier now that the thread is not destroyed dynamically.

> -            ret = connect(m_workerThread, SIGNAL(finished()), m_workerThread, SLOT(deleteLater()));
> -            if (!ret)
> -                qWarning() << "Failed to connect deleteLater() to the m_workerThread finished signal";
> -            // startWorkerThread signal comes from an Android layer callback that resides down in
> -            // the AudioRecordHybris class
> -            ret = connect(this, SIGNAL(startWorkerThread()), m_audioCapture, SLOT(run()));
> -            if (!ret)
> -                qWarning() << "Failed to connect run() to the local startWorkerThread signal";
> -
> -            // Call onStartThreadCb when the reader side of the named pipe has been setup
> -            m_audioCapture->init(&AalMediaRecorderControl::onStartThreadCb, this);
> -        }
> -
>          if (m_mediaRecorder == 0) {
>              qWarning() << "Unable to create new media recorder";
>              Q_EMIT error(RECORDER_INITIALIZATION_ERROR, "Unable to create new media recorder");
> -        } else {
> -            setStatus(QMediaRecorder::LoadedStatus);
> -            android_recorder_set_error_cb(m_mediaRecorder, &AalMediaRecorderControl::errorCB, this);
> -            android_camera_unlock(m_service->androidControl());
> +            return false;
>          }
> -    }
> -
> -    if (m_recordingTimer == 0) {
> -        m_recordingTimer = new QTimer(this);
> -        m_recordingTimer->setInterval(DURATION_UPDATE_INTERVAL);
> -        m_recordingTimer->setSingleShot(false);
> -        QObject::connect(m_recordingTimer, SIGNAL(timeout()),
> -                         this, SLOT(updateDuration()));
> -    }
> +
> +        m_audioCaptureAvailable = initAudioCapture();
> +
> +        android_recorder_set_error_cb(m_mediaRecorder, &AalMediaRecorderControl::errorCB, this);
> +        android_camera_unlock(m_service->androidControl());
> +    }
> +
> +    return true;
>  }
>  
>  /*!
> @@ -245,6 +207,42 @@
>      setStatus(QMediaRecorder::UnloadedStatus);
>  }
>  
> +bool AalMediaRecorderControl::initAudioCapture()
> +{
> +    // setting up audio recording; m_audioCapture is executed within the m_workerThread affinity
> +    m_audioCapture = new AudioCapture(m_mediaRecorder);
> +    if (!m_audioCapture->setupMicrophoneStream())
> +    {
> +        qWarning() << "Failed to setup PulseAudio microphone recording stream";
> +        delete m_audioCapture;
> +        m_audioCapture = 0;
> +        return false;
> +    } else {
> +        m_audioCapture->moveToThread(&m_audioCaptureThread);
> +
> +        // startWorkerThread signal comes from an Android layer callback that resides down in
> +        // the AudioRecordHybris class
> +        connect(this, SIGNAL(audioCaptureThreadStarted()), m_audioCapture, SLOT(run()));
> +
> +        // Call recorderReadAudioCallback when the reader side of the named pipe has been setup
> +        m_audioCapture->init(&AalMediaRecorderControl::recorderReadAudioCallback, this);
> +        return true;
> +    }
> +}
> +
> +void AalMediaRecorderControl::deleteAudioCapture()
> +{
> +    if (m_audioCapture == 0)
> +        return;
> +
> +    m_audioCapture->stopCapture();
> +    m_audioCaptureThread.quit();
> +    m_audioCaptureThread.wait();
> +

Fix pushed.

> +    delete m_audioCapture;
> +    m_audioCapture = 0;
> +}
> +
>  /*!
>   * \brief AalMediaRecorderControl::errorCB handles errors from the android layer
>   * \param context
> 
> === modified file 'src/audiocapture.cpp'
> --- src/audiocapture.cpp	2014-12-03 17:08:38 +0000
> +++ src/audiocapture.cpp	2015-08-21 19:32:33 +0000
> @@ -122,18 +117,6 @@
>  }
>  
>  /*!
> - * \brief Signals AalMediaRecorderControl to start the main thread loop.

I don't understand the comment. Can you write at the right place and push it?

> - * \detail This is necessary due to thread contexts. Starting of the main thread loop
> - * for AudioCapture must be done in the main thread context and not in the AudioCapture
> - * thread context, otherwise the loop start signal will never be seen.
> - */
> -void AudioCapture::startThreadLoop()
> -{
> -    Q_EMIT startThread();
> -    qDebug() << "Emitted startThread(), should start reading from mic";
> -}
> -
> -/*!
>   * \brief Sets up the Pulseaudio microphone input channel
>   */
>  bool AudioCapture::setupMicrophoneStream()


-- 
https://code.launchpad.net/~phablet-team/qtubuntu-camera/fix-trust-prompt-bugs/+merge/268776
Your team Ubuntu Phablet Team is subscribed to branch lp:qtubuntu-camera/stable.



More information about the Ubuntu-reviews mailing list