Switch to using Qt's QAudioOutput for outputHEADflounder_5.99.1flounder/5.99.1dab_3.99.1dab/3.99.15.99.13.99.1master

The underlying issue in the hang reported in SPEC-455 is that due to
the synchronous nature of the pa_simple_* PulseAudio API, the
pa_simple_write call used blocks when a stream is corked .  That
prevents the tuner plugin's output thread from exiting when playback is
stopped, resulting in the observed hang.  After examining the available
options, it seemed like switching to Qt's QAudioOutput class made sense
since it allows using the asynchronous PulseAudio API easily, and like
the QRadio class the tuner plugin implements, it is part of QtMultimedia
itself.

Note that the radio_output.* files have been removed as the code is no
longer used, and a new pair of OutputBuffer source files have been added
to contain the small class that is used to connect the RTL-SDR output to
QAudioOutput.

Bug-AGL: SPEC-455

Change-Id: I0d690143b9c70fdca24f9fbf3b016feef8ae627b
Signed-off-by: Scott Murray <scott.murray@konsulko.com>

6 files changed, 189 insertions, 265 deletions

diff --git a/OutputBuffer.cpp b/OutputBuffer.cpp
new file mode 100644
index 0000000..c6fe30a
--- /dev/null
+++ b/OutputBuffer.cpp
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2017 Konsulko Group
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <OutputBuffer.h>
+
+OutputBuffer::OutputBuffer(const QAudioFormat &format, QObject *parent)
+  : QIODevice(parent)
+{
+    m_sample_size = format.bytesPerFrame();
+}
+
+OutputBuffer::~OutputBuffer()
+{
+}
+
+void OutputBuffer::start()
+{
+    open(QIODevice::ReadWrite);
+}
+
+void OutputBuffer::stop()
+{
+    close();
+    QMutexLocker locker(&m_mutex);
+    m_buffer.clear();
+}
+
+qint64 OutputBuffer::readData(char *data, qint64 len)
+{
+    QMutexLocker locker(&m_mutex);
+    int n = qMin((qint64) m_buffer.size(), len);
+
+    // Make sure reads are in units of the sample size
+    n = n - (n % m_sample_size);
+
+    if (!m_buffer.isEmpty()) {
+        memcpy(data, m_buffer.constData(), n);
+	m_buffer.remove(0, n);
+    }
+    return n;
+}
+
+qint64 OutputBuffer::writeData(const char *data, qint64 len)
+{
+    QMutexLocker locker(&m_mutex);
+    m_buffer.append(data, len);
+    return len;
+}
+
+qint64 OutputBuffer::bytesAvailable()
+{
+    QMutexLocker locker(&m_mutex);
+    return m_buffer.size() + QIODevice::bytesAvailable();
+}
diff --git a/OutputBuffer.h b/OutputBuffer.h
new file mode 100644
index 0000000..d1773ae
--- /dev/null
+++ b/OutputBuffer.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2017 Konsulko Group
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+/*
+ * Simple buffer device to act as input device for QAudioInput.
+ *
+ * It basically wraps QByteArray with some additional locking that
+ * the QBuffer class does not provide.  It also ensures that reads
+ * return in units of the given format's sample size.  This is
+ * required to handle the RTL-SDR library output sometimes arriving
+ * with half a sample at the end.  QAudioOutput will blindly pass
+ * that half sample along and trigger a PulseAudio error if we do
+ * not handle it here.
+ */
+
+#ifndef OUTPUTBUFFER_H
+#define OUTPUTBUFFER_H
+
+#include <QtCore/QIODevice>
+#include <QtCore/QMutex>
+#include <QtMultimedia/QAudioFormat>
+
+class OutputBuffer : public QIODevice
+{
+    Q_OBJECT
+
+public:
+    OutputBuffer(const QAudioFormat &format, QObject *parent);
+    ~OutputBuffer();
+
+    void start();
+    void stop();
+
+    qint64 readData(char *data, qint64 maxlen);
+    qint64 writeData(const char *data, qint64 len);
+    qint64 bytesAvailable();
+
+private:
+    QMutex m_mutex;
+    QByteArray m_buffer;
+    unsigned int m_sample_size;
+};
+
+#endif // OUTPUTBUFFER_H
diff --git a/radio_output.cpp b/radio_output.cpp
deleted file mode 100644
index 350bf75..0000000
--- a/radio_output.cpp
+++ /dev/null
@@ -1,152 +0,0 @@
-/*
- * A standalone AM/FM Radio QML plugin (for RTL2832U and Maxim hardware)
- * Copyright © 2015-2016 Manuel Bachmann <manuel.bachmann@iot.bzh>
- * Copyright © 2016 Scott Murray <scott.murray@konsulko.com>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option) any later version.
- *
- * 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 General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <iostream>
-#include "radio_output.h"
-#include "rtl_fm.h"
-
-RadioOutputAlsa::RadioOutputAlsa() : RadioOutputImplementation(),
-				     dev(NULL),
-				     hw_params(NULL)
-{
-	unsigned int rate = 24000;
-
-	if (snd_pcm_open(&dev, "default", SND_PCM_STREAM_PLAYBACK, 0) < 0) {
-		std::cerr << "Could not open primary ALSA device" << std::endl;
-		works = false;
-		return;
-	}
-
-	snd_pcm_hw_params_malloc(&hw_params);
-	snd_pcm_hw_params_any(dev, hw_params);
-
-	snd_pcm_hw_params_set_access (dev, hw_params, SND_PCM_ACCESS_RW_INTERLEAVED);
-	snd_pcm_hw_params_set_format (dev, hw_params, SND_PCM_FORMAT_S16_LE);
-	snd_pcm_hw_params_set_rate_near (dev, hw_params, &rate, 0);
-	snd_pcm_hw_params_set_channels (dev, hw_params, 2);
-
-	if (snd_pcm_hw_params (dev, hw_params) < 0) {
-		std::cerr << "Could not set hardware parameters" << std::endl;
-		works = false;
-	} else {
-		works = true;
-	}
-	snd_pcm_hw_params_free (hw_params);
-
-	snd_pcm_prepare (dev);
-}
-
-RadioOutputAlsa::~RadioOutputAlsa()
-{
-	snd_pcm_close (dev);
-}
-
-bool RadioOutputAlsa::play(void *buf, int len)
-{
-	int16_t *cbuf = (int16_t *)buf;
-	int frames = len / 2;
-	int res;
-
-	if ((res = snd_pcm_writei(dev, cbuf, frames)) != frames) {
-		snd_pcm_recover(dev, res, 0);
-		snd_pcm_prepare(dev);
-	}
-	//snd_pcm_drain(dev);
-
-	return true;
-}
-
-
-RadioOutputPulse::RadioOutputPulse() : RadioOutputImplementation(),
-				       pa(NULL),
-				       pa_spec(NULL)
-{
-	int error;
-
-	pa_spec = (pa_sample_spec*) malloc(sizeof(pa_sample_spec));
-	pa_spec->format = PA_SAMPLE_S16LE;
-	pa_spec->rate = 24000;
-	pa_spec->channels = 2;
-
-	if (!(pa = pa_simple_new(NULL, "qtmultimedia-rtlfm-radio-plugin", PA_STREAM_PLAYBACK, NULL,
-	                         "radio-output", pa_spec, NULL, NULL, &error))) {
-		std::cerr << "Error connecting to PulseAudio : " << pa_strerror(error) << std::endl;
-		works = false;
-	} else {
-		std::cerr << "RadioOutputPulse::RadioOutputPulse: Connected to PulseAudio" << std::endl;
-		works = true;
-	}
-
-	extra = 0;
-	output_buf = new unsigned char[RTL_FM_MAXIMUM_BUF_LENGTH];
-
-	free(pa_spec);	
-}
-
-RadioOutputPulse::~RadioOutputPulse()
-{
-	pa_simple_free(pa);
-	delete [] output_buf;
-}
-
-bool RadioOutputPulse::play(void *buf, int len)
-{
-	int error;
-	size_t n = len * 2;
-	void *p;
-
-	if (!buf) {
-		std::cerr << "Error buf == null!" << std::endl;
-		return false;
-	}
-
-	// Handle the rtl_fm code giving us an odd number of samples, which
-	// PA does not like.  This extra buffer copying approach is not
-	// particularly efficient, but works for now.  It looks feasible to
-	// hack in something in the demod and output thread routines in
-	// rtl_fm.c to handle it there if more performance is required.
-	p = output_buf;
-	if(extra) {
-		memcpy(output_buf, extra_buf, sizeof(int16_t));
-		if((extra + len) % 2) {
-			// We end up with len + 1 samples, n remains the same, store the extra
-			memcpy(output_buf + sizeof(int16_t), buf, n - 2);
-			memcpy(extra_buf, ((unsigned char*) buf) + n - 2, sizeof(int16_t));
-		} else {
-			// We end up with an extra sample
-			memcpy(output_buf + sizeof(int16_t), buf, n);
-			n += 2;
-			extra = 0;
-		}
-	} else if(len % 2) {
-		// We have an extra sample, store it, and decrease n
-		n -= 2;
-		memcpy(output_buf + sizeof(int16_t), buf, n);
-		memcpy(extra_buf, ((unsigned char*) buf) + n, sizeof(int16_t));
-		extra = 1;
-	} else {
-		p = buf;
-	}
-
-	if (pa_simple_write(pa, p, n, &error) < 0)
-		std::cerr << "Error writing " << n << " bytes to PulseAudio : " << pa_strerror(error) << std::endl;
-	//pa_simple_drain(pa, &error);
-
-	return true;
-}
diff --git a/radio_output.h b/radio_output.h
deleted file mode 100644
index 5051101..0000000
--- a/radio_output.h
+++ /dev/null
@@ -1,75 +0,0 @@
-/*
- * A standalone AM/FM Radio QML plugin (for RTL2832U and Maxim hardware)
- * Copyright © 2015-2016 Manuel Bachmann <manuel.bachmann@iot.bzh>
- * Copyright © 2016 Scott Murray <scott.murray@konsulko.com>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option) any later version.
- *
- * 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 General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef RADIO_OUTPUT_H
-#define RADIO_OUTPUT_H
-
-#include <alsa/asoundlib.h>
-
-#include <pulse/simple.h>
-#include <pulse/error.h>
-
- /*
-  * RadioOutputImplementation is a virtual class, with 2 implementations :
-  * - RadioOutputAlsa ;
-  * - RadioOutputPulse ;
-  */
-class RadioOutputImplementation
-{
-public:
-	RadioOutputImplementation() {};
-	virtual ~RadioOutputImplementation() {};
-
-	virtual bool play(void*, int) = 0;
-
-	bool works;
-};
-
-
-class RadioOutputAlsa : public RadioOutputImplementation
-{
-public:
-	RadioOutputAlsa();
-	~RadioOutputAlsa() override;
-
-private:
-	bool play(void *, int) override;
-
-	snd_pcm_t *dev;
-	snd_pcm_hw_params_t *hw_params;
-};
-
-class RadioOutputPulse : public RadioOutputImplementation
-{
-public:
-	RadioOutputPulse();
-	~RadioOutputPulse() override;
-
-private:
-	bool play(void *, int) override;
-
-	pa_simple *pa;
-	pa_sample_spec *pa_spec;
-
-	unsigned int extra;
-	int16_t extra_buf[1];
-	unsigned char *output_buf;
-};
-
-#endif  // RTLSDR_RADIO_H
diff --git a/rtlfmradio.pro b/rtlfmradio.pro
index 5824ec7..200984e 100644
--- a/rtlfmradio.pro
+++ b/rtlfmradio.pro
@@ -1,11 +1,11 @@
 TEMPLATE = lib
 CONFIG += plugin c++11 link_pkgconfig
-PKGCONFIG += librtlsdr libpulse-simple alsa
+PKGCONFIG += librtlsdr
 TARGET = rtlfmradio
 QT = multimedia
 
-HEADERS = rtlfmradioplugin.h rtlfmradioservice.h rtlfmradiotunercontrol.h radio_output.h rtl_fm.h convenience/convenience.h
-SOURCES = rtlfmradioplugin.cpp rtlfmradioservice.cpp rtlfmradiotunercontrol.cpp radio_output.cpp rtl_fm.c convenience/convenience.c
+HEADERS = rtlfmradioplugin.h rtlfmradioservice.h rtlfmradiotunercontrol.h OutputBuffer.h rtl_fm.h convenience/convenience.h
+SOURCES = rtlfmradioplugin.cpp rtlfmradioservice.cpp rtlfmradiotunercontrol.cpp OutputBuffer.cpp rtl_fm.c convenience/convenience.c
 DISTFILES += rtlfmradio.json
 
 target.path = $$[QT_INSTALL_PLUGINS]/mediaservice
diff --git a/rtlfmradiotunercontrol.cpp b/rtlfmradiotunercontrol.cpp
index 27a8d08..937411a 100644
--- a/rtlfmradiotunercontrol.cpp
+++ b/rtlfmradiotunercontrol.cpp
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2016, The Qt Company Ltd. All Rights Reserved.
- * Copyright (C) 2016, Scott Murray <scott.murray@konsulko.com>
+ * Copyright (C) 2016, 2017 Konsulko Group
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -8,12 +8,13 @@
  */
 
 #include "rtlfmradiotunercontrol.h"
-#include "radio_output.h"
 #include "rtl_fm.h"
 
 #include <QtCore/QDebug>
 #include <QtCore/QTimer>
 #include <QtCore/QSettings>
+#include <QtMultimedia/QAudioOutput>
+#include "OutputBuffer.h"
 
 // Structure to describe FM band plans, all values in Hz.
 struct fmBandPlan {
@@ -22,12 +23,23 @@ struct fmBandPlan {
     unsigned int freqStep;
 };
 
+// Structure to hold output context for RTL-SDR callback
+struct OutputContext {
+    OutputBuffer *buffer;
+    bool muted;
+};
+
 class RtlFmRadioTunerControl::Private
 {
 public:
     Private(RtlFmRadioTunerControl *parent);
     ~Private();
 
+    bool muted() { return mOutputContext.muted; }
+    void setMuted(bool muted) { mOutputContext.muted = muted; }
+    void outputStart(void);
+    void outputStop(void);
+
 private:
     RtlFmRadioTunerControl *q;
 
@@ -38,7 +50,6 @@ public:
     QRadioTuner::StereoMode stereoMode;
     int signalStrength;
     int volume;
-    static bool muted;
     bool searching;
     QRadioTuner::Error error;
     QString errorString;
@@ -49,6 +60,13 @@ public:
     struct fmBandPlan fmBandPlan;
 
 private:
+    void outputInit(void);
+
+    static void rtlOutputThreadFunc(int16_t *result, int result_len, void *ctx);
+
+    QAudioOutput* mAudioOutput;
+    OutputContext mOutputContext;
+
     QMap<QString, struct fmBandPlan> knownFmBandPlans = {
         { QString::fromUtf8("US"), { .minFreq = 87900000, .maxFreq = 107900000, .freqStep = 200000 } },
         { QString::fromUtf8("JP"), { .minFreq = 76100000, .maxFreq = 89900000, .freqStep = 100000 } },
@@ -56,14 +74,8 @@ private:
         { QString::fromUtf8("ITU-1"), { .minFreq = 87500000, .maxFreq = 108000000, .freqStep = 50000 } },
         { QString::fromUtf8("ITU-2"), { .minFreq = 87900000, .maxFreq = 107900000, .freqStep = 50000 } }
     };
-
-    static void output_thread_fn(int16_t *result, int result_len, void *ctx);
-    static RadioOutputImplementation *mRadioOutput;
 };
 
-bool RtlFmRadioTunerControl::Private::muted;
-RadioOutputImplementation *RtlFmRadioTunerControl::Private::mRadioOutput;
-
 RtlFmRadioTunerControl::Private::Private(RtlFmRadioTunerControl *parent)
   : q(parent),
     state(QRadioTuner::StoppedState),
@@ -76,28 +88,11 @@ RtlFmRadioTunerControl::Private::Private(RtlFmRadioTunerControl *parent)
     searchOne(true),
     step(0)
 {
-    mRadioOutput = NULL;
-    muted = false;
-
-    // Initialize output
-    // Note that the optional ALSA support is currently not entirely
-    // functional and needs further debugging.
-    char *impl_env = getenv("RADIO_OUTPUT");
-    if (impl_env) {
-        if (strcasecmp(impl_env, "Pulse") == 0)
-	    mRadioOutput = new RadioOutputPulse();
-	if (strcasecmp(impl_env, "Alsa") == 0)
-	    mRadioOutput = new RadioOutputAlsa();
-    }
-    if (!mRadioOutput) {
-        mRadioOutput = new RadioOutputPulse();
-	if (!mRadioOutput->works)
-	    mRadioOutput = new RadioOutputAlsa();
-    }
+    outputInit();
 
     // Initialize RTL-SDR dongle
     present = false;
-    if(rtl_fm_init(frequency, 200000, 48000, output_thread_fn, NULL) < 0) {
+    if(rtl_fm_init(frequency, 200000, 48000, rtlOutputThreadFunc, &mOutputContext) < 0) {
         qDebug("%s: no RTL USB adapter?", Q_FUNC_INFO);
     } else {
         present = true;
@@ -132,15 +127,59 @@ RtlFmRadioTunerControl::Private::Private(RtlFmRadioTunerControl *parent)
     });
 }
 
-void RtlFmRadioTunerControl::Private::output_thread_fn(int16_t *result, int result_len, void *ctx)
+void RtlFmRadioTunerControl::Private::outputInit(void)
 {
-    if (!muted)
-        mRadioOutput->play((void*) result, result_len);
+    // Set up the format
+    QAudioFormat format;
+    format.setSampleRate(24000);
+    format.setChannelCount(2);
+    format.setSampleSize(16);
+    format.setCodec("audio/pcm");
+    format.setByteOrder(QAudioFormat::LittleEndian);
+    format.setSampleType(QAudioFormat::SignedInt);
+
+    QAudioDeviceInfo info(QAudioDeviceInfo::defaultOutputDevice());
+    if (!info.isFormatSupported(format)) {
+        qWarning() << "Raw audio format not supported by backend, cannot play audio.";
+	return;
+    }
+
+    // Create output
+    mAudioOutput = new QAudioOutput(format);
+    mAudioOutput->setCategory(QString("radio"));
+
+    // Initialize context to connect RTL-SDR library to QAudioOutput,
+    // including output buffer and muted state
+    mOutputContext.buffer = new OutputBuffer(format, q);
+    mOutputContext.muted = false;
+}
+
+void RtlFmRadioTunerControl::Private::outputStart(void)
+{
+    mOutputContext.buffer->start();
+    mAudioOutput->start(mOutputContext.buffer);
+}
+
+void RtlFmRadioTunerControl::Private::outputStop(void)
+{
+    mAudioOutput->stop();
+    mOutputContext.buffer->stop();
+}
+
+void RtlFmRadioTunerControl::Private::rtlOutputThreadFunc(int16_t *result, int result_len, void *ctx)
+{
+    const char *p = (const char *) result;
+    OutputContext *context = reinterpret_cast<OutputContext*>(ctx);
+
+    if (!context->muted) {
+	context->buffer->writeData(p, result_len * 2);
+    }
 }
 
 RtlFmRadioTunerControl::Private::~Private()
 {
-    delete mRadioOutput;
+    delete mAudioOutput;
+    delete mOutputContext.buffer;
     rtl_fm_cleanup();
 }
 
@@ -174,6 +213,8 @@ void RtlFmRadioTunerControl::setBand(QRadioTuner::Band band)
         return;
     d->band = band;
     emit bandChanged(band);
+#else
+    Q_UNUSED(band);
 #endif
 }
 
@@ -297,14 +338,14 @@ void RtlFmRadioTunerControl::setVolume(int volume)
 
 bool RtlFmRadioTunerControl::isMuted() const
 {
-    return d->muted;
+    return d->muted();
 }
 
 void RtlFmRadioTunerControl::setMuted(bool muted)
 {
-    if (d->muted == muted)
+    if (d->muted() == muted)
         return;
-    d->muted = muted;
+    d->setMuted(muted);
     emit mutedChanged(muted);
 }
 
@@ -366,6 +407,7 @@ void RtlFmRadioTunerControl::start()
 
     //rtl_fm_stop();
     rtl_fm_start();
+    d->outputStart();
     d->state = QRadioTuner::ActiveState;
     emit stateChanged(d->state);
     qDebug() << "RtlFmRadioTunerControl::start - exit\n";
@@ -383,6 +425,7 @@ void RtlFmRadioTunerControl::stop()
     }
 
     rtl_fm_stop();
+    d->outputStop();
     d->state = QRadioTuner::StoppedState;
     emit stateChanged(d->state);
     qDebug() << "RtlFmRadioTunerControl::stop - exit\n";