From e059062a012ad15b6f4685269e2edea47bb431e3 Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Thu, 24 Jan 2019 17:39:57 -0500
Subject: [PATCH] encoder: cleanup and simplify
Cleans up the encoder a bit and removes methods that weren't used or did
nothing. In any case, it should not be the encoder's responsibility to
mute audio/video.
Removes direct accesses to private data. All these options are accessible
through the av_opt_* API with the AV_OPT_SEARCH_CHILDREN flag.
Adding streams to the encoder is now done separately than opening an RTP
output in an effort to streamline the encoder setup (file vs RTP).
Change-Id: I7a868d098fa942697cfbe3246f368fb9fc7bfb0f
---
src/media/audio/audio_sender.cpp | 10 +-
src/media/media_encoder.cpp | 163 +++++++++------------
src/media/media_encoder.h | 19 +--
src/media/media_recorder.cpp | 6 +-
src/media/video/video_sender.cpp | 14 +-
src/media/video/video_sender.h | 1 -
test/unitTest/media/test_media_encoder.cpp | 6 +-
7 files changed, 87 insertions(+), 132 deletions(-)
diff --git a/src/media/audio/audio_sender.cpp b/src/media/audio/audio_sender.cpp
index af5c9f5c72..6dd1732528 100644
--- a/src/media/audio/audio_sender.cpp
+++ b/src/media/audio/audio_sender.cpp
@@ -69,11 +69,12 @@ AudioSender::setup(SocketPair& socketPair)
try {
/* Encoder setup */
- RING_DBG("audioEncoder_->openLiveOutput %s", dest_.c_str());
- audioEncoder_->setMuted(muteState_);
- audioEncoder_->openLiveOutput(dest_, args_);
+ RING_DBG("audioEncoder_->openOutput %s", dest_.c_str());
+ audioEncoder_->openOutput(dest_, "rtp");
+ audioEncoder_->setOptions(args_);
+ audioEncoder_->addStream(args_.codec->systemCodecInfo);
audioEncoder_->setInitSeqVal(seqVal_);
- audioEncoder_->setIOContext(muxContext_);
+ audioEncoder_->setIOContext(muxContext_->getContext());
audioEncoder_->startIO();
} catch (const MediaEncoderException &e) {
RING_ERR("%s", e.what());
@@ -114,7 +115,6 @@ AudioSender::setMuted(bool isMuted)
{
muteState_ = isMuted;
audioInput_->setMuted(isMuted);
- audioEncoder_->setMuted(isMuted);
}
uint16_t
diff --git a/src/media/media_encoder.cpp b/src/media/media_encoder.cpp
index a7dd59611d..14a47efba3 100644
--- a/src/media/media_encoder.cpp
+++ b/src/media/media_encoder.cpp
@@ -24,9 +24,7 @@
#include "media_codec.h"
#include "media_encoder.h"
#include "media_buffer.h"
-#include "media_io_handle.h"
-#include "audio/audiobuffer.h"
#include "fileutils.h"
#include "string_utils.h"
#include "logger.h"
@@ -54,10 +52,8 @@ MediaEncoder::MediaEncoder()
MediaEncoder::~MediaEncoder()
{
if (outputCtx_) {
- if (outputCtx_->priv_data)
- av_write_trailer(outputCtx_);
-
- for (auto encoderCtx : encoders_)
+ av_write_trailer(outputCtx_);
+ for (auto encoderCtx : encoders_) {
if (encoderCtx) {
#ifndef _MSC_VER
avcodec_free_context(&encoderCtx);
@@ -65,14 +61,14 @@ MediaEncoder::~MediaEncoder()
avcodec_close(encoderCtx);
#endif
}
-
+ }
avformat_free_context(outputCtx_);
}
-
av_dict_free(&options_);
}
-void MediaEncoder::setDeviceOptions(const DeviceParams& args)
+void
+MediaEncoder::setDeviceOptions(const DeviceParams& args)
{
device_ = args;
// Make sure width and height are even (required by x264)
@@ -88,7 +84,8 @@ void MediaEncoder::setDeviceOptions(const DeviceParams& args)
libav_utils::setDictValue(&options_, "framerate", ring::to_string(device_.framerate.real()));
}
-void MediaEncoder::setOptions(const MediaDescription& args)
+void
+MediaEncoder::setOptions(const MediaDescription& args)
{
codec_ = args.codec;
@@ -115,21 +112,46 @@ void MediaEncoder::setOptions(const MediaDescription& args)
libav_utils::setDictValue(&options_, "parameters", args.parameters);
}
+void
+MediaEncoder::setOptions(std::map<std::string, std::string> options)
+{
+ const auto& titleIt = options.find("title");
+ if (titleIt != options.end() and not titleIt->second.empty())
+ libav_utils::setDictValue(&outputCtx_->metadata, titleIt->first, titleIt->second);
+ const auto& descIt = options.find("description");
+ if (descIt != options.end() and not descIt->second.empty())
+ libav_utils::setDictValue(&outputCtx_->metadata, descIt->first, descIt->second);
+
+ auto bitrate = SystemCodecInfo::DEFAULT_MAX_BITRATE;
+ auto quality = SystemCodecInfo::DEFAULT_CODEC_QUALITY;
+ // ensure all options retrieved later on are in options_ (insert does nothing if key exists)
+ options.insert({"max_rate", ring::to_string(bitrate)});
+ options.insert({"crf", ring::to_string(quality)});
+ options.insert({"sample_rate", "8000"});
+ options.insert({"channels", "2"});
+ int sampleRate = atoi(options["sample_rate"].c_str());
+ options.insert({"frame_size", ring::to_string(static_cast<unsigned>(0.02*sampleRate))});
+ options.insert({"width", "320"});
+ options.insert({"height", "240"});
+ options.insert({"framerate", "30.00"});
+ for (const auto& it : options)
+ libav_utils::setDictValue(&options_, it.first, it.second);
+}
+
void
MediaEncoder::setInitSeqVal(uint16_t seqVal)
{
//only set not default value (!=0)
if (seqVal != 0)
- libav_utils::setDictValue(&options_, "seq", ring::to_string(seqVal));
+ av_opt_set_int(outputCtx_, "seq", seqVal, AV_OPT_SEARCH_CHILDREN);
}
uint16_t
MediaEncoder::getLastSeqValue()
{
- int64_t retVal;
- auto ret = av_opt_get_int(outputCtx_->priv_data, "seq", AV_OPT_SEARCH_CHILDREN, &retVal);
- if (ret == 0)
- return (uint16_t) retVal;
+ int64_t retVal;
+ if (av_opt_get_int(outputCtx_, "seq", AV_OPT_SEARCH_CHILDREN, &retVal) >= 0)
+ return (uint16_t)retVal;
else
return 0;
}
@@ -141,60 +163,17 @@ MediaEncoder::getEncoderName() const
}
void
-MediaEncoder::openLiveOutput(const std::string& filename,
- const ring::MediaDescription& args)
-{
- setOptions(args);
- AVOutputFormat *oformat = av_guess_format("rtp", filename.c_str(), nullptr);
-
- if (!oformat) {
- RING_ERR("Unable to find a suitable output format for %s", filename.c_str());
- throw MediaEncoderException("No output format");
- }
-
- outputCtx_->oformat = oformat;
-#if LIBAVFORMAT_VERSION_INT >= AV_VERSION_INT(58, 7, 100)
- // c_str guarantees null termination
- outputCtx_->url = av_strdup(filename.c_str()); // must be compatible with av_free
-#else
- strncpy(outputCtx_->filename, filename.c_str(), sizeof(outputCtx_->filename));
- // in case our filename is longer than the space reserved for AVFormatContext.filename
- outputCtx_->filename[sizeof(outputCtx_->filename) - 1] = '\0';
-#endif
-
- addStream(args.codec->systemCodecInfo, args.parameters);
-}
-
-void
-MediaEncoder::openFileOutput(const std::string& filename, std::map<std::string, std::string> options)
+MediaEncoder::openOutput(const std::string& filename, const std::string& format)
{
avformat_free_context(outputCtx_);
- avformat_alloc_output_context2(&outputCtx_, nullptr, nullptr, filename.c_str());
-
- if (!options["title"].empty())
- libav_utils::setDictValue(&outputCtx_->metadata, "title", options["title"]);
- if (!options["description"].empty())
- libav_utils::setDictValue(&outputCtx_->metadata, "description", options["description"]);
-
- auto bitrate = SystemCodecInfo::DEFAULT_MAX_BITRATE;
- auto quality = SystemCodecInfo::DEFAULT_CODEC_QUALITY;
- // ensure all options retrieved later on are in options_ (insert does nothing if key exists)
- options.insert({"max_rate", ring::to_string(bitrate)});
- options.insert({"crf", ring::to_string(quality)});
- options.insert({"sample_rate", "8000"});
- options.insert({"channels", "2"});
- int sampleRate = atoi(options["sample_rate"].c_str());
- options.insert({"frame_size", ring::to_string(static_cast<unsigned>(0.02*sampleRate))});
- options.insert({"width", "320"});
- options.insert({"height", "240"});
- options.insert({"framerate", "30.00"});
- for (const auto& it : options)
- libav_utils::setDictValue(&options_, it.first, it.second);
- // for a file output, addStream is done by the caller, as there may be multiple streams
+ if (format.empty())
+ avformat_alloc_output_context2(&outputCtx_, nullptr, nullptr, filename.c_str());
+ else
+ avformat_alloc_output_context2(&outputCtx_, nullptr, format.c_str(), filename.c_str());
}
int
-MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo, std::string parameters)
+MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo)
{
AVCodec* outputCodec = nullptr;
AVCodecContext* encoderCtx = nullptr;
@@ -219,7 +198,8 @@ MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo, std::string para
/* let x264 preset override our encoder settings */
if (systemCodecInfo.avcodecId == AV_CODEC_ID_H264) {
- extractProfileLevelID(parameters, encoderCtx);
+ auto profileLevelId = libav_utils::getDictValue(options_, "parameters");
+ extractProfileLevelID(profileLevelId, encoderCtx);
forcePresetX264(encoderCtx);
// For H264 :
// Streaming => VBV (constrained encoding) + CRF (Constant Rate Factor)
@@ -227,7 +207,7 @@ MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo, std::string para
crf = 30; // good value for H264-720p@30
RING_DBG("H264 encoder setup: crf=%u, maxrate=%u, bufsize=%u", crf, maxBitrate, bufSize);
- av_opt_set_int(encoderCtx->priv_data, "crf", crf, 0);
+ av_opt_set_int(encoderCtx, "crf", crf, AV_OPT_SEARCH_CHILDREN);
encoderCtx->rc_buffer_size = bufSize;
encoderCtx->rc_max_rate = maxBitrate;
} else if (systemCodecInfo.avcodecId == AV_CODEC_ID_VP8) {
@@ -237,14 +217,14 @@ MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo, std::string para
// 2- otherwise set rc_max_rate and rc_buffer_size
// Using information given on this page:
// http://www.webmproject.org/docs/encoder-parameters/
- av_opt_set(encoderCtx->priv_data, "quality", "realtime", 0);
- av_opt_set_int(encoderCtx->priv_data, "error-resilient", 1, 0);
- av_opt_set_int(encoderCtx->priv_data, "cpu-used", 7, 0); // value obtained from testing
- av_opt_set_int(encoderCtx->priv_data, "lag-in-frames", 0, 0);
+ av_opt_set(encoderCtx, "quality", "realtime", AV_OPT_SEARCH_CHILDREN);
+ av_opt_set_int(encoderCtx, "error-resilient", 1, AV_OPT_SEARCH_CHILDREN);
+ av_opt_set_int(encoderCtx, "cpu-used", 7, AV_OPT_SEARCH_CHILDREN); // value obtained from testing
+ av_opt_set_int(encoderCtx, "lag-in-frames", 0, AV_OPT_SEARCH_CHILDREN);
// allow encoder to drop frames if buffers are full and
// to undershoot target bitrate to lessen strain on resources
- av_opt_set_int(encoderCtx->priv_data, "drop-frame", 25, 0);
- av_opt_set_int(encoderCtx->priv_data, "undershoot-pct", 95, 0);
+ av_opt_set_int(encoderCtx, "drop-frame", 25, AV_OPT_SEARCH_CHILDREN);
+ av_opt_set_int(encoderCtx, "undershoot-pct", 95, AV_OPT_SEARCH_CHILDREN);
// don't set encoderCtx->gop_size: let libvpx decide when to insert a keyframe
encoderCtx->slices = 2; // VP8E_SET_TOKEN_PARTITIONS
encoderCtx->qmin = 4;
@@ -252,7 +232,7 @@ MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo, std::string para
encoderCtx->rc_buffer_size = maxBitrate;
encoderCtx->bit_rate = maxBitrate;
if (crf != SystemCodecInfo::DEFAULT_NO_QUALITY) {
- av_opt_set_int(encoderCtx->priv_data, "crf", crf, 0);
+ av_opt_set_int(encoderCtx, "crf", crf, AV_OPT_SEARCH_CHILDREN);
RING_DBG("Using quality factor %d", crf);
} else {
RING_DBG("Using Max bitrate %d", maxBitrate);
@@ -308,20 +288,11 @@ MediaEncoder::addStream(const SystemCodecInfo& systemCodecInfo, std::string para
return stream->index;
}
-void MediaEncoder::setInterruptCallback(int (*cb)(void*), void *opaque)
-{
- if (cb) {
- outputCtx_->interrupt_callback.callback = cb;
- outputCtx_->interrupt_callback.opaque = opaque;
- } else {
- outputCtx_->interrupt_callback.callback = 0;
- }
-}
-
-void MediaEncoder::setIOContext(const std::unique_ptr<MediaIOHandle> &ioctx)
+void
+MediaEncoder::setIOContext(AVIOContext* ioctx)
{
if (ioctx) {
- outputCtx_->pb = ioctx->getContext();
+ outputCtx_->pb = ioctx;
outputCtx_->packet_size = outputCtx_->pb->buffer_size;
} else {
int ret = 0;
@@ -387,7 +358,8 @@ MediaEncoder::encode(VideoFrame& input, bool is_keyframe,
}
#endif // RING_VIDEO
-int MediaEncoder::encodeAudio(AudioFrame& frame)
+int
+MediaEncoder::encodeAudio(AudioFrame& frame)
{
frame.pointer()->pts = sent_samples;
sent_samples += frame.pointer()->nb_samples;
@@ -487,7 +459,8 @@ MediaEncoder::print_sdp()
return result;
}
-AVCodecContext* MediaEncoder::prepareEncoderContext(AVCodec* outputCodec, bool is_video)
+AVCodecContext*
+MediaEncoder::prepareEncoderContext(AVCodec* outputCodec, bool is_video)
{
AVCodecContext* encoderCtx = avcodec_alloc_context3(outputCodec);
@@ -577,17 +550,19 @@ AVCodecContext* MediaEncoder::prepareEncoderContext(AVCodec* outputCodec, bool i
return encoderCtx;
}
-void MediaEncoder::forcePresetX264(AVCodecContext* encoderCtx)
+void
+MediaEncoder::forcePresetX264(AVCodecContext* encoderCtx)
{
const char *speedPreset = "ultrafast";
- if (av_opt_set(encoderCtx->priv_data, "preset", speedPreset, 0))
+ if (av_opt_set(encoderCtx, "preset", speedPreset, AV_OPT_SEARCH_CHILDREN))
RING_WARN("Failed to set x264 preset '%s'", speedPreset);
const char *tune = "zerolatency";
- if (av_opt_set(encoderCtx->priv_data, "tune", tune, 0))
+ if (av_opt_set(encoderCtx, "tune", tune, AV_OPT_SEARCH_CHILDREN))
RING_WARN("Failed to set x264 tune '%s'", tune);
}
-void MediaEncoder::extractProfileLevelID(const std::string ¶meters,
+void
+MediaEncoder::extractProfileLevelID(const std::string ¶meters,
AVCodecContext *ctx)
{
// From RFC3984:
@@ -635,12 +610,6 @@ void MediaEncoder::extractProfileLevelID(const std::string ¶meters,
RING_DBG("Using profile %x and level %d", ctx->profile, ctx->level);
}
-void
-MediaEncoder::setMuted(bool isMuted)
-{
- is_muted = isMuted;
-}
-
bool
MediaEncoder::useCodec(const ring::AccountCodecInfo* codec) const noexcept
{
diff --git a/src/media/media_encoder.h b/src/media/media_encoder.h
index 6832b64944..9487a69e8d 100644
--- a/src/media/media_encoder.h
+++ b/src/media/media_encoder.h
@@ -41,18 +41,14 @@
#include <vector>
struct AVCodecContext;
-struct AVStream;
struct AVFormatContext;
struct AVDictionary;
struct AVCodec;
namespace ring {
-class AudioBuffer;
-class MediaIOHandle;
struct MediaDescription;
struct AccountCodecInfo;
-class MediaRecorder;
class MediaEncoderException : public std::runtime_error {
public:
@@ -64,14 +60,13 @@ public:
MediaEncoder();
~MediaEncoder();
- void setInterruptCallback(int (*cb)(void*), void *opaque);
-
+ void openOutput(const std::string& filename, const std::string& format="");
void setDeviceOptions(const DeviceParams& args);
- void openLiveOutput(const std::string& filename, const MediaDescription& args);
- void openFileOutput(const std::string& filename, std::map<std::string, std::string> options);
- int addStream(const SystemCodecInfo& codec, std::string parameters = "");
+ void setOptions(const MediaDescription& args);
+ void setOptions(std::map<std::string, std::string> options);
+ int addStream(const SystemCodecInfo& codec);
+ void setIOContext(AVIOContext* ioctx);
void startIO();
- void setIOContext(const std::unique_ptr<MediaIOHandle> &ioctx);
bool send(AVPacket& packet, int streamIdx = -1);
@@ -93,7 +88,6 @@ public:
int getWidth() const { return device_.width; }
int getHeight() const { return device_.height; }
- void setMuted(bool isMuted);
void setInitSeqVal(uint16_t seqVal);
uint16_t getLastSeqValue();
std::string getEncoderName() const;
@@ -105,8 +99,6 @@ public:
private:
NON_COPYABLE(MediaEncoder);
- void setOptions(const MediaDescription& args);
- void setScaleDest(void *data, int width, int height, int pix_fmt);
AVCodecContext* prepareEncoderContext(AVCodec* outputCodec, bool is_video);
void forcePresetX264(AVCodecContext* encoderCtx);
void extractProfileLevelID(const std::string ¶meters, AVCodecContext *ctx);
@@ -123,7 +115,6 @@ private:
std::vector<uint8_t> scaledFrameBuffer_;
int scaledFrameBufferSize_ = 0;
- bool is_muted = false;
protected:
void readConfig(AVDictionary** dict, AVCodecContext* encoderCtx);
diff --git a/src/media/media_recorder.cpp b/src/media/media_recorder.cpp
index d8d7fff6f2..1b466dec06 100644
--- a/src/media/media_recorder.cpp
+++ b/src/media/media_recorder.cpp
@@ -225,7 +225,8 @@ MediaRecorder::initRecord()
encoderOptions["channels"] = std::to_string(audioStream.nbChannels);
}
- encoder_->openFileOutput(getPath(), encoderOptions);
+ encoder_->openOutput(getPath());
+ encoder_->setOptions(encoderOptions);
if (hasVideo_) {
auto videoCodec = std::static_pointer_cast<ring::SystemVideoCodecInfo>(
@@ -248,8 +249,7 @@ MediaRecorder::initRecord()
}
try {
- std::unique_ptr<MediaIOHandle> ioHandle;
- encoder_->setIOContext(ioHandle);
+ encoder_->setIOContext(nullptr);
encoder_->startIO();
} catch (const MediaEncoderException& e) {
RING_ERR() << "Could not start recorder: " << e.what();
diff --git a/src/media/video/video_sender.cpp b/src/media/video/video_sender.cpp
index 8d2d897554..3a62163a7a 100644
--- a/src/media/video/video_sender.cpp
+++ b/src/media/video/video_sender.cpp
@@ -42,11 +42,13 @@ VideoSender::VideoSender(const std::string& dest, const DeviceParams& dev,
: muxContext_(socketPair.createIOContext(mtu))
, videoEncoder_(new MediaEncoder)
{
- videoEncoder_->setDeviceOptions(dev);
keyFrameFreq_ = dev.framerate.numerator() * KEY_FRAME_PERIOD;
- videoEncoder_->openLiveOutput(dest, args);
+ videoEncoder_->openOutput(dest, "rtp");
+ videoEncoder_->setDeviceOptions(dev);
+ videoEncoder_->setOptions(args);
+ videoEncoder_->addStream(args.codec->systemCodecInfo);
videoEncoder_->setInitSeqVal(seqVal);
- videoEncoder_->setIOContext(muxContext_);
+ videoEncoder_->setIOContext(muxContext_->getContext());
videoEncoder_->startIO();
videoEncoder_->print_sdp();
@@ -101,12 +103,6 @@ VideoSender::forceKeyFrame()
++forceKeyFrame_;
}
-void
-VideoSender::setMuted(bool isMuted)
-{
- videoEncoder_->setMuted(isMuted);
-}
-
uint16_t
VideoSender::getLastSeqValue()
{
diff --git a/src/media/video/video_sender.h b/src/media/video/video_sender.h
index 59f8196ee5..551808dab4 100644
--- a/src/media/video/video_sender.h
+++ b/src/media/video/video_sender.h
@@ -57,7 +57,6 @@ public:
void update(Observable<std::shared_ptr<MediaFrame>>* obs,
const std::shared_ptr<MediaFrame>& frame_p) override;
- void setMuted(bool isMuted);
uint16_t getLastSeqValue();
bool useCodec(const AccountVideoCodecInfo* codec) const;
diff --git a/test/unitTest/media/test_media_encoder.cpp b/test/unitTest/media/test_media_encoder.cpp
index fdbc5cf18e..414c688fa0 100644
--- a/test/unitTest/media/test_media_encoder.cpp
+++ b/test/unitTest/media/test_media_encoder.cpp
@@ -48,7 +48,6 @@ private:
CPPUNIT_TEST_SUITE_END();
std::unique_ptr<MediaEncoder> encoder_;
- std::unique_ptr<MediaIOHandle> ioHandle_;
std::vector<std::string> files_;
};
@@ -162,7 +161,8 @@ MediaEncoderTest::testMultiStream()
);
try {
- encoder_->openFileOutput("test.mkv", options);
+ encoder_->openOutput("test.mkv");
+ encoder_->setOptions(options);
int videoIdx = encoder_->addStream(*vp8Codec.get());
CPPUNIT_ASSERT(videoIdx >= 0);
CPPUNIT_ASSERT(encoder_->getStreamCount() == 1);
@@ -170,7 +170,7 @@ MediaEncoderTest::testMultiStream()
CPPUNIT_ASSERT(audioIdx >= 0);
CPPUNIT_ASSERT(videoIdx != audioIdx);
CPPUNIT_ASSERT(encoder_->getStreamCount() == 2);
- encoder_->setIOContext(ioHandle_);
+ encoder_->setIOContext(nullptr);
encoder_->startIO();
int sentSamples = 0;
AVFrame* audio = nullptr;
--
GitLab