Skip to content

Commit

Permalink
Fix slideshow keyframe detection (#825)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lodoyun
lodoyun authored Mar 27, 2017
1 parent a9d71e9 commit e04984d
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 89 deletions.
129 changes: 61 additions & 68 deletions erizo/src/erizo/rtp/RtpSlideShowHandler.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
#include "./MediaDefinitions.h"
#include "rtp/RtpSlideShowHandler.h"
#include "rtp/RtpUtils.h"

namespace erizo {

DEFINE_LOGGER(RtpSlideShowHandler, "rtp.RtpSlideShowHandler");

RtpSlideShowHandler::RtpSlideShowHandler()
: connection_{nullptr},
slideshow_seq_num_{-1}, last_original_seq_num_{-1}, seq_num_offset_{0},
: connection_{nullptr}, highest_seq_num_initialized_{false},
highest_seq_num_ {0},
slideshow_is_active_{false},
sending_keyframe_ {false} {}
current_keyframe_timestamp_{0} {}


void RtpSlideShowHandler::enable() {
Expand All @@ -32,48 +33,51 @@ void RtpSlideShowHandler::read(Context *ctx, std::shared_ptr<dataPacket> packet)
ctx->fireRead(packet);
return;
}
if (seq_num_offset_ > 0) {
char* buf = packet->data;
char* report_pointer = buf;
int rtcp_length = 0;
int total_length = 0;
do {
report_pointer += rtcp_length;
chead = reinterpret_cast<RtcpHeader*>(report_pointer);
rtcp_length = (ntohs(chead->length) + 1) * 4;
total_length += rtcp_length;
switch (chead->packettype) {
case RTCP_Receiver_PT:
if ((chead->getHighestSeqnum() + seq_num_offset_) < chead->getHighestSeqnum()) {
// The seqNo adjustment causes a wraparound, add to cycles
chead->setSeqnumCycles(chead->getSeqnumCycles() + 1);
RtpUtils::forEachRRBlock(packet, [this](RtcpHeader *chead) {
switch (chead->packettype) {
case RTCP_Receiver_PT:
{
uint16_t incoming_seq_num = chead->getHighestSeqnum();
ELOG_DEBUG("Received RR, highest %u", incoming_seq_num);
SequenceNumber input_seq_num = translator_.reverse(incoming_seq_num);
if (input_seq_num.type != SequenceNumberType::Valid) {
break;
}
if (RtpUtils::sequenceNumberLessThan(input_seq_num.input, incoming_seq_num)) {
chead->setSeqnumCycles(chead->getSeqnumCycles() - 1);
}
chead->setHighestSeqnum(chead->getHighestSeqnum() + seq_num_offset_);

chead->setHighestSeqnum(input_seq_num.input);
ELOG_DEBUG("Setting highest %u, output %u", input_seq_num.input, input_seq_num.output);
break;
case RTCP_RTP_Feedback_PT:
chead->setNackPid(chead->getNackPid() + seq_num_offset_);
break;
default:
}
case RTCP_RTP_Feedback_PT:
{
ELOG_DEBUG("Received NACK, PID %u", chead->getNackPid());
SequenceNumber input_seq_num = translator_.reverse(chead->getNackPid());
if (input_seq_num.type == SequenceNumberType::Valid) {
chead->setNackPid(input_seq_num.input);
ELOG_DEBUG("Setting PID %u, output %u", input_seq_num.input, input_seq_num.output);
}
break;
}
} while (total_length < packet->length);
}
}
default:
break;
}
});
ctx->fireRead(packet);
}

void RtpSlideShowHandler::write(Context *ctx, std::shared_ptr<dataPacket> packet) {
RtpHeader *rtp_header = reinterpret_cast<RtpHeader*>(packet->data);
RtcpHeader *rtcp_header = reinterpret_cast<RtcpHeader*>(packet->data);

if (packet->type != VIDEO_PACKET || rtcp_header->isRtcp()) {
ctx->fireWrite(packet);
return;
}
last_original_seq_num_ = rtp_header->getSeqNumber();
if (slideshow_seq_num_ == -1) { // We didn't receive any packets before setting up slideshow
slideshow_seq_num_ = last_original_seq_num_;
}
bool should_skip_packet = false;

uint16_t packet_seq_num = rtp_header->getSeqNumber();
if (slideshow_is_active_) {
bool is_keyframe = false;
RtpMap *codec = connection_->getRemoteSdpInfo().getCodecByExternalPayloadType(rtp_header->getPayloadType());
Expand All @@ -82,43 +86,40 @@ void RtpSlideShowHandler::write(Context *ctx, std::shared_ptr<dataPacket> packet
} else if (codec && codec->encoding_name == "VP9") {
is_keyframe = isVP9Keyframe(packet);
}
if (is_keyframe) {
setPacketSeqNumber(packet, slideshow_seq_num_++);
ctx->fireWrite(packet);
}
} else {
if (seq_num_offset_ > 0) {
setPacketSeqNumber(packet, (last_original_seq_num_ - seq_num_offset_));
}
should_skip_packet = !is_keyframe;
}
maybeUpdateHighestSeqNum(rtp_header->getSeqNumber());
SequenceNumber sequence_number_info = translator_.get(packet_seq_num, should_skip_packet);
if (!should_skip_packet && sequence_number_info.type == SequenceNumberType::Valid) {
rtp_header->setSeqNumber(sequence_number_info.output);
ELOG_DEBUG("Writing packet incoming seq %u, output %u", packet_seq_num, sequence_number_info.output);
ctx->fireWrite(packet);
} else {
ELOG_DEBUG("Skipped packet %u", packet_seq_num);
}
}

bool RtpSlideShowHandler::isVP8Keyframe(std::shared_ptr<dataPacket> packet) {
bool is_keyframe = false;
RtpHeader *rtp_header = reinterpret_cast<RtpHeader*>(packet->data);
unsigned char* start_buffer = reinterpret_cast<unsigned char*> (packet->data);
start_buffer = start_buffer + rtp_header->getHeaderLength();
RTPPayloadVP8* payload = vp8_parser_.parseVP8(
start_buffer, packet->length - rtp_header->getHeaderLength());
if (!payload->frameType) { // Its a keyframe first packet
sending_keyframe_ = true;
}
delete payload;
is_keyframe = sending_keyframe_;
if (sending_keyframe_ && rtp_header->getMarker()) {
sending_keyframe_ = false;
uint16_t seq_num = rtp_header->getSeqNumber();
uint32_t timestamp = rtp_header->getTimestamp();

if (packet->is_keyframe &&
(RtpUtils::sequenceNumberLessThan(highest_seq_num_, seq_num) || !highest_seq_num_initialized_)) {
is_keyframe = true;
current_keyframe_timestamp_ = timestamp;
} else if (timestamp == current_keyframe_timestamp_) {
is_keyframe = true;
}

ELOG_DEBUG("packet is_keyframe %d, packet timestamp %u, current_keyframe timestamp %u, result %d",
packet->is_keyframe, rtp_header->getTimestamp(), current_keyframe_timestamp_, is_keyframe);
return is_keyframe;
}

bool RtpSlideShowHandler::isVP9Keyframe(std::shared_ptr<dataPacket> packet) {
RtpHeader *rtp_header = reinterpret_cast<RtpHeader*>(packet->data);
unsigned char* start_buffer = reinterpret_cast<unsigned char*> (packet->data);
start_buffer = start_buffer + rtp_header->getHeaderLength();
RTPPayloadVP9* payload = vp9_parser_.parseVP9(
start_buffer, packet->length - rtp_header->getHeaderLength());
return !payload->frameType;
return packet->is_keyframe;
}

void RtpSlideShowHandler::setSlideShowMode(bool active) {
Expand All @@ -127,27 +128,19 @@ void RtpSlideShowHandler::setSlideShowMode(bool active) {
}

if (active) {
slideshow_seq_num_ = last_original_seq_num_ - seq_num_offset_;
sending_keyframe_ = false;
slideshow_is_active_ = true;
connection_->setFeedbackReports(false, 0);
ELOG_DEBUG("%s message: Setting seqNo, seqNo: %u", connection_->toLog(), slideshow_seq_num_);
} else {
seq_num_offset_ = last_original_seq_num_ - slideshow_seq_num_ + 1;
ELOG_DEBUG("%s message: Changing offset manually, original_seq_num: %u, slideshow_seq_num: %u, offset: %u",
connection_->toLog(), last_original_seq_num_, slideshow_seq_num_, seq_num_offset_);
slideshow_is_active_ = false;
connection_->setFeedbackReports(true, 0);
}
}

inline void RtpSlideShowHandler::setPacketSeqNumber(std::shared_ptr<dataPacket> packet, uint16_t seq_number) {
RtpHeader *head = reinterpret_cast<RtpHeader*> (packet->data);
RtcpHeader *chead = reinterpret_cast<RtcpHeader*> (packet->data);
if (chead->isRtcp()) {
return;
void RtpSlideShowHandler::maybeUpdateHighestSeqNum(uint16_t seq_num) {
if (RtpUtils::sequenceNumberLessThan(highest_seq_num_, seq_num) || !highest_seq_num_initialized_) {
highest_seq_num_ = seq_num;
highest_seq_num_initialized_ = true;
}
head->setSeqNumber(seq_number);
}

} // namespace erizo
14 changes: 7 additions & 7 deletions erizo/src/erizo/rtp/RtpSlideShowHandler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "pipeline/Handler.h"
#include "./logger.h"
#include "./WebRtcConnection.h"
#include "rtp/SequenceNumberTranslator.h"
#include "rtp/RtpVP8Parser.h"
#include "rtp/RtpVP9Parser.h"

Expand All @@ -32,17 +33,16 @@ class RtpSlideShowHandler : public Handler {
private:
bool isVP8Keyframe(std::shared_ptr<dataPacket> packet);
bool isVP9Keyframe(std::shared_ptr<dataPacket> packet);
void maybeUpdateHighestSeqNum(uint16_t seq_num);

private:
WebRtcConnection* connection_;
int32_t slideshow_seq_num_, last_original_seq_num_;
uint16_t seq_num_offset_;
SequenceNumberTranslator translator_;
bool highest_seq_num_initialized_;
uint16_t highest_seq_num_;

bool slideshow_is_active_, sending_keyframe_;
RtpVP8Parser vp8_parser_;
RtpVP9Parser vp9_parser_;

inline void setPacketSeqNumber(std::shared_ptr<dataPacket> packet, uint16_t seq_number);
bool slideshow_is_active_;
uint32_t current_keyframe_timestamp_;
};
} // namespace erizo

Expand Down
2 changes: 1 addition & 1 deletion erizo/src/test/log4cxx.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ log4j.logger.rtp.RtpPacketQueue=ERROR
log4j.logger.rtp.RtpRetransmissionHandler=ERROR
log4j.logger.rtp.RtpVP8Fragmenter=ERROR
log4j.logger.rtp.RtpVP8Parser=ERROR
log4j.logger.rtp.RtpVP8SlideShowHandler=ERROR
log4j.logger.rtp.RtpSlideShowHandler=ERROR
log4j.logger.rtp.RtpAudioMuteHandler=ERROR
log4j.logger.rtp.RtpSink=ERROR
log4j.logger.rtp.RtpSource=ERROR
Expand Down
42 changes: 30 additions & 12 deletions erizo/src/test/rtp/RtpSlideShowHandlerTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ using erizo::OutboundHandler;
using erizo::Worker;
using std::queue;

const uint32_t kArbitraryTimestamp = 1000;

class RtpSlideShowHandlerTest : public erizo::HandlerTest {
public:
RtpSlideShowHandlerTest() {
Expand Down Expand Up @@ -95,13 +97,13 @@ TEST_F(RtpSlideShowHandlerTest, shouldTransmitJustKeyframesInVP9) {
packet_queue.pop();
}
}
TEST_F(RtpSlideShowHandlerTest, shouldTransmitFromBeginningOfKFrameToMarkerPacketsWhenActive) {
TEST_F(RtpSlideShowHandlerTest, shouldTransmitFromBeginningOfKFrameToTimestampChange) {
uint16_t seq_number = erizo::kArbitrarySeqNumber;
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, true, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, true));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number, kArbitraryTimestamp - 1, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp, true, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp, false, true));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp + 1, false, false));
slideshow_handler->setSlideShowMode(true);

EXPECT_CALL(*writer.get(), write(_, _)).Times(3);
Expand All @@ -111,6 +113,22 @@ TEST_F(RtpSlideShowHandlerTest, shouldTransmitFromBeginningOfKFrameToMarkerPacke
}
}

TEST_F(RtpSlideShowHandlerTest, shouldIgnoreKeyframeIfIsKeyframeIsNotFirstPacket) {
uint16_t seq_number = erizo::kArbitrarySeqNumber;
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number, kArbitraryTimestamp - 1, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number + 2, kArbitraryTimestamp, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number + 1, kArbitraryTimestamp, true, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp, false, true));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp + 1, false, false));
slideshow_handler->setSlideShowMode(true);

EXPECT_CALL(*writer.get(), write(_, _)).Times(0);
while (!packet_queue.empty()) {
pipeline->write(packet_queue.front());
packet_queue.pop();
}
}

TEST_F(RtpSlideShowHandlerTest, shouldMantainSequenceNumberInSlideShow) {
uint16_t seq_number = erizo::kArbitrarySeqNumber;
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number, true, false));
Expand Down Expand Up @@ -157,17 +175,17 @@ TEST_F(RtpSlideShowHandlerTest, shouldAdjustSequenceNumberAfterSlideShow) {

uint16_t seq_number = erizo::kArbitrarySeqNumber;
uint16_t packets_after_handler = 0;
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number, true, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(seq_number, kArbitraryTimestamp, true, false));
packets_after_handler++;
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, true));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp, false, true));
packets_after_handler++;

packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp + 1, false, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp + 2, false, false));

packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, true, false));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp + 3, true, false));
packets_after_handler++;
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, false, true));
packet_queue.push(erizo::PacketTools::createVP8Packet(++seq_number, kArbitraryTimestamp + 3, false, true));
packets_after_handler++;

slideshow_handler->setSlideShowMode(true);
Expand Down
27 changes: 26 additions & 1 deletion erizo/src/test/utils/Tools.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,31 @@ class PacketTools {
return packet;
}

static std::shared_ptr<dataPacket> createVP8Packet(uint16_t seq_number, uint32_t timestamp,
bool is_keyframe, bool is_marker) {
erizo::RtpHeader *header = new erizo::RtpHeader();
header->setPayloadType(96);
header->setSeqNumber(seq_number);
header->setSSRC(kVideoSsrc);
header->setTimestamp(timestamp);
header->setMarker(is_marker);
char packet_buffer[200];
memset(packet_buffer, 0, 200);
char* data_pointer;
char* parsing_pointer;
memcpy(packet_buffer, reinterpret_cast<char*>(header), header->getHeaderLength());
data_pointer = packet_buffer + header->getHeaderLength();
parsing_pointer = data_pointer;

*parsing_pointer = 0x10;
parsing_pointer++;
*parsing_pointer = is_keyframe? 0x00: 0x01;

auto packet = std::make_shared<dataPacket>(0, packet_buffer, 200, VIDEO_PACKET);
packet->is_keyframe = is_keyframe;
return packet;
}

static std::shared_ptr<dataPacket> createVP9Packet(uint16_t seq_number, bool is_keyframe, bool is_marker) {
erizo::RtpHeader *header = new erizo::RtpHeader();
header->setPayloadType(98);
Expand Down Expand Up @@ -186,7 +211,7 @@ class BaseHandlerTest {
EXPECT_CALL(*reader, notifyUpdate()).Times(testing::AtLeast(1));
EXPECT_CALL(*writer, notifyUpdate()).Times(testing::AtLeast(1));

EXPECT_CALL(*reader, read(_,_)).Times(testing::AtLeast(0));
EXPECT_CALL(*reader, read(_, _)).Times(testing::AtLeast(0));
EXPECT_CALL(*writer, write(_, _)).Times(testing::AtLeast(0));

std::shared_ptr<erizo::WebRtcConnection> connection_ptr = std::dynamic_pointer_cast<WebRtcConnection>(connection);
Expand Down

0 comments on commit e04984d

Please sign in to comment.