Commit ad477a2f authored by Guillaume Roguez's avatar Guillaume Roguez

media: refactor live bitrate change

This refactoring improves user experience, fix an issue and simplify code.

- less exposed "internal" stuff (RTP sequence value)
- don't expose internal business logic to upper layers.
- remove uneeded full media restart: only media_encoder is restarted
- fixed non protected shared access to codec isRunning value causing
  a race condition on access and good design practices violation
  (tight coupling between unrelated classes).

Refs #77483

Change-Id: Idb1e3940601ff0d483e7721f627a0c8dd766f844
parent e8475cd7
......@@ -51,6 +51,7 @@ namespace ring {
class VoIPLink;
class Account;
class AccountVideoCodecInfo;
template <class T> using CallMap = std::map<std::string, std::shared_ptr<T> >;
......@@ -327,6 +328,10 @@ class Call : public Recordable, public std::enable_shared_from_this<Call> {
return iceTransport_;
}
virtual bool useVideoCodec(const AccountVideoCodecInfo* /*codec*/) const {
return false;
}
virtual void restartMediaSender() = 0;
protected:
......
......@@ -410,12 +410,13 @@ setCodecDetails(const std::string& accountID,
if (auto foundCodec = std::static_pointer_cast<ring::AccountVideoCodecInfo>(codec)) {
foundCodec->setCodecSpecifications(details);
RING_WARN("parameters for %s changed ",
foundCodec->systemCodecInfo.name.c_str());
if (foundCodec->isRunning) {
RING_WARN("%s running. Need to restart encoding",
foundCodec->systemCodecInfo.name.c_str());
if (auto call = ring::Manager::instance().getCurrentCall())
foundCodec->systemCodecInfo.name.c_str());
if (auto call = ring::Manager::instance().getCurrentCall()) {
if (call->useVideoCodec(foundCodec.get())) {
RING_WARN("%s running. Need to restart encoding",
foundCodec->systemCodecInfo.name.c_str());
call->restartMediaSender();
}
}
ring::emitSignal<ConfigurationSignal::MediaParametersChanged>(accountID);
return true;
......
......@@ -135,7 +135,7 @@ class IAXCall : public Call
void muteMedia(const std::string& mediaType, bool isMuted) {}
//TODO: implement restartMedia for IAX
void restartMediaSender() {}
void restartMediaSender() override {}
void peerHungup();
......
......@@ -394,6 +394,16 @@ AudioRtpSession::startSender()
}
}
void
AudioRtpSession::restartSender()
{
std::lock_guard<std::recursive_mutex> lock(mutex_);
// continue on last sequence number
initSeqVal_ = sender_->getLastSeqValue() + 1;
startSender();
}
void
AudioRtpSession::startReceiver()
{
......@@ -491,20 +501,4 @@ AudioRtpSession::setMuted(bool isMuted)
sender_->setMuted(isMuted);
}
void
AudioRtpSession::setSenderInitSeqVal(const uint16_t seqVal)
{
std::lock_guard<std::recursive_mutex> lock(mutex_);
initSeqVal_ = seqVal;
}
uint16_t
AudioRtpSession::getSenderLastSeqValue()
{
std::lock_guard<std::recursive_mutex> lock(mutex_);
if (sender_)
return sender_->getLastSeqValue();
return 0;
}
} // namespace ring
......@@ -54,10 +54,9 @@ class AudioRtpSession : public RtpSession {
void start();
void start(std::unique_ptr<IceSocket> rtp_sock,
std::unique_ptr<IceSocket> rtcp_sock);
void restartSender() override;
void stop();
void setMuted(bool isMuted);
void setSenderInitSeqVal(const uint16_t seqVal);
uint16_t getSenderLastSeqValue();
private:
void startSender();
......@@ -66,7 +65,7 @@ class AudioRtpSession : public RtpSession {
std::unique_ptr<AudioSender> sender_;
std::unique_ptr<AudioReceiveThread> receiveThread_;
std::shared_ptr<RingBuffer> ringbuffer_;
uint16_t initSeqVal_;
uint16_t initSeqVal_ = 0;
};
} // namespace ring
......
......@@ -145,7 +145,6 @@ AccountCodecInfo::AccountCodecInfo(const SystemCodecInfo& sysCodecInfo)
: systemCodecInfo(sysCodecInfo)
, order(0)
, isActive(true)
, isRunning(false)
, payloadType(sysCodecInfo.payloadType)
, bitrate(sysCodecInfo.bitrate)
{}
......
......@@ -147,7 +147,6 @@ struct AccountCodecInfo
const SystemCodecInfo& systemCodecInfo;
unsigned order; /*used to define prefered codec list order in UI*/
bool isActive;
bool isRunning; /*is this codec processing data or not*/
/* account custom values */
unsigned payloadType;
unsigned bitrate;
......
......@@ -54,10 +54,8 @@ MediaEncoder::MediaEncoder()
MediaEncoder::~MediaEncoder()
{
if (outputCtx_ and outputCtx_->priv_data) {
if (outputCtx_ and outputCtx_->priv_data)
av_write_trailer(outputCtx_);
*isCodecRunning = false;
}
if (encoderCtx_)
avcodec_close(encoderCtx_);
......@@ -83,6 +81,8 @@ void MediaEncoder::setDeviceOptions(const DeviceParams& args)
void MediaEncoder::setOptions(const MediaDescription& args)
{
codec_ = args.codec;
av_dict_set(&options_, "payload_type", ring::to_string(args.payload_type).c_str(), 0);
av_dict_set(&options_, "bitrate", ring::to_string(args.codec->bitrate).c_str(), 0);
......@@ -103,8 +103,6 @@ void MediaEncoder::setOptions(const MediaDescription& args)
if (not args.parameters.empty())
av_dict_set(&options_, "parameters", args.parameters.c_str(), 0);
isCodecRunning = &(args.codec->isRunning);
}
void
......@@ -240,7 +238,6 @@ MediaEncoder::startIO()
}
av_dump_format(outputCtx_, 0, outputCtx_->filename, 1);
*isCodecRunning = true;
}
static void
......@@ -668,4 +665,10 @@ MediaEncoder::setMuted(bool isMuted)
is_muted = isMuted;
}
bool
MediaEncoder::useCodec(const ring::AccountCodecInfo* codec) const noexcept
{
return codec_.get() == codec;
}
} // namespace ring
......@@ -58,6 +58,7 @@ namespace ring {
class AudioBuffer;
class MediaIOHandle;
class MediaDescription;
class AccountCodecInfo;
class MediaEncoderException : public std::runtime_error {
public:
......@@ -94,6 +95,7 @@ public:
void setInitSeqVal(uint16_t seqVal);
uint16_t getLastSeqValue();
bool useCodec(const AccountCodecInfo* codec) const noexcept;
private:
NON_COPYABLE(MediaEncoder);
......@@ -122,11 +124,11 @@ private:
int encoderBufferSize_ = 0;
#endif
bool is_muted = false;
bool* isCodecRunning = nullptr;
protected:
AVDictionary *options_ = nullptr;
DeviceParams device_;
std::shared_ptr<const AccountCodecInfo> codec_;
};
} // namespace ring
......
......@@ -49,6 +49,7 @@ public:
virtual void start() = 0;
virtual void start(std::unique_ptr<IceSocket> rtp_sock,
std::unique_ptr<IceSocket> rtcp_sock) = 0;
virtual void restartSender() = 0;
virtual void stop() = 0;
virtual void updateMedia(const MediaDescription& send,
......@@ -57,9 +58,6 @@ public:
receive_ = receive;
}
virtual void setSenderInitSeqVal(uint16_t seqVal) = 0;
virtual uint16_t getSenderLastSeqValue() = 0;
bool isSending() { return send_.enabled; }
bool isReceiving() { return receive_.enabled; }
......
......@@ -101,9 +101,19 @@ void VideoRtpSession::startSender()
}
}
void VideoRtpSession::restartSender()
void
VideoRtpSession::restartSender()
{
startSender();
std::lock_guard<std::recursive_mutex> lock(mutex_);
// continue on last sequence number
initSeqVal_ = sender_->getLastSeqValue() + 1;
startSender();
if (sender_) {
if (videoLocal_)
videoLocal_->attach(sender_.get());
}
}
void VideoRtpSession::startReceiver()
......@@ -287,20 +297,10 @@ void VideoRtpSession::exitConference()
conference_ = nullptr;
}
void
VideoRtpSession::setSenderInitSeqVal(const uint16_t seqVal)
{
std::lock_guard<std::recursive_mutex> lock(mutex_);
initSeqVal_ = seqVal;
}
uint16_t
VideoRtpSession::getSenderLastSeqValue()
bool
VideoRtpSession::useCodec(const ring::AccountVideoCodecInfo* codec) const
{
std::lock_guard<std::recursive_mutex> lock(mutex_);
if (sender_)
return sender_->getLastSeqValue();
return 0;
return sender_->useCodec(codec);
}
}} // namespace ring::video
......@@ -59,6 +59,7 @@ public:
void start();
void start(std::unique_ptr<IceSocket> rtp_sock,
std::unique_ptr<IceSocket> rtcp_sock);
void restartSender() override;
void stop();
void forceKeyFrame();
......@@ -70,9 +71,7 @@ public:
input_ = input;
}
void restartSender();
void setSenderInitSeqVal(uint16_t seqVal);
uint16_t getSenderLastSeqValue();
bool useCodec(const AccountVideoCodecInfo* codec) const;
private:
void setupConferenceVideoPipeline(Conference *conference);
......
......@@ -97,4 +97,10 @@ VideoSender::getLastSeqValue()
return videoEncoder_->getLastSeqValue();
}
bool
VideoSender::useCodec(const ring::AccountVideoCodecInfo* codec) const
{
return videoEncoder_->useCodec(codec);
}
}} // namespace ring::video
......@@ -42,8 +42,10 @@
#include <memory>
#include <atomic>
// Forward declarations
namespace ring {
class SocketPair;
class SocketPair;
class AccountVideoCodecInfo;
}
namespace ring { namespace video {
......@@ -69,6 +71,8 @@ public:
void setMuted(bool isMuted);
uint16_t getLastSeqValue();
bool useCodec(const AccountVideoCodecInfo* codec) const;
private:
NON_COPYABLE(VideoSender);
......
......@@ -800,8 +800,14 @@ SIPCall::startIce()
return iceTransport_->start(rem_ice_attrs, getAllRemoteCandidates());
}
bool
SIPCall::useVideoCodec(const AccountVideoCodecInfo* codec) const
{
return videortp_.useCodec(codec);
}
void
SIPCall::startAllMedia(const uint16_t seqVideoInitVal, const uint16_t seqAudioInitVal)
SIPCall::startAllMedia()
{
if (isSecure() && not transport_->isSecure()) {
RING_ERR("Can't perform secure call over insecure SIP transport");
......@@ -849,14 +855,9 @@ SIPCall::startAllMedia(const uint16_t seqVideoInitVal, const uint16_t seqAudioIn
}
#ifdef RING_VIDEO
if (local.type == MEDIA_VIDEO) {
if (local.type == MEDIA_VIDEO)
videortp_.switchInput(videoInput_);
rtp->setSenderInitSeqVal(seqVideoInitVal);
}
#endif
if (local.type == MEDIA_AUDIO){
rtp->setSenderInitSeqVal(seqAudioInitVal);
}
rtp->updateMedia(remote, local);
if (isIceRunning()) {
......@@ -888,18 +889,11 @@ SIPCall::startAllMedia(const uint16_t seqVideoInitVal, const uint16_t seqAudioIn
void
SIPCall::restartMediaSender()
{
uint16_t lastSeqVideoVal, lastSeqAudioVal = 0;
RING_WARN("[call:%s] restarting TX media streams", getCallId().c_str());
avformatrtp_->restartSender();
#ifdef RING_VIDEO
lastSeqVideoVal = videortp_.getSenderLastSeqValue() + 1;
lastSeqAudioVal = avformatrtp_->getSenderLastSeqValue() + 1;
RING_WARN("restarting video and audio streams");
#else
lastSeqAudioVal = avformatrtp_->getSenderLastSeqValue() + 1;
RING_WARN("restarting audio streams");
videortp_.restartSender();
#endif
stopAllMedia();
RING_ERR("lastSeqVideoVal=%u, lastSeqAudioVal=%u", lastSeqVideoVal,lastSeqAudioVal);
startAllMedia(lastSeqVideoVal, lastSeqAudioVal);
}
void
......
......@@ -199,8 +199,7 @@ class SIPCall : public Call
bool startIce();
void startAllMedia(const uint16_t seqVideoInitVal = 0,
const uint16_t seqAudioInitVal = 0);
void startAllMedia();
void onMediaUpdate();
......@@ -210,7 +209,9 @@ class SIPCall : public Call
void muteMedia(const std::string& mediaType, bool isMuted);
void restartMediaSender();
void restartMediaSender() override;
bool useVideoCodec(const AccountVideoCodecInfo* codec) const override;
virtual std::map<std::string, std::string> getDetails() const;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment