From 8d0d80b3f74f7121672361e281deba9338ee4f8a Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Mon, 31 Dec 2018 12:15:51 -0500
Subject: [PATCH] resampler: set matrix on uninitialized context

A comment in swr_set_matrix says "[SwrContext] needs to be allocated
but not initialized" before returning an error code.

Adds a unit test for reinitializing a matrix.

Change-Id: If8f38a5dd15393f8ae9578b4f272c2554a663c11
---
 src/media/audio/resampler.cpp                | 38 +++++++++++++-------
 test/unitTest/media/audio/test_resampler.cpp | 27 ++++++++++++++
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/src/media/audio/resampler.cpp b/src/media/audio/resampler.cpp
index 1652147251..3700a05fe1 100644
--- a/src/media/audio/resampler.cpp
+++ b/src/media/audio/resampler.cpp
@@ -43,15 +43,22 @@ Resampler::~Resampler()
 void
 Resampler::reinit(const AVFrame* in, const AVFrame* out)
 {
-    av_opt_set_int(swrCtx_, "ich", in->channels, 0);
-    av_opt_set_int(swrCtx_, "icl", in->channel_layout, 0);
-    av_opt_set_int(swrCtx_, "isr", in->sample_rate, 0);
-    av_opt_set_sample_fmt(swrCtx_, "isf", static_cast<AVSampleFormat>(in->format), 0);
+    // NOTE swr_set_matrix should be called on an uninitialized context
+    auto swrCtx = swr_alloc();
+    if (!swrCtx) {
+        RING_ERR() << "Cannot allocate resampler context";
+        throw std::bad_alloc();
+    }
+
+    av_opt_set_int(swrCtx, "ich", in->channels, 0);
+    av_opt_set_int(swrCtx, "icl", in->channel_layout, 0);
+    av_opt_set_int(swrCtx, "isr", in->sample_rate, 0);
+    av_opt_set_sample_fmt(swrCtx, "isf", static_cast<AVSampleFormat>(in->format), 0);
 
-    av_opt_set_int(swrCtx_, "och", out->channels, 0);
-    av_opt_set_int(swrCtx_, "ocl", out->channel_layout, 0);
-    av_opt_set_int(swrCtx_, "osr", out->sample_rate, 0);
-    av_opt_set_sample_fmt(swrCtx_, "osf", static_cast<AVSampleFormat>(out->format), 0);
+    av_opt_set_int(swrCtx, "och", out->channels, 0);
+    av_opt_set_int(swrCtx, "ocl", out->channel_layout, 0);
+    av_opt_set_int(swrCtx, "osr", out->sample_rate, 0);
+    av_opt_set_sample_fmt(swrCtx, "osf", static_cast<AVSampleFormat>(out->format), 0);
 
     /**
      * Downmixing from 5.1 requires extra setup, since libswresample can't do it automatically
@@ -82,7 +89,7 @@ Resampler::reinit(const AVFrame* in, const AVFrame* out)
             matrix[1][3] = 1;
             matrix[1][4] = 0;
             matrix[1][5] = 0.707;
-            swr_set_matrix(swrCtx_, matrix[0], 6);
+            swr_set_matrix(swrCtx, matrix[0], 6);
         } else {
             double matrix[1][6];
             // M = 1.0*FL + 1.414*FC + 1.0*FR + 0.707*BL + 0.707*BR + 2.0*LFE
@@ -92,12 +99,19 @@ Resampler::reinit(const AVFrame* in, const AVFrame* out)
             matrix[0][3] = 2;
             matrix[0][4] = 0.707;
             matrix[0][5] = 0.707;
-            swr_set_matrix(swrCtx_, matrix[0], 6);
+            swr_set_matrix(swrCtx, matrix[0], 6);
         }
     }
 
-    swr_init(swrCtx_);
-    ++initCount_;
+    if (swr_init(swrCtx) >= 0) {
+        std::swap(swrCtx_, swrCtx);
+        swr_free(&swrCtx);
+        ++initCount_;
+    } else {
+        std::string msg = "Failed to initialize resampler context";
+        RING_ERR() << msg;
+        throw std::runtime_error(msg);
+    }
 }
 
 int
diff --git a/test/unitTest/media/audio/test_resampler.cpp b/test/unitTest/media/audio/test_resampler.cpp
index 511949bc9b..617c1e2141 100644
--- a/test/unitTest/media/audio/test_resampler.cpp
+++ b/test/unitTest/media/audio/test_resampler.cpp
@@ -41,10 +41,12 @@ public:
 private:
     void testAudioBuffer();
     void testAudioFrame();
+    void testRematrix();
 
     CPPUNIT_TEST_SUITE(ResamplerTest);
     CPPUNIT_TEST(testAudioBuffer);
     CPPUNIT_TEST(testAudioFrame);
+    CPPUNIT_TEST(testRematrix);
     CPPUNIT_TEST_SUITE_END();
 
     std::unique_ptr<Resampler> resampler_;
@@ -105,6 +107,31 @@ ResamplerTest::testAudioFrame()
     CPPUNIT_ASSERT(output->data[0][0] == 0);
 }
 
+void
+ResamplerTest::testRematrix()
+{
+    int ret = 0;
+    const constexpr AudioFormat inFormat = AudioFormat(44100, 6);
+    resampler_.reset(new Resampler);
+
+    auto input = std::make_unique<DRing::AudioFrame>(inFormat, 882);
+    CPPUNIT_ASSERT(input->pointer() && input->pointer()->data);
+
+    auto output1 = std::make_unique<DRing::AudioFrame>(AudioFormat::STEREO(), 960);
+    CPPUNIT_ASSERT(output1->pointer() && output1->pointer()->data);
+
+    ret = resampler_->resample(input->pointer(), output1->pointer());
+    CPPUNIT_ASSERT_MESSAGE(libav_utils::getError(ret).c_str(), ret >= 0);
+    CPPUNIT_ASSERT(output1->pointer()->data && output1->pointer()->data[0]);
+
+    auto output2 = std::make_unique<DRing::AudioFrame>(AudioFormat::MONO(), 960);
+    CPPUNIT_ASSERT(output2->pointer() && output2->pointer()->data);
+
+    ret = resampler_->resample(input->pointer(), output2->pointer());
+    CPPUNIT_ASSERT_MESSAGE(libav_utils::getError(ret).c_str(), ret >= 0);
+    CPPUNIT_ASSERT(output2->pointer()->data && output2->pointer()->data[0]);
+}
+
 }} // namespace ring::test
 
 RING_TEST_RUNNER(ring::test::ResamplerTest::name());
-- 
GitLab