[Merge] lp:~jhodapp/qtubuntu-camera/audio-recording into lp:qtubuntu-camera

Manuel de la Peña manuel.delapena at canonical.com
Thu Jul 31 13:56:10 UTC 2014


Review: Needs Fixing



Diff comments:

> === modified file 'debian/control'
> --- debian/control	2014-05-29 08:35:50 +0000
> +++ debian/control	2014-07-31 13:31:20 +0000
> @@ -13,6 +13,7 @@
>                 pkg-config,
>                 qt5-default,
>                 qtmultimedia5-dev,
> +               libpulse-dev
>  Standards-Version: 3.9.4
>  Homepage: https://launchpad.net/qtubuntu-camera
>  # If you aren't a member of ~phablet-team but need to upload packaging changes,
> 
> === modified file 'src/aalcameraservice.cpp'
> --- src/aalcameraservice.cpp	2014-07-21 15:21:42 +0000
> +++ src/aalcameraservice.cpp	2014-07-31 13:31:20 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2013 Canonical, Ltd.
> + * Copyright (C) 2013-2014 Canonical, Ltd.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU Lesser General Public License as published by
> @@ -27,8 +27,8 @@
>  #include "aalvideoencodersettingscontrol.h"
>  #include "aalvideorenderercontrol.h"
>  #include "aalviewfindersettingscontrol.h"
> +#include "storagemanager.h"
>  #include "aalcameraexposurecontrol.h"
> -#include <storagemanager.h>
>  
>  #include <hybris/camera/camera_compatibility_layer.h>
>  
> 
> === modified file 'src/aalcameraservice.h'
> --- src/aalcameraservice.h	2014-07-21 15:21:42 +0000
> +++ src/aalcameraservice.h	2014-07-31 13:31:20 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2013 Canonical, Ltd.
> + * Copyright (C) 2013-2014 Canonical, Ltd.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU Lesser General Public License as published by
> 
> === modified file 'src/aalmediarecordercontrol.cpp'
> --- src/aalmediarecordercontrol.cpp	2014-06-19 20:54:56 +0000
> +++ src/aalmediarecordercontrol.cpp	2014-07-31 13:31:20 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2013 Canonical, Ltd.
> + * Copyright (C) 2013-2014 Canonical, Ltd.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU Lesser General Public License as published by
> @@ -19,10 +19,12 @@
>  #include "aalmetadatawritercontrol.h"
>  #include "aalvideoencodersettingscontrol.h"
>  #include "aalviewfindersettingscontrol.h"
> +#include "audiocapture.h"
>  #include "storagemanager.h"
>  
>  #include <QDebug>
>  #include <QFile>
> +#include <QThread>
>  #include <QTimer>
>  
>  #include <hybris/camera/camera_compatibility_layer.h>
> @@ -37,7 +39,7 @@
>  const int AalMediaRecorderControl::RECORDER_NOT_AVAILABLE_ERROR;
>  const int AalMediaRecorderControl::RECORDER_INITIALIZATION_ERROR;
>  
> -const int AalMediaRecorderControl::DURATION_UPDATE_INTERVALL;
> +const int AalMediaRecorderControl::DURATION_UPDATE_INTERVAL;
>  
>  const QLatin1String AalMediaRecorderControl::PARAM_AUDIO_BITRATE = QLatin1String("audio-param-encoding-bitrate");
>  const QLatin1String AalMediaRecorderControl::PARAM_AUDIO_CHANNELS = QLatin1String("audio-param-number-of-channels");
> @@ -55,10 +57,12 @@
>     : QMediaRecorderControl(parent),
>      m_service(service),
>      m_mediaRecorder(0),
> +    m_audioCapture(0),
>      m_duration(0),
>      m_currentState(QMediaRecorder::StoppedState),
>      m_currentStatus(QMediaRecorder::UnloadedStatus),
> -    m_recordingTimer(0)
> +    m_recordingTimer(0),
> +    m_workerThread(new QThread)
>  {
>  }
>  
> @@ -68,6 +72,8 @@
>  AalMediaRecorderControl::~AalMediaRecorderControl()
>  {
>      delete m_recordingTimer;
> +    delete m_workerThread;
> +    delete m_audioCapture;
>      deleteRecorder();

As a rule of thumd using deleteLater on a QObject is better than using delete. The main reason is that if the destructor is called before all QEvents are processed the main loop of qt will crash (due to using and old mem address).

>  }
>  
> @@ -143,6 +149,17 @@
>  }
>  
>  /*!
> + * \brief Starts the main microphone reader/writer loop in AudioCapture (run)
> + */
> +void AalMediaRecorderControl::onStartThread()
> +{
> +    qDebug() << "Starting microphone reader/writer worker thread";
> +    // Start the microphone read/write worker thread
> +    m_workerThread->start();
> +    Q_EMIT startWorkerThread();
> +}
> +
> +/*!
>   * \brief AalMediaRecorderControl::init makes sure the mediarecorder is
>   * initialized
>   */
> @@ -151,6 +168,34 @@
>      if (m_mediaRecorder == 0) {
>          m_mediaRecorder = android_media_new_recorder();
>  
> +        m_audioCapture = new AudioCapture(m_mediaRecorder);
> +
> +        if (m_audioCapture == 0) {

Please use nullptr over 0 or NULL.

> +            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_workerThread, SIGNAL(finished()), m_audioCapture, 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");
> @@ -163,7 +208,7 @@
>  
>      if (m_recordingTimer == 0) {
>          m_recordingTimer = new QTimer(this);
> -        m_recordingTimer->setInterval(DURATION_UPDATE_INTERVALL);
> +        m_recordingTimer->setInterval(DURATION_UPDATE_INTERVAL);
>          m_recordingTimer->setSingleShot(false);
>          QObject::connect(m_recordingTimer, SIGNAL(timeout()),
>                           this, SLOT(updateDuration()));
> @@ -196,6 +241,16 @@
>                                "handleError", Qt::QueuedConnection);
>  }
>  
> +MediaRecorderWrapper* AalMediaRecorderControl::mediaRecorder() const
> +{
> +    return m_mediaRecorder;
> +}

Should we use something like a QSharedPointer?? That way, if the class is delete the ref is valid on the class that called the property.

> +
> +AudioCapture *AalMediaRecorderControl::audioCapture()
> +{
> +    return m_audioCapture;

Should we use something like a QSharedPointer?? That way, if the class is delete the ref is valid on the class that called the property.

> +}
> +
>  /*!
>   * \reimp
>   */
> @@ -243,7 +298,7 @@
>  
>  void AalMediaRecorderControl::updateDuration()
>  {
> -    m_duration += DURATION_UPDATE_INTERVALL;
> +    m_duration += DURATION_UPDATE_INTERVAL;
>      Q_EMIT durationChanged(m_duration);
>  }
>  
> @@ -273,7 +328,7 @@
>   * FIXME add support for recording audio only
>   */
>  int AalMediaRecorderControl::startRecording()
> -{    
> +{
>      if (m_service->androidControl() == 0) {
>          Q_EMIT error(RECORDER_INITIALIZATION_ERROR, "No camera connection");
>          return RECORDER_INITIALIZATION_ERROR;
> @@ -406,7 +461,13 @@
>   */
>  void AalMediaRecorderControl::stopRecording()
>  {
> +    qDebug() << __PRETTY_FUNCTION__;
>      if (m_mediaRecorder == 0) {

I would check against nullptr.

> +        qWarning() << "Can't stop recording properly, m_mediaRecorder is NULL";
> +        return;
> +    }
> +    if (m_audioCapture == 0) {

I would check against nullptr.

> +        qWarning() << "Can't stop recording properly, m_audioCapture is NULL";
>          return;
>      }
>  
> @@ -419,6 +480,12 @@
>          return;
>      }
>  
> +    // Stop microphone reader/writer loop
> +    // NOTE: This must come after the android_recorder_stop call, otherwise the
> +    // RecordThread instance will block the MPEG4Writer pthread_join when trying to
> +    // cleanly stop recording.
> +    m_audioCapture->stopCapture();
> +
>      android_recorder_reset(m_mediaRecorder);
>  
>      m_currentState = QMediaRecorder::StoppedState;
> @@ -438,3 +505,10 @@
>      QString param =  parameter + QChar('=') + QString::number(value);
>      android_recorder_setParameters(m_mediaRecorder, param.toLocal8Bit().data());
>  }
> +
> +void AalMediaRecorderControl::onStartThreadCb(void *context)
> +{
> +    AalMediaRecorderControl *thiz = static_cast<AalMediaRecorderControl*>(context);
> +    if (thiz != NULL)

I would check agsint nullptr instead of NULL.

> +        thiz->onStartThread();
> +}
> 
> === modified file 'src/aalmediarecordercontrol.h'
> --- src/aalmediarecordercontrol.h	2013-06-22 06:54:48 +0000
> +++ src/aalmediarecordercontrol.h	2014-07-31 13:31:20 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2013 Canonical, Ltd.
> + * Copyright (C) 2013-2014 Canonical, Ltd.
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU Lesser General Public License as published by
> @@ -28,6 +28,8 @@
>  struct CameraControl;
>  struct CameraControlListener;
>  struct MediaRecorderWrapper;
> +class AudioCapture;
> +class QThread;
>  class QTimer;
>  
>  class AalMediaRecorderControl : public QMediaRecorderControl
> @@ -49,11 +51,17 @@
>      static void errorCB(void* context);
>  
>      void init(CameraControl *control, CameraControlListener *listener);
> +    MediaRecorderWrapper* mediaRecorder() const;
> +    AudioCapture *audioCapture();

Why is this not a const method?

>  
>  public Q_SLOTS:
>      virtual void setMuted(bool muted);
>      virtual void setState(QMediaRecorder::State state);
>      virtual void setVolume(qreal gain);
> +    void onStartThread();
> +
> +signals:
> +    void startWorkerThread();
>  
>  private Q_SLOTS:
>      virtual void updateDuration();
> @@ -66,20 +74,23 @@
>      int startRecording();
>      void stopRecording();
>      void setParameter(const QString &parameter, int value);
> +    static void onStartThreadCb(void *context);
>  
>      AalCameraService *m_service;
>      MediaRecorderWrapper *m_mediaRecorder;
> +    AudioCapture *m_audioCapture;
>      QUrl m_outputLocation;
>      qint64 m_duration;
>      QMediaRecorder::State m_currentState;
>      QMediaRecorder::Status m_currentStatus;
>      QTimer *m_recordingTimer;
> +    QThread *m_workerThread;
>  
>      static const int RECORDER_GENERAL_ERROR = -1;
>      static const int RECORDER_NOT_AVAILABLE_ERROR = -2;
>      static const int RECORDER_INITIALIZATION_ERROR = -3;
>  
> -    static const int DURATION_UPDATE_INTERVALL = 1000; // update every second
> +    static const int DURATION_UPDATE_INTERVAL = 1000; // update every second

Do you want this to be in the class or can we move it to an anonymus namespace?

>  
>      static const QLatin1String PARAM_AUDIO_BITRATE;
>      static const QLatin1String PARAM_AUDIO_CHANNELS;
> 
> === added file 'src/audiocapture.cpp'
> --- src/audiocapture.cpp	1970-01-01 00:00:00 +0000
> +++ src/audiocapture.cpp	2014-07-31 13:31:20 +0000
> @@ -0,0 +1,217 @@
> +/*
> + * Copyright (C) 2014 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Jim Hodapp <jim.hodapp at canonical.com>
> + */
> +
> +#include "audiocapture.h"
> +
> +#include <pulse/simple.h>
> +#include <pulse/error.h>
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +
> +#include <QDebug>
> +#include <QThread>
> +
> +AudioCapture::AudioCapture(MediaRecorderWrapper *mediaRecorder)
> +    : m_paStream(NULL),
> +      m_audioPipe(-1),
> +      m_flagExit(false),
> +      m_mediaRecorder(mediaRecorder)
> +{
> +}
> +
> +AudioCapture::~AudioCapture()
> +{
> +    if (m_audioPipe >= 0)
> +        close(m_audioPipe);
> +    if (m_paStream != NULL)

I would check against nullptr.

> +        pa_simple_free(m_paStream);
> +}
> +
> +/*!
> + * \brief Initializes AudioCapture so that it's ready to read microphone data from Pulseaudio
> + */
> +bool AudioCapture::init(StartWorkerThreadCb cb, void *context)
> +{
> +    // The MediaRecorderLayer will call method (cb) when it's ready to encode a new audio buffer
> +    android_recorder_set_audio_read_cb(m_mediaRecorder, cb, context);
> +
> +    return true;

Always return true??

> +}
> +
> +/*!
> + * \brief Stops the microphone data capture thread loop
> + */
> +void AudioCapture::stopCapture()
> +{
> +    qDebug() << __PRETTY_FUNCTION__;
> +    m_flagExit = true;
> +}
> +
> +/*!
> + * \brief The main microphone reader/writer loop. Reads from Pulseaudio, writes to named pipe
> + */
> +void AudioCapture::run()
> +{
> +    qDebug() << __PRETTY_FUNCTION__;
> +
> +    int bytesWritten = 0, bytesRead = 0;
> +    const size_t readSize = sizeof(m_audioBuf);
> +
> +    if (!setupMicrophoneStream())
> +    {
> +        qWarning() << "Failed to setup PulseAudio microphone recording stream";
> +        return;
> +    }
> +
> +    if (!setupPipe())
> +    {
> +        qWarning() << "Failed to open /dev/socket/micshm, cannot write data to pipe";
> +        return;
> +    }
> +
> +    do {
> +        bytesRead = readMicrophone();
> +        if (bytesRead > 0)
> +        {
> +            bytesWritten = writeDataToPipe();
> +        }
> +    } while (bytesRead == readSize
> +                && bytesWritten == readSize
> +                && !m_flagExit);
> +
> +    Q_EMIT finished();
> +}
> +
> +/*!
> + * \brief Reads microphone data from Pulseaudio
> + */
> +int AudioCapture::readMicrophone()
> +{
> +    int ret = 0, error = 0;
> +    const size_t readSize = sizeof(m_audioBuf);
> +    ret = pa_simple_read(m_paStream, m_audioBuf, readSize, &error);
> +    if (ret < 0)
> +    {
> +        qWarning() << "Failed to read audio from the microphone: " << pa_strerror(error);
> +        goto exit;

Why not remove the goto and simple do the return here? return ret; would have the same effect.

> +    }
> +    else
> +        ret = readSize;
> +
> +exit:
> +    return ret;
> +}
> +
> +/*!
> + * \brief Signals AalMediaRecorderControl to start the main thread loop.
> + * \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()
> +{
> +    // FIXME: Get these parameters more dynamically from the control
> +    static const pa_sample_spec ss = {
> +        .format = PA_SAMPLE_S16LE,
> +        .rate = 48000,
> +        .channels = 1
> +    };
> +    int error = 0;
> +
> +    m_paStream = pa_simple_new(NULL, "qtubuntu-camera", PA_STREAM_RECORD, NULL, "record", &ss, NULL, NULL, &error);
> +    if (m_paStream == NULL)

I would check against nullptr.

> +    {
> +        qWarning() << "Failed to open a PulseAudio channel to read the microphone: " << pa_strerror(error);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +/*!
> + * \brief Opens the named pipe /dev/socket/micshm for writing mic data to the Android (reader) side
> + */
> +bool AudioCapture::setupPipe()
> +{
> +    if (m_audioPipe >= 0)
> +    {
> +        qWarning() << "/dev/socket/micshm already opened, not opening twice";
> +        return true;
> +    }
> +
> +    // Open the named pipe for writing only
> +    m_audioPipe = open("/dev/socket/micshm", O_WRONLY);
> +    if (m_audioPipe < 0)
> +    {
> +        qWarning() << "Failed to open audio data pipe /dev/socket/micshm: " << strerror(errno);
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +/*!
> + * \brief Writes mic data to the named pipe /dev/socket/micshm
> + */
> +int AudioCapture::writeDataToPipe()
> +{
> +    // Don't open the named pipe twice
> +    if (m_audioPipe < 0)

I'd do a if (m_audioPipe < 0 && !setupPipe()) { return 0; } the second part of the && clause wont be executed if m_audioPipe >= 0 and we will reduce the complexity.

> +    {
> +        if (!setupPipe())
> +        {
> +            qWarning() << "Failed to open /dev/socket/micshm, cannot write data to pipe";
> +            return 0;
> +        }
> +    }
> +
> +    int num = 0;
> +    const size_t writeSize = sizeof(m_audioBuf);
> +    num = loopWrite(m_audioPipe, m_audioBuf, writeSize);
> +    if (num != writeSize)
> +        qWarning() << "Failed to write " << num << " bytes to /dev/socket/micshm: " << strerror(errno) << " (" << errno << ")";
> +
> +    return num;
> +}
> +
> +ssize_t AudioCapture::loopWrite(int fd, const void *data, size_t size)
> +{
> +    ssize_t ret = 0;
> +    while (size > 0)
> +    {
> +        ssize_t r;
> +        if ((r = write(fd, data, size)) < 0)
> +            return r;
> +        if (r == 0)
> +            break;
> +        ret += r;
> +        data = (const int16_t*) data + r;
> +        size -= (size_t) r;
> +    }
> +    return ret;
> +}
> 
> === added file 'src/audiocapture.h'
> --- src/audiocapture.h	1970-01-01 00:00:00 +0000
> +++ src/audiocapture.h	2014-07-31 13:31:20 +0000
> @@ -0,0 +1,71 @@
> +/*
> + * Copyright (C) 2014 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Jim Hodapp <jim.hodapp at canonical.com>
> + */
> +
> +#ifndef AUDIOCAPTURE_H
> +#define AUDIOCAPTURE_H
> +
> +#include <hybris/media/media_recorder_layer.h>
> +
> +#include <stdint.h>
> +
> +#include <QObject>
> +
> +class AalMediaRecorderControl;
> +struct MediaRecorderWrapper;
> +
> +struct pa_simple;
> +
> +class AudioCapture : public QObject
> +{
> +    Q_OBJECT
> +
> +    typedef void (*StartWorkerThreadCb)(void *context);
> +public:
> +    explicit AudioCapture(MediaRecorderWrapper *mediaRecorder);
> +    ~AudioCapture();
> +
> +    bool init(StartWorkerThreadCb cb, void *context);
> +    /* Terminates the Pulseaudio reader/writer QThread */
> +    void stopCapture();
> +
> +signals:
> +    void startThread();
> +    void finished();
> +
> +public Q_SLOTS:
> +    void run();
> +
> +private Q_SLOTS:
> +    void startThreadLoop();
> +
> +private:
> +    int readMicrophone();
> +    bool setupMicrophoneStream();
> +    bool setupPipe();
> +    ssize_t loopWrite(int fd, const void *data, size_t len);
> +    int writeDataToPipe();
> +
> +    pa_simple *m_paStream;
> +    int16_t m_audioBuf[MIC_READ_BUF_SIZE];
> +
> +    int m_audioPipe;
> +    bool m_flagExit;
> +    MediaRecorderWrapper *m_mediaRecorder;
> +};
> +
> +#endif // AUDIOCAPTURE_H
> 
> === modified file 'src/src.pro'
> --- src/src.pro	2014-07-08 15:12:58 +0000
> +++ src/src.pro	2014-07-31 13:31:20 +0000
> @@ -2,7 +2,7 @@
>  TARGET = aalcamera
>  TEMPLATE = lib
>  CONFIG += plugin
> -QT += multimedia opengl
> +QT += concurrent multimedia opengl
>  
>  PLUGIN_TYPE = mediaservice
>  
> @@ -14,7 +14,9 @@
>      -lcamera \
>      -lmedia \
>      -lubuntu_application_api \
> -    -lqtubuntu-media-signals
> +    -lqtubuntu-media-signals \
> +    -lpulse \
> +    -lpulse-simple
>  
>  OTHER_FILES += aalcamera.json
>  
> @@ -33,6 +35,7 @@
>      aalvideoencodersettingscontrol.h \
>      aalvideorenderercontrol.h \
>      aalviewfindersettingscontrol.h \
> +    audiocapture.h \
>      aalcameraexposurecontrol.h \
>      storagemanager.h
>  
> @@ -51,5 +54,6 @@
>      aalvideoencodersettingscontrol.cpp \
>      aalvideorenderercontrol.cpp \
>      aalviewfindersettingscontrol.cpp \
> +    audiocapture.cpp \
>      aalcameraexposurecontrol.cpp \
>      storagemanager.cpp
> 
> === modified file 'unittests/aalmediarecordercontrol/aalmediarecordercontrol.pro'
> --- unittests/aalmediarecordercontrol/aalmediarecordercontrol.pro	2014-01-16 14:20:08 +0000
> +++ unittests/aalmediarecordercontrol/aalmediarecordercontrol.pro	2014-07-31 13:31:20 +0000
> @@ -12,14 +12,16 @@
>      ../../src/aalcameraservice.h \
>      ../../src/aalvideoencodersettingscontrol.h \
>      ../../src/aalmetadatawritercontrol.h \
> +    ../../src/audiocapture.h \

You can use ${CMAKE_SOURCE_DIR} instead of ../../ which is more mantainable.

>      ../../src/storagemanager.h
>  
>  SOURCES += tst_aalmediarecordercontrol.cpp \
> +    ../stubs/audiocapture_stub.cpp \
>      ../../src/aalmediarecordercontrol.cpp \
>      ../stubs/aalcameraservice_stub.cpp \
>      ../stubs/aalvideoencodersettingscontrol_stub.cpp \
>      ../stubs/aalmetadatawritercontrol_stub.cpp \
> -    ../stubs/storagemanager_stub.cpp 
> +    ../stubs/storagemanager_stub.cpp

Similar, you can use ${CMAKE_SOURCE_DIR}.

>  
>  check.depends = $${TARGET}
>  check.commands = ./$${TARGET}
> 
> === added file 'unittests/stubs/audiocapture_stub.cpp'
> --- unittests/stubs/audiocapture_stub.cpp	1970-01-01 00:00:00 +0000
> +++ unittests/stubs/audiocapture_stub.cpp	2014-07-31 13:31:20 +0000
> @@ -0,0 +1,54 @@
> +/*
> + * Copyright (C) 2014 Canonical, Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "audiocapture.h"
> +
> +#include <QThread>
> +
> +#include <hybris/media/media_recorder_layer.h>
> +
> +AudioCapture::AudioCapture(MediaRecorderWrapper *mediaRecorder)
> +{
> +    Q_UNUSED(mediaRecorder);
> +}
> +
> +AudioCapture::~AudioCapture()
> +{
> +}
> +
> +bool AudioCapture::init(StartWorkerThreadCb cb, void *context)
> +{
> +    Q_UNUSED(cb);
> +    Q_UNUSED(context);
> +    return true;
> +}
> +
> +void AudioCapture::stopCapture()
> +{
> +}
> +
> +void AudioCapture::run()
> +{
> +}
> +
> +void AudioCapture::startThreadLoop()
> +{
> +}
> +
> +int AudioCapture::readMicrophone()
> +{
> +    return 0;
> +}
> 


-- 
https://code.launchpad.net/~jhodapp/qtubuntu-camera/audio-recording/+merge/228935
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~jhodapp/qtubuntu-camera/audio-recording into lp:qtubuntu-camera.



More information about the Ubuntu-reviews mailing list