Avoid undefined format conversion and measure clipping

5 files changed, 181 insertions, 110 deletions

diff --git a/src/DabMod.cpp b/src/DabMod.cpp
index e5436ad..805fab5 100644
--- a/src/DabMod.cpp
+++ b/src/DabMod.cpp
@@ -364,19 +364,25 @@ int launch_modulator(int argc, char* argv[])
         etiLog.level(debug) << "FFTW planning done.";
     }
 
-    shared_ptr<FormatConverter> format_converter;
+    std::string output_format;
     if (mod_settings.useFileOutput and
             (mod_settings.fileOutputFormat == "s8" or
              mod_settings.fileOutputFormat == "u8" or
              mod_settings.fileOutputFormat == "s16")) {
-        format_converter = make_shared<FormatConverter>(mod_settings.fileOutputFormat);
+        output_format = mod_settings.fileOutputFormat;
     }
     else if (mod_settings.useBladeRFOutput or mod_settings.useDexterOutput) {
-        format_converter = make_shared<FormatConverter>("s16");
+        output_format = "s16";
     }
 
     auto output = prepare_output(mod_settings);
 
+    if (not output_format.empty()) {
+        if (auto o = dynamic_pointer_cast<Output::SDR>(output)) {
+            o->set_sample_size(FormatConverter::get_format_size(output_format));
+        }
+    }
+
     // Set thread priority to realtime
     if (int r = set_realtime_prio(1)) {
         etiLog.level(error) << "Could not set priority for modulator:" << r;
@@ -426,25 +432,15 @@ int launch_modulator(int argc, char* argv[])
         shared_ptr<DabModulator> modulator;
         if (inputReader) {
             m.etiReader = make_shared<EtiReader>(mod_settings.tist_offset_s);
-            modulator = make_shared<DabModulator>(*m.etiReader, mod_settings);
+            modulator = make_shared<DabModulator>(*m.etiReader, mod_settings, output_format);
         }
         else if (ediInput) {
-            modulator = make_shared<DabModulator>(ediInput->ediReader, mod_settings);
+            modulator = make_shared<DabModulator>(ediInput->ediReader, mod_settings, output_format);
         }
 
         rcs.enrol(modulator.get());
 
-        if (format_converter) {
-            flowgraph.connect(modulator, format_converter);
-            flowgraph.connect(format_converter, output);
-
-            if (auto o = dynamic_pointer_cast<Output::SDR>(output)) {
-                o->set_sample_size(format_converter->get_format_size());
-            }
-        }
-        else {
-            flowgraph.connect(modulator, output);
-        }
+        flowgraph.connect(modulator, output);
 
         if (inputReader) {
             etiLog.level(info) << inputReader->GetPrintableInfo();
diff --git a/src/DabModulator.cpp b/src/DabModulator.cpp
index 64ebf03..5213d8d 100644
--- a/src/DabModulator.cpp
+++ b/src/DabModulator.cpp
@@ -59,19 +59,22 @@
 using namespace std;
 
 DabModulator::DabModulator(EtiSource& etiSource,
-                           mod_settings_t& settings) :
+                           mod_settings_t& settings,
+                           const std::string& format) :
     ModInput(),
     RemoteControllable("modulator"),
     m_settings(settings),
-    myEtiSource(etiSource),
-    myFlowgraph()
+    m_format(format),
+    m_etiSource(etiSource),
+    m_flowgraph()
 {
     PDEBUG("DabModulator::DabModulator() @ %p\n", this);
 
     RC_ADD_PARAMETER(rate, "(Read-only) IQ output samplerate");
+    RC_ADD_PARAMETER(num_clipped_samples, "(Read-only) Number of samples clipped in last frame during format conversion");
 
     if (m_settings.dabMode == 0) {
-        setMode(2);
+        setMode(1);
     }
     else {
         setMode(m_settings.dabMode);
@@ -83,36 +86,36 @@ void DabModulator::setMode(unsigned mode)
 {
     switch (mode) {
     case 1:
-        myNbSymbols = 76;
-        myNbCarriers = 1536;
-        mySpacing = 2048;
-        myNullSize = 2656;
-        mySymSize = 2552;
-        myFicSizeOut = 288;
+        m_nbSymbols = 76;
+        m_nbCarriers = 1536;
+        m_spacing = 2048;
+        m_nullSize = 2656;
+        m_symSize = 2552;
+        m_ficSizeOut = 288;
         break;
     case 2:
-        myNbSymbols = 76;
-        myNbCarriers = 384;
-        mySpacing = 512;
-        myNullSize = 664;
-        mySymSize = 638;
-        myFicSizeOut = 288;
+        m_nbSymbols = 76;
+        m_nbCarriers = 384;
+        m_spacing = 512;
+        m_nullSize = 664;
+        m_symSize = 638;
+        m_ficSizeOut = 288;
         break;
     case 3:
-        myNbSymbols = 153;
-        myNbCarriers = 192;
-        mySpacing = 256;
-        myNullSize = 345;
-        mySymSize = 319;
-        myFicSizeOut = 384;
+        m_nbSymbols = 153;
+        m_nbCarriers = 192;
+        m_spacing = 256;
+        m_nullSize = 345;
+        m_symSize = 319;
+        m_ficSizeOut = 384;
         break;
     case 4:
-        myNbSymbols = 76;
-        myNbCarriers = 768;
-        mySpacing = 1024;
-        myNullSize = 1328;
-        mySymSize = 1276;
-        myFicSizeOut = 288;
+        m_nbSymbols = 76;
+        m_nbCarriers = 768;
+        m_spacing = 1024;
+        m_nullSize = 1328;
+        m_symSize = 1276;
+        m_ficSizeOut = 288;
         break;
     default:
         throw std::runtime_error("DabModulator::setMode invalid mode size");
@@ -126,27 +129,27 @@ int DabModulator::process(Buffer* dataOut)
 
     PDEBUG("DabModulator::process(dataOut: %p)\n", dataOut);
 
-    if (not myFlowgraph) {
+    if (not m_flowgraph) {
         etiLog.level(debug) << "Setting up DabModulator...";
         const unsigned mode = m_settings.dabMode;
         setMode(mode);
 
-        myFlowgraph = make_shared<Flowgraph>(m_settings.showProcessTime);
+        m_flowgraph = make_shared<Flowgraph>(m_settings.showProcessTime);
         ////////////////////////////////////////////////////////////////
         // CIF data initialisation
         ////////////////////////////////////////////////////////////////
         auto cifPrbs = make_shared<PrbsGenerator>(864 * 8, 0x110);
-        auto cifMux = make_shared<FrameMultiplexer>(myEtiSource);
+        auto cifMux = make_shared<FrameMultiplexer>(m_etiSource);
         auto cifPart = make_shared<BlockPartitioner>(mode);
 
-        auto cifMap = make_shared<QpskSymbolMapper>(myNbCarriers);
+        auto cifMap = make_shared<QpskSymbolMapper>(m_nbCarriers);
         auto cifRef = make_shared<PhaseReference>(mode);
         auto cifFreq = make_shared<FrequencyInterleaver>(mode);
-        auto cifDiff = make_shared<DifferentialModulator>(myNbCarriers);
+        auto cifDiff = make_shared<DifferentialModulator>(m_nbCarriers);
 
-        auto cifNull = make_shared<NullSymbol>(myNbCarriers);
+        auto cifNull = make_shared<NullSymbol>(m_nbCarriers);
         auto cifSig = make_shared<SignalMultiplexer>(
-                (1 + myNbSymbols) * myNbCarriers * sizeof(complexf));
+                (1 + m_nbSymbols) * m_nbCarriers * sizeof(complexf));
 
         // TODO this needs a review
         bool useCicEq = false;
@@ -167,8 +170,8 @@ int DabModulator::process(Buffer* dataOut)
         shared_ptr<CicEqualizer> cifCicEq;
         if (useCicEq) {
             cifCicEq = make_shared<CicEqualizer>(
-                myNbCarriers,
-                (float)mySpacing * (float)m_settings.outputRate / 2048000.0f,
+                m_nbCarriers,
+                (float)m_spacing * (float)m_settings.outputRate / 2048000.0f,
                 cic_ratio);
         }
 
@@ -186,9 +189,9 @@ int DabModulator::process(Buffer* dataOut)
         }
 
         auto cifOfdm = make_shared<OfdmGenerator>(
-                (1 + myNbSymbols),
-                myNbCarriers,
-                mySpacing,
+                (1 + m_nbSymbols),
+                m_nbCarriers,
+                m_spacing,
                 m_settings.enableCfr,
                 m_settings.cfrClip,
                 m_settings.cfrErrorClip);
@@ -196,7 +199,7 @@ int DabModulator::process(Buffer* dataOut)
         rcs.enrol(cifOfdm.get());
 
         auto cifGain = make_shared<GainControl>(
-                mySpacing,
+                m_spacing,
                 m_settings.gainMode,
                 m_settings.digitalgain,
                 m_settings.normalise,
@@ -205,7 +208,7 @@ int DabModulator::process(Buffer* dataOut)
         rcs.enrol(cifGain.get());
 
         auto cifGuard = make_shared<GuardIntervalInserter>(
-                myNbSymbols, mySpacing, myNullSize, mySymSize,
+                m_nbSymbols, m_spacing, m_nullSize, m_symSize,
                 m_settings.ofdmWindowOverlap);
         rcs.enrol(cifGuard.get());
 
@@ -227,17 +230,21 @@ int DabModulator::process(Buffer* dataOut)
             cifRes = make_shared<Resampler>(
                     2048000,
                     m_settings.outputRate,
-                    mySpacing);
+                    m_spacing);
         }
 
-        myOutput = make_shared<OutputMemory>(dataOut);
+        if (not m_format.empty()) {
+            m_formatConverter = make_shared<FormatConverter>(m_format);
+        }
+
+        m_output = make_shared<OutputMemory>(dataOut);
 
-        myFlowgraph->connect(cifPrbs, cifMux);
+        m_flowgraph->connect(cifPrbs, cifMux);
 
         ////////////////////////////////////////////////////////////////
         // Processing FIC
         ////////////////////////////////////////////////////////////////
-        shared_ptr<FicSource> fic(myEtiSource.getFic());
+        shared_ptr<FicSource> fic(m_etiSource.getFic());
         ////////////////////////////////////////////////////////////////
         // Data initialisation
         ////////////////////////////////////////////////////////////////
@@ -269,15 +276,15 @@ int DabModulator::process(Buffer* dataOut)
         PDEBUG(" Adding tail\n");
         ficPunc->append_tail_rule(PuncturingRule(3, 0xcccccc));
 
-        myFlowgraph->connect(fic, ficPrbs);
-        myFlowgraph->connect(ficPrbs, ficConv);
-        myFlowgraph->connect(ficConv, ficPunc);
-        myFlowgraph->connect(ficPunc, cifPart);
+        m_flowgraph->connect(fic, ficPrbs);
+        m_flowgraph->connect(ficPrbs, ficConv);
+        m_flowgraph->connect(ficConv, ficPunc);
+        m_flowgraph->connect(ficPunc, cifPart);
 
         ////////////////////////////////////////////////////////////////
         // Configuring subchannels
         ////////////////////////////////////////////////////////////////
-        for (const auto& subchannel : myEtiSource.getSubchannels()) {
+        for (const auto& subchannel : m_etiSource.getSubchannels()) {
 
             ////////////////////////////////////////////////////////////
             // Data initialisation
@@ -329,23 +336,23 @@ int DabModulator::process(Buffer* dataOut)
             // Configuring time interleaver
             auto subchInterleaver = make_shared<TimeInterleaver>(subchSizeOut);
 
-            myFlowgraph->connect(subchannel, subchPrbs);
-            myFlowgraph->connect(subchPrbs, subchConv);
-            myFlowgraph->connect(subchConv, subchPunc);
-            myFlowgraph->connect(subchPunc, subchInterleaver);
-            myFlowgraph->connect(subchInterleaver, cifMux);
+            m_flowgraph->connect(subchannel, subchPrbs);
+            m_flowgraph->connect(subchPrbs, subchConv);
+            m_flowgraph->connect(subchConv, subchPunc);
+            m_flowgraph->connect(subchPunc, subchInterleaver);
+            m_flowgraph->connect(subchInterleaver, cifMux);
         }
 
-        myFlowgraph->connect(cifMux, cifPart);
-        myFlowgraph->connect(cifPart, cifMap);
-        myFlowgraph->connect(cifMap, cifFreq);
-        myFlowgraph->connect(cifRef, cifDiff);
-        myFlowgraph->connect(cifFreq, cifDiff);
-        myFlowgraph->connect(cifNull, cifSig);
-        myFlowgraph->connect(cifDiff, cifSig);
+        m_flowgraph->connect(cifMux, cifPart);
+        m_flowgraph->connect(cifPart, cifMap);
+        m_flowgraph->connect(cifMap, cifFreq);
+        m_flowgraph->connect(cifRef, cifDiff);
+        m_flowgraph->connect(cifFreq, cifDiff);
+        m_flowgraph->connect(cifNull, cifSig);
+        m_flowgraph->connect(cifDiff, cifSig);
         if (tii) {
-            myFlowgraph->connect(tiiRef, tii);
-            myFlowgraph->connect(tii, cifSig);
+            m_flowgraph->connect(tiiRef, tii);
+            m_flowgraph->connect(tii, cifSig);
         }
 
         shared_ptr<ModPlugin> prev_plugin = static_pointer_cast<ModPlugin>(cifSig);
@@ -354,15 +361,18 @@ int DabModulator::process(Buffer* dataOut)
                 static_pointer_cast<ModPlugin>(cifOfdm),
                 static_pointer_cast<ModPlugin>(cifGain),
                 static_pointer_cast<ModPlugin>(cifGuard),
-                static_pointer_cast<ModPlugin>(cifFilter), // optional block
-                static_pointer_cast<ModPlugin>(cifRes),    // optional block
-                static_pointer_cast<ModPlugin>(cifPoly),   // optional block
-                static_pointer_cast<ModPlugin>(myOutput),
+                // optional blocks
+                static_pointer_cast<ModPlugin>(cifFilter),
+                static_pointer_cast<ModPlugin>(cifRes),
+                static_pointer_cast<ModPlugin>(cifPoly),
+                static_pointer_cast<ModPlugin>(m_formatConverter),
+                // mandatory block
+                static_pointer_cast<ModPlugin>(m_output),
                 });
 
         for (auto& p : plugins) {
             if (p) {
-                myFlowgraph->connect(prev_plugin, p);
+                m_flowgraph->connect(prev_plugin, p);
                 prev_plugin = p;
             }
         }
@@ -372,13 +382,13 @@ int DabModulator::process(Buffer* dataOut)
     ////////////////////////////////////////////////////////////////////
     // Processing data
     ////////////////////////////////////////////////////////////////////
-    return myFlowgraph->run();
+    return m_flowgraph->run();
 }
 
 meta_vec_t DabModulator::process_metadata(const meta_vec_t& metadataIn)
 {
-    if (myOutput) {
-        return myOutput->get_latest_metadata();
+    if (m_output) {
+        return m_output->get_latest_metadata();
     }
 
     return {};
@@ -390,6 +400,9 @@ void DabModulator::set_parameter(const string& parameter, const string& value)
     if (parameter == "rate") {
         throw ParameterError("Parameter 'rate' is read-only");
     }
+    else if (parameter == "num_clipped_samples") {
+        throw ParameterError("Parameter 'num_clipped_samples' is read-only");
+    }
     else {
         stringstream ss;
         ss << "Parameter '" << parameter <<
@@ -404,6 +417,16 @@ const string DabModulator::get_parameter(const string& parameter) const
     if (parameter == "rate") {
         ss << m_settings.outputRate;
     }
+    else if (parameter == "num_clipped_samples") {
+        if (m_formatConverter) {
+            ss << m_formatConverter->get_num_clipped_samples();
+        }
+        else {
+            ss << "Parameter '" << parameter <<
+                "' is not available when no format conversion is done.";
+            throw ParameterError(ss.str());
+        }
+    }
     else {
         ss << "Parameter '" << parameter <<
             "' is not exported by controllable " << get_rc_name();
@@ -416,5 +439,6 @@ const RemoteControllable::map_t DabModulator::get_all_values() const
 {
     map_t map;
     map["rate"] = m_settings.outputRate;
+    map["num_clipped_samples"] = m_formatConverter ? m_formatConverter->get_num_clipped_samples() : 0;
     return map;
 }
diff --git a/src/DabModulator.h b/src/DabModulator.h
index 0d46e9f..6381252 100644
--- a/src/DabModulator.h
+++ b/src/DabModulator.h
@@ -39,6 +39,7 @@
 #include "ConfigParser.h"
 #include "EtiReader.h"
 #include "Flowgraph.h"
+#include "FormatConverter.h"
 #include "GainControl.h"
 #include "OutputMemory.h"
 #include "RemoteControl.h"
@@ -49,7 +50,8 @@
 class DabModulator : public ModInput, public ModMetadata, public RemoteControllable
 {
 public:
-    DabModulator(EtiSource& etiSource, mod_settings_t& settings);
+    DabModulator(EtiSource& etiSource, mod_settings_t& settings, const std::string& format);
+    // Allowed formats: s8, u8 and s16. Empty string means no conversion
 
     int process(Buffer* dataOut) override;
     const char* name() override { return "DabModulator"; }
@@ -57,7 +59,7 @@ public:
     virtual meta_vec_t process_metadata(const meta_vec_t& metadataIn) override;
 
     /* Required to get the timestamp */
-    EtiSource* getEtiSource() { return &myEtiSource; }
+    EtiSource* getEtiSource() { return &m_etiSource; }
 
     /******* REMOTE CONTROL ********/
     virtual void set_parameter(const std::string& parameter, const std::string& value) override;
@@ -68,17 +70,19 @@ protected:
     void setMode(unsigned mode);
 
     mod_settings_t& m_settings;
+    std::string m_format;
 
-    EtiSource& myEtiSource;
-    std::shared_ptr<Flowgraph> myFlowgraph;
+    EtiSource& m_etiSource;
+    std::shared_ptr<Flowgraph> m_flowgraph;
 
-    size_t myNbSymbols;
-    size_t myNbCarriers;
-    size_t mySpacing;
-    size_t myNullSize;
-    size_t mySymSize;
-    size_t myFicSizeOut;
+    size_t m_nbSymbols;
+    size_t m_nbCarriers;
+    size_t m_spacing;
+    size_t m_nullSize;
+    size_t m_symSize;
+    size_t m_ficSizeOut;
 
-    std::shared_ptr<OutputMemory> myOutput;
+    std::shared_ptr<FormatConverter> m_formatConverter;
+    std::shared_ptr<OutputMemory> m_output;
 };
 
diff --git a/src/FormatConverter.cpp b/src/FormatConverter.cpp
index cda8a4d..fc4cc2f 100644
--- a/src/FormatConverter.cpp
+++ b/src/FormatConverter.cpp
@@ -2,7 +2,7 @@
    Copyright (C) 2005, 2006, 2007, 2008, 2009, 2010, 2011 Her Majesty
    the Queen in Right of Canada (Communications Research Center Canada)
 
-   Copyright (C) 2022
+   Copyright (C) 2023
    Matthias P. Braendli, matthias.braendli@mpb.li
 
     http://opendigitalradio.org
@@ -45,6 +45,8 @@ int FormatConverter::process(Buffer* const dataIn, Buffer* dataOut)
     PDEBUG("FormatConverter::process(dataIn: %p, dataOut: %p)\n",
             dataIn, dataOut);
 
+    size_t num_clipped_samples = 0;
+
     size_t sizeIn = dataIn->getLength() / sizeof(float);
     float* in = reinterpret_cast<float*>(dataIn->getData());
 
@@ -53,7 +55,17 @@ int FormatConverter::process(Buffer* const dataIn, Buffer* dataOut)
         int16_t* out = reinterpret_cast<int16_t*>(dataOut->getData());
 
         for (size_t i = 0; i < sizeIn; i++) {
-            out[i] = in[i];
+            if (in[i] < INT16_MIN) {
+                out[i] = INT16_MIN;
+                num_clipped_samples++;
+            }
+            else if (in[i] > INT16_MAX) {
+                out[i] = INT16_MAX;
+                num_clipped_samples++;
+            }
+            else {
+                out[i] = in[i];
+            }
         }
     }
     else if (m_format == "u8") {
@@ -61,7 +73,19 @@ int FormatConverter::process(Buffer* const dataIn, Buffer* dataOut)
         uint8_t* out = reinterpret_cast<uint8_t*>(dataOut->getData());
 
         for (size_t i = 0; i < sizeIn; i++) {
-            out[i] = in[i] + 128;
+            const auto samp = in[i] + 128.0f;
+            if (samp < 0) {
+                out[i] = 0;
+                num_clipped_samples++;
+            }
+            else if (samp > INT8_MAX) {
+                out[i] = INT8_MAX;
+                num_clipped_samples++;
+            }
+            else {
+                out[i] = samp;
+            }
+
         }
     }
     else if (m_format == "s8") {
@@ -69,13 +93,25 @@ int FormatConverter::process(Buffer* const dataIn, Buffer* dataOut)
         int8_t* out = reinterpret_cast<int8_t*>(dataOut->getData());
 
         for (size_t i = 0; i < sizeIn; i++) {
-            out[i] = in[i];
+            if (in[i] < INT8_MIN) {
+                out[i] = INT8_MIN;
+                num_clipped_samples++;
+            }
+            else if (in[i] > INT8_MAX) {
+                out[i] = INT8_MAX;
+                num_clipped_samples++;
+            }
+            else {
+                out[i] = in[i];
+            }
         }
     }
     else {
         throw std::runtime_error("FormatConverter: Invalid format " + m_format);
     }
 
+    m_num_clipped_samples.store(num_clipped_samples);
+
     return dataOut->getLength();
 }
 
@@ -84,19 +120,25 @@ const char* FormatConverter::name()
     return "FormatConverter";
 }
 
-size_t FormatConverter::get_format_size() const
+size_t FormatConverter::get_num_clipped_samples() const
+{
+    return m_num_clipped_samples.load();
+}
+
+
+size_t FormatConverter::get_format_size(const std::string& format)
 {
     // Returns 2*sizeof(SAMPLE_TYPE) because we have I + Q
-    if (m_format == "s16") {
+    if (format == "s16") {
         return 4;
     }
-    else if (m_format == "u8") {
+    else if (format == "u8") {
         return 2;
     }
-    else if (m_format == "s8") {
+    else if (format == "s8") {
         return 2;
     }
     else {
-        throw std::runtime_error("FormatConverter: Invalid format " + m_format);
+        throw std::runtime_error("FormatConverter: Invalid format " + format);
     }
 }
diff --git a/src/FormatConverter.h b/src/FormatConverter.h
index ceb2e17..05511c0 100644
--- a/src/FormatConverter.h
+++ b/src/FormatConverter.h
@@ -34,22 +34,27 @@
 
 #include "ModPlugin.h"
 #include <complex>
+#include <atomic>
 #include <string>
 #include <cstdint>
 
 class FormatConverter : public ModCodec
 {
     public:
+        static size_t get_format_size(const std::string& format);
+
         // Allowed formats: s8, u8 and s16
         FormatConverter(const std::string& format);
 
         int process(Buffer* const dataIn, Buffer* dataOut);
         const char* name();
 
-        size_t get_format_size() const;
+        size_t get_num_clipped_samples() const;
 
     private:
         std::string m_format;
+
+        std::atomic<size_t> m_num_clipped_samples = 0;
 };