From 950ce8f142bedd2d6b8e5319f3519f897b01d9cf Mon Sep 17 00:00:00 2001
From: philippegorley <philippe.gorley@savoirfairelinux.com>
Date: Sun, 26 Nov 2017 19:53:32 -0500
Subject: [PATCH] video: request keyframe on missed packet
Keyframes will only be sent on request instead of a timely interval.
This may interfere with clients that do not yet have this patch.
Change-Id: Ie786160744257052ef9d103d50fe46cd90138927
Reviewed-by: Olivier Soldano <olivier.soldano@savoirfairelinux.com>
---
src/media/media_encoder.cpp | 2 ++
src/media/rtp_session.h | 2 +-
src/media/video/video_receive_thread.cpp | 18 ++++++++++++++++--
src/media/video/video_receive_thread.h | 3 ++-
src/media/video/video_rtp_session.cpp | 6 ++----
src/media/video/video_sender.cpp | 4 ++--
src/media/video/video_sender.h | 3 ++-
7 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/src/media/media_encoder.cpp b/src/media/media_encoder.cpp
index d9e288f950..5d66871b29 100644
--- a/src/media/media_encoder.cpp
+++ b/src/media/media_encoder.cpp
@@ -285,8 +285,10 @@ MediaEncoder::encode(VideoFrame& input, bool is_keyframe,
if (is_keyframe) {
frame->pict_type = AV_PICTURE_TYPE_I;
+ frame->key_frame = 1;
} else {
frame->pict_type = AV_PICTURE_TYPE_NONE;
+ frame->key_frame = 0;
}
AVPacket pkt;
diff --git a/src/media/rtp_session.h b/src/media/rtp_session.h
index 2a80154ccd..7f17dc9560 100644
--- a/src/media/rtp_session.h
+++ b/src/media/rtp_session.h
@@ -57,7 +57,7 @@ public:
protected:
std::recursive_mutex mutex_;
- std::unique_ptr<SocketPair> socketPair_;
+ std::shared_ptr<SocketPair> socketPair_;
const std::string callID_;
MediaDescription send_;
diff --git a/src/media/video/video_receive_thread.cpp b/src/media/video/video_receive_thread.cpp
index f9397beef1..b40fed421a 100644
--- a/src/media/video/video_receive_thread.cpp
+++ b/src/media/video/video_receive_thread.cpp
@@ -36,6 +36,8 @@ namespace ring { namespace video {
using std::string;
+static constexpr uint32_t RTCP_RR_FRACTION_MASK = 0xFF000000;
+
VideoReceiveThread::VideoReceiveThread(const std::string& id,
const std::string &sdp,
const bool isReset,
@@ -162,15 +164,27 @@ int VideoReceiveThread::readFunction(void *opaque, uint8_t *buf, int buf_size)
return is.gcount();
}
-void VideoReceiveThread::addIOContext(SocketPair &socketPair)
+void VideoReceiveThread::addIOContext(std::shared_ptr<SocketPair> socketPair)
{
- demuxContext_.reset(socketPair.createIOContext(mtu_));
+ demuxContext_.reset(socketPair->createIOContext(mtu_));
+ socketPair_ = socketPair;
}
bool VideoReceiveThread::decodeFrame()
{
const auto ret = videoDecoder_->decode(getNewFrame());
+ if (requestKeyFrameCallback_) {
+ auto rtcpPackets = socketPair_->getRtcpInfo();
+ if (rtcpPackets.size() != 0) {
+ // recent packet loss detected, ask for keyframe
+ if (ntohl(rtcpPackets.back().fraction_lost) & RTCP_RR_FRACTION_MASK) {
+ RING_DBG("Sending keyframe request");
+ requestKeyFrameCallback_(id_);
+ }
+ }
+ }
+
switch (ret) {
case MediaDecoder::Status::FrameFinished:
publishFrame();
diff --git a/src/media/video/video_receive_thread.h b/src/media/video/video_receive_thread.h
index 5b2253023a..08d84cdaaa 100644
--- a/src/media/video/video_receive_thread.h
+++ b/src/media/video/video_receive_thread.h
@@ -51,7 +51,7 @@ public:
~VideoReceiveThread();
void startLoop();
- void addIOContext(SocketPair &socketPair);
+ void addIOContext(std::shared_ptr<SocketPair> socketPair);
void setRequestKeyFrameCallback(void (*)(const std::string &));
void enterConference();
void exitConference();
@@ -81,6 +81,7 @@ private:
std::atomic_bool restartDecoder_;
bool isReset_;
uint16_t mtu_;
+ std::shared_ptr<SocketPair> socketPair_;
void (*requestKeyFrameCallback_)(const std::string &);
void openDecoder();
diff --git a/src/media/video/video_rtp_session.cpp b/src/media/video/video_rtp_session.cpp
index db9cad6ddf..5d35eb9018 100644
--- a/src/media/video/video_rtp_session.cpp
+++ b/src/media/video/video_rtp_session.cpp
@@ -154,12 +154,10 @@ void VideoRtpSession::startReceiver()
new VideoReceiveThread(callID_, receive_.receiving_sdp, isReset, mtu_)
);
- /* ebail: keyframe requests can lead to timeout if they are not answered.
- * we decided so to disable them for the moment
+ // XXX keyframe requests can timeout if unanswered
receiveThread_->setRequestKeyFrameCallback(&SIPVoIPLink::enqueueKeyframeRequest);
- */
receiverRestartThread_.start();
- receiveThread_->addIOContext(*socketPair_);
+ receiveThread_->addIOContext(socketPair_);
receiveThread_->startLoop();
} else {
RING_DBG("Video receiving disabled");
diff --git a/src/media/video/video_sender.cpp b/src/media/video/video_sender.cpp
index c8246c29f4..b0c3bae157 100644
--- a/src/media/video/video_sender.cpp
+++ b/src/media/video/video_sender.cpp
@@ -61,7 +61,7 @@ VideoSender::~VideoSender()
void
VideoSender::encodeAndSendVideo(VideoFrame& input_frame)
{
- bool is_keyframe = forceKeyFrame_ > 0 \
+ bool is_keyframe = forceKeyFrame_ > 0
or (keyFrameFreq_ > 0 and (frameNumber_ % keyFrameFreq_) == 0);
if (is_keyframe)
@@ -84,7 +84,7 @@ VideoSender::update(Observable<std::shared_ptr<VideoFrame>>* /*obs*/,
void
VideoSender::forceKeyFrame()
{
- RING_DBG("keyframe requested");
+ RING_DBG("Peer has requested a key frame");
++forceKeyFrame_;
}
diff --git a/src/media/video/video_sender.h b/src/media/video/video_sender.h
index 42562e6c0b..01419a65d1 100644
--- a/src/media/video/video_sender.h
+++ b/src/media/video/video_sender.h
@@ -64,7 +64,7 @@ public:
private:
static constexpr int KEYFRAMES_AT_START {4}; // Number of keyframes to enforce at stream startup
- static constexpr unsigned KEY_FRAME_PERIOD {5}; // seconds before forcing a keyframe
+ static constexpr unsigned KEY_FRAME_PERIOD {0}; // seconds before forcing a keyframe
NON_COPYABLE(VideoSender);
@@ -74,6 +74,7 @@ private:
std::unique_ptr<MediaIOHandle> muxContext_ = nullptr;
std::unique_ptr<MediaEncoder> videoEncoder_ = nullptr;
+ // XXX forceKeyFrame_ is always at -1, incremented to 0 when a keyframe is requested (still works though)
std::atomic<int> forceKeyFrame_ {KEYFRAMES_AT_START};
int keyFrameFreq_ {0}; // Set keyframe rate, 0 to disable auto-keyframe. Computed in constructor
int64_t frameNumber_ = 0;
--
GitLab