From bc594b5d0eb3c16f56615af2a3678dba9e5c5c63 Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Wed, 19 Dec 2018 10:05:29 -0500
Subject: [PATCH] resampler: detect infinite loops
Under certain conditions, the resampler reinitializes itself in an
infinite loop. Prevent this by detecting this state and throwing an
exception.
This is not normal behaviour and is indicative of an underlying bug.
Change-Id: I70d58465daad12645d6bb88833e62e1232891e3b
---
src/media/audio/resampler.cpp | 22 +++++++-----
src/media/audio/resampler.h | 64 ++++++++++++++++++++---------------
2 files changed, 51 insertions(+), 35 deletions(-)
diff --git a/src/media/audio/resampler.cpp b/src/media/audio/resampler.cpp
index cc0818a0e5..6409287a66 100644
--- a/src/media/audio/resampler.cpp
+++ b/src/media/audio/resampler.cpp
@@ -3,6 +3,7 @@
*
* Author: Emmanuel Milou <emmanuel.milou@savoirfairelinux.com>
* Author: Alexandre Savard <alexandre.savard@savoirfairelinux.com>
+ * Author: Philippe Gorley <philippe.gorley@savoirfairelinux.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
@@ -21,11 +22,7 @@
#include "libav_deps.h"
#include "logger.h"
-#include "media_buffer.h"
-#include "media_filter.h"
-#include "media_stream.h"
#include "resampler.h"
-#include "ring_types.h"
extern "C" {
#include <libswresample/swresample.h>
@@ -35,7 +32,7 @@ namespace ring {
Resampler::Resampler()
: swrCtx_(swr_alloc())
- , initialized_(false)
+ , initCount_(0)
{}
Resampler::~Resampler()
@@ -57,17 +54,25 @@ Resampler::reinit(const AVFrame* in, const AVFrame* out)
av_opt_set_sample_fmt(swrCtx_, "osf", static_cast<AVSampleFormat>(out->format), 0);
swr_init(swrCtx_);
- initialized_ = true;
+ ++initCount_;
}
int
Resampler::resample(const AVFrame* input, AVFrame* output)
{
- if (!initialized_)
+ if (!initCount_)
reinit(input, output);
int ret = swr_convert_frame(swrCtx_, output, input);
if (ret & AVERROR_INPUT_CHANGED || ret & AVERROR_OUTPUT_CHANGED) {
+ // Under certain conditions, the resampler reinits itself in an infinite loop. This is
+ // indicative of an underlying problem in the code. This check is so the backtrace
+ // doesn't get mangled with a bunch of calls to Resampler::resample
+ if (initCount_ > 1) {
+ std::string msg = "Infinite loop detected in audio resampler, please open an issue on https://git.jami.net";
+ RING_ERR() << msg;
+ throw std::runtime_error(msg);
+ }
reinit(input, output);
return resample(input, output);
} else if (ret < 0) {
@@ -75,6 +80,8 @@ Resampler::resample(const AVFrame* input, AVFrame* output)
return -1;
}
+ // Resampling worked, reset count to 1 so reinit isn't called again
+ initCount_ = 1;
return 0;
}
@@ -126,5 +133,4 @@ Resampler::resample(std::shared_ptr<AudioFrame>&& in, const AudioFormat& format)
return output;
}
-
} // namespace ring
diff --git a/src/media/audio/resampler.h b/src/media/audio/resampler.h
index 9bc5b19e65..b0a6c1d233 100644
--- a/src/media/audio/resampler.h
+++ b/src/media/audio/resampler.h
@@ -3,6 +3,7 @@
*
* Author: Emmanuel Milou <emmanuel.milou@savoirfairelinux.com>
* Author: Alexandre Savard <alexandre.savard@savoirfairelinux.com>
+ * Author: Philippe Gorley <philippe.gorley@savoirfairelinux.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
@@ -22,8 +23,8 @@
#pragma once
#include "audiobuffer.h"
+#include "media_buffer.h"
#include "noncopyable.h"
-#include "ring_types.h"
struct AVFrame;
struct SwrContext;
@@ -34,38 +35,47 @@ namespace ring {
* Wrapper class for libswresample
*/
class Resampler {
- public:
- Resampler();
- ~Resampler();
+public:
+ Resampler();
+ ~Resampler();
- /**
- * Resample from @input format to @output format.
- * NOTE: sample_rate, channel_layout, and format should be set on @output
- */
- int resample(const AVFrame* input, AVFrame* output);
+ /**
+ * Resample from @input format to @output format.
+ * NOTE: sample_rate, channel_layout, and format should be set on @output
+ */
+ int resample(const AVFrame* input, AVFrame* output);
- /**
- * Resample from @dataIn format to @dataOut format.
- *
- * NOTE: This is a wrapper for resample(AVFrame*, AVFrame*)
- */
- void resample(const AudioBuffer& dataIn, AudioBuffer& dataOut);
+ /**
+ * Wrappers around resample(AVFrame*, AVFrame*) for convenience.
+ */
+ void resample(const AudioBuffer& dataIn, AudioBuffer& dataOut);
+ std::unique_ptr<AudioFrame> resample(std::unique_ptr<AudioFrame>&& in, const AudioFormat& out);
+ std::shared_ptr<AudioFrame> resample(std::shared_ptr<AudioFrame>&& in, const AudioFormat& out);
- std::unique_ptr<AudioFrame> resample(std::unique_ptr<AudioFrame>&& in, const AudioFormat& out);
- std::shared_ptr<AudioFrame> resample(std::shared_ptr<AudioFrame>&& in, const AudioFormat& out);
+private:
+ NON_COPYABLE(Resampler);
- private:
- NON_COPYABLE(Resampler);
+ /**
+ * Reinitializes the resampler when new settings are detected. As long as both input and
+ * output formats don't change, this will only be called once.
+ */
+ void reinit(const AVFrame* in, const AVFrame* out);
- /**
- * Reinitializes the resampler when new settings are detected. As long as both input and
- * output buffers always have the same formats, will never be called, as the first
- * initialization is done in swr_convert_frame.
- */
- void reinit(const AVFrame* in, const AVFrame* out);
+ /**
+ * Libswresample resampler context.
+ *
+ * NOTE SwrContext is an imcomplete type and cannot be stored in a smart pointer.
+ */
+ SwrContext* swrCtx_;
- SwrContext* swrCtx_; // incomplete type, cannot be a unique_ptr
- bool initialized_;
+ /**
+ * Number of times @swrCtx_ has been initialized with no successful audio resampling.
+ *
+ * 0: Uninitialized
+ * 1: Initialized
+ * >1: Invalid frames or formats, reinit is going to be called in an infinite loop
+ */
+ unsigned initCount_;
};
} // namespace ring
--
GitLab