From 66a0af90530551583f29fdb7efad4635885a13e4 Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Thu, 8 Nov 2018 14:31:40 -0500
Subject: [PATCH] resampler: improve reliability

Improves reliability of resampler by setting the input and output
channel counts and by forcing initialization on the first resampling.

Removes redundancy by using the sample format in AudioFormat.

Change-Id: Ie0154d5b46db6735dbc8da81fdb0e6be795a3efa
---
 src/debug_utils.h                            |  1 +
 src/media/audio/audiobuffer.h                |  6 ++++++
 src/media/audio/resampler.cpp                | 22 +++++++++++++-------
 src/media/audio/resampler.h                  |  4 ++--
 src/media/audio/sound/audiofile.cpp          | 10 +++++----
 test/unitTest/media/audio/test_resampler.cpp |  1 +
 6 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/src/debug_utils.h b/src/debug_utils.h
index 5bb3bf0ff0..3807bae316 100644
--- a/src/debug_utils.h
+++ b/src/debug_utils.h
@@ -160,6 +160,7 @@ private:
                 output->format = AV_SAMPLE_FMT_S64;
             output->sample_rate = input->sample_rate;
             output->channel_layout = input->channel_layout;
+            output->channels = input->channels;
             resampler_->resample(input, output);
             av_frame_unref(input);
             av_frame_move_ref(frame, output);
diff --git a/src/media/audio/audiobuffer.h b/src/media/audio/audiobuffer.h
index 47bf07843b..a79f4cfe4b 100644
--- a/src/media/audio/audiobuffer.h
+++ b/src/media/audio/audiobuffer.h
@@ -57,6 +57,12 @@ struct AudioFormat {
         , sampleFormat(AV_SAMPLE_FMT_S16)
     {}
 
+    constexpr AudioFormat(unsigned sr, unsigned c, AVSampleFormat f)
+        : sample_rate(sr)
+        , nb_channels(c)
+        , sampleFormat(f)
+    {}
+
     inline bool operator == (const AudioFormat &b) const {
         return ( (b.sample_rate == sample_rate) &&
                  (b.nb_channels == nb_channels) &&
diff --git a/src/media/audio/resampler.cpp b/src/media/audio/resampler.cpp
index 3424d5e708..d61741a678 100644
--- a/src/media/audio/resampler.cpp
+++ b/src/media/audio/resampler.cpp
@@ -35,6 +35,7 @@ namespace ring {
 
 Resampler::Resampler()
     : swrCtx_(swr_alloc())
+    , initialized_(false)
 {}
 
 Resampler::~Resampler()
@@ -43,29 +44,33 @@ Resampler::~Resampler()
 }
 
 void
-Resampler::reinit(const AudioFormat& in, const int inSampleFmt,
-                  const AudioFormat& out, const int outSampleFmt)
+Resampler::reinit(const AudioFormat& in, const AudioFormat& out)
 {
-    av_opt_set_int(swrCtx_, "ich", 0, 0);
+    av_opt_set_int(swrCtx_, "ich", in.nb_channels, 0);
     av_opt_set_int(swrCtx_, "icl", av_get_default_channel_layout(in.nb_channels), 0);
     av_opt_set_int(swrCtx_, "isr", in.sample_rate, 0);
-    av_opt_set_sample_fmt(swrCtx_, "isf", static_cast<AVSampleFormat>(inSampleFmt), 0);
+    av_opt_set_sample_fmt(swrCtx_, "isf", in.sampleFormat, 0);
 
-    av_opt_set_int(swrCtx_, "och", 0, 0);
+    av_opt_set_int(swrCtx_, "och", out.nb_channels, 0);
     av_opt_set_int(swrCtx_, "ocl", av_get_default_channel_layout(out.nb_channels), 0);
     av_opt_set_int(swrCtx_, "osr", out.sample_rate, 0);
-    av_opt_set_sample_fmt(swrCtx_, "osf", static_cast<AVSampleFormat>(outSampleFmt), 0);
+    av_opt_set_sample_fmt(swrCtx_, "osf", out.sampleFormat, 0);
 
     swr_init(swrCtx_);
+    initialized_ = true;
 }
 
 int
 Resampler::resample(const AVFrame* input, AVFrame* output)
 {
+    const auto inFmt = AudioFormat((unsigned)input->sample_rate, (unsigned)input->channels, (AVSampleFormat)input->format);
+    const auto outFmt = AudioFormat((unsigned)output->sample_rate, (unsigned)output->channels, (AVSampleFormat)output->format);
+    if (!initialized_)
+        reinit(inFmt, outFmt);
+
     int ret = swr_convert_frame(swrCtx_, output, input);
     if (ret & AVERROR_INPUT_CHANGED || ret & AVERROR_OUTPUT_CHANGED) {
-        reinit(AudioFormat{(unsigned)input->sample_rate, (unsigned)input->channels}, input->format,
-               AudioFormat{(unsigned)output->sample_rate, (unsigned)output->channels}, output->format);
+        reinit(inFmt, outFmt);
         return resample(input, output);
     } else if (ret < 0) {
         RING_ERR() << "Failed to resample frame";
@@ -84,6 +89,7 @@ Resampler::resample(const AudioBuffer& dataIn, AudioBuffer& dataOut)
     auto output = resampled.pointer();
     output->sample_rate = dataOut.getSampleRate();
     output->channel_layout = av_get_default_channel_layout(dataOut.channels());
+    output->channels = dataOut.channels();
     output->format = AV_SAMPLE_FMT_S16;
 
     if (resample(input, output) < 0)
diff --git a/src/media/audio/resampler.h b/src/media/audio/resampler.h
index 57dc99bc65..9ab237899e 100644
--- a/src/media/audio/resampler.h
+++ b/src/media/audio/resampler.h
@@ -59,10 +59,10 @@ class Resampler {
          * output buffers always have the same formats, will never be called, as the first
          * initialization is done in swr_convert_frame.
          */
-        void reinit(const AudioFormat& in, const int inSampleFmt,
-                    const AudioFormat& out, const int outSampleFmt);
+        void reinit(const AudioFormat& in, const AudioFormat& out);
 
         SwrContext* swrCtx_; // incomplete type, cannot be a unique_ptr
+        bool initialized_;
 };
 
 } // namespace ring
diff --git a/src/media/audio/sound/audiofile.cpp b/src/media/audio/sound/audiofile.cpp
index 214d665b58..e4a5d6cc32 100644
--- a/src/media/audio/sound/audiofile.cpp
+++ b/src/media/audio/sound/audiofile.cpp
@@ -70,7 +70,8 @@ AudioFile::AudioFile(const std::string &fileName, unsigned int sampleRate) :
         throw AudioFileException("Decoder setup failed: " + fileName);
 
     auto resampler = std::make_unique<Resampler>();
-    auto buf = std::make_unique<AudioBuffer>(0, getFormat());
+    const auto& format = getFormat();
+    auto buf = std::make_unique<AudioBuffer>(0, format);
     bool done = false;
     while (!done) {
         AudioFrame input;
@@ -78,9 +79,10 @@ AudioFile::AudioFile(const std::string &fileName, unsigned int sampleRate) :
         auto resampled = output.pointer();
         switch (decoder->decode(input)) {
         case MediaDecoder::Status::FrameFinished:
-            resampled->sample_rate = getFormat().sample_rate;
-            resampled->channel_layout = av_get_default_channel_layout(getFormat().nb_channels);
-            resampled->format = AV_SAMPLE_FMT_S16;
+            resampled->sample_rate = format.sample_rate;
+            resampled->channel_layout = av_get_default_channel_layout(format.nb_channels);
+            resampled->channels = format.nb_channels;
+            resampled->format = format.sampleFormat;
             if (resampler->resample(input.pointer(), resampled) < 0)
                 throw AudioFileException("Frame could not be resampled");
             if (buf->append(output) < 0)
diff --git a/test/unitTest/media/audio/test_resampler.cpp b/test/unitTest/media/audio/test_resampler.cpp
index 90d6ecc5de..ffa8873d66 100644
--- a/test/unitTest/media/audio/test_resampler.cpp
+++ b/test/unitTest/media/audio/test_resampler.cpp
@@ -97,6 +97,7 @@ ResamplerTest::testAudioFrame()
     output->format = AV_SAMPLE_FMT_FLT;
     output->sample_rate = 48000;
     output->channel_layout = AV_CH_LAYOUT_STEREO;
+    output->channels = 2;
 
     int ret = resampler_->resample(input->pointer(), output);
     CPPUNIT_ASSERT_MESSAGE(libav_utils::getError(ret).c_str(), ret >= 0);
-- 
GitLab