Fix quality filter with timestamp switching (#782)

lynckia · Feb 24, 2017 · 82afe70 · 82afe70
1 parent dd4fe81
commit 82afe70
Show file tree

Hide file tree

Showing 5 changed files with 86 additions and 22 deletions.
diff --git a/erizo/src/erizo/rtp/QualityFilterHandler.cpp b/erizo/src/erizo/rtp/QualityFilterHandler.cpp
@@ -10,9 +10,10 @@ DEFINE_LOGGER(QualityFilterHandler, "rtp.QualityFilterHandler");
 
 QualityFilterHandler::QualityFilterHandler()
   : connection_{nullptr}, enabled_{true}, initialized_{false},
-  receiving_multiple_ssrc_{false}, target_spatial_layer_{0}, target_temporal_layer_{0},
+  receiving_multiple_ssrc_{false}, changing_spatial_layer_{false}, target_spatial_layer_{0},
+  future_spatial_layer_{-1}, target_temporal_layer_{0},
   video_sink_ssrc_{0}, video_source_ssrc_{0}, last_ssrc_received_{0},
-  max_video_bw_{0} {}
+  max_video_bw_{0}, last_timestamp_sent_{0}, timestamp_offset_{0} {}
 
 void QualityFilterHandler::enable() {
   enabled_ = true;
@@ -24,11 +25,19 @@ void QualityFilterHandler::disable() {
 
 void QualityFilterHandler::handleFeedbackPackets(std::shared_ptr<dataPacket> packet) {
   RtpUtils::forEachRRBlock(packet, [this](RtcpHeader *chead) {
+    // TODO(javier): Find a better way to terminate RTCP
+    if (chead->packettype == RTCP_Receiver_PT) {
+      chead->setFractionLost(0);
+      chead->setLostPackets(0);
+      chead->setJitter(0);
+    }
+
     RtpUtils::updateREMB(chead, max_video_bw_);
 
     RtpUtils::forEachNack(chead, [this, chead](uint16_t seq_num, uint16_t plb) {
       SequenceNumber result = translator_.reverse(seq_num);
       if (result.type == SequenceNumberType::Valid) {
+        chead->setSourceSSRC(last_ssrc_received_);
         chead->setNackPid(result.input);
       }
     });
@@ -49,23 +58,41 @@ void QualityFilterHandler::checkLayers() {
   int new_spatial_layer = quality_manager_->getSpatialLayer();
   if (new_spatial_layer != target_spatial_layer_) {
     sendPLI();
-    target_spatial_layer_ = new_spatial_layer;
+    sendPLI();
+    sendPLI();
+    future_spatial_layer_ = new_spatial_layer;
+    changing_spatial_layer_ = true;
   }
   int new_temporal_layer = quality_manager_->getTemporalLayer();
   target_temporal_layer_ = new_temporal_layer;
 }
 
-void QualityFilterHandler::checkSSRCChange(uint32_t ssrc) {
+bool QualityFilterHandler::checkSSRCChange(uint32_t ssrc) {
+  bool changed = false;
   if (last_ssrc_received_ != ssrc) {
-    translator_.reset();
+    changed = true;
   }
   last_ssrc_received_ = ssrc;
+  return changed;
 }
 
 void QualityFilterHandler::sendPLI() {
   getContext()->fireRead(RtpUtils::createPLI(video_sink_ssrc_, video_source_ssrc_));
 }
 
+void QualityFilterHandler::changeSpatialLayerOnKeyframeReceived(std::shared_ptr<dataPacket> packet) {
+  if (future_spatial_layer_ == -1) {
+    return;
+  }
+
+  if (packet->belongsToSpatialLayer(future_spatial_layer_) &&
+      packet->belongsToTemporalLayer(target_temporal_layer_) &&
+      packet->is_keyframe) {
+    target_spatial_layer_ = future_spatial_layer_;
+    future_spatial_layer_ = -1;
+  }
+}
+
 void QualityFilterHandler::write(Context *ctx, std::shared_ptr<dataPacket> packet) {
   RtcpHeader *chead = reinterpret_cast<RtcpHeader*>(packet->data);
   if (!chead->isRtcp() && enabled_ && packet->type == VIDEO_PACKET) {
@@ -80,15 +107,23 @@ void QualityFilterHandler::write(Context *ctx, std::shared_ptr<dataPacket> packe
       receiving_multiple_ssrc_ = true;
     }
 
+    changeSpatialLayerOnKeyframeReceived(packet);
+
     if (!packet->belongsToSpatialLayer(target_spatial_layer_)) {
-      if (ssrc == video_sink_ssrc_) {
+      if (!receiving_multiple_ssrc_) {
         translator_.get(sequence_number, true);
       }
       return;
     }
 
-    checkSSRCChange(ssrc);
-    rtp_header->setSSRC(video_sink_ssrc_);
+    uint32_t new_timestamp = rtp_header->getTimestamp();
+
+    if (checkSSRCChange(ssrc)) {
+      translator_.reset();
+      if (last_timestamp_sent_ > 0) {
+        timestamp_offset_ = last_timestamp_sent_ - new_timestamp + 1;
+      }
+    }
 
     if (!packet->belongsToTemporalLayer(target_temporal_layer_)) {
       translator_.get(sequence_number, true);
@@ -104,7 +139,11 @@ void QualityFilterHandler::write(Context *ctx, std::shared_ptr<dataPacket> packe
       rtp_header->setMarker(1);
     }
 
+    rtp_header->setSSRC(video_sink_ssrc_);
     rtp_header->setSeqNumber(sequence_number_info.output);
+
+    last_timestamp_sent_ = new_timestamp + timestamp_offset_;
+    rtp_header->setTimestamp(last_timestamp_sent_);
   }
 
   // TODO(javier): Handle SRs and translate Sequence Numbers?

diff --git a/erizo/src/erizo/rtp/QualityFilterHandler.h b/erizo/src/erizo/rtp/QualityFilterHandler.h
@@ -37,7 +37,8 @@ class QualityFilterHandler: public Handler, public std::enable_shared_from_this<
   void sendPLI();
   void checkLayers();
   void handleFeedbackPackets(std::shared_ptr<dataPacket> packet);
-  void checkSSRCChange(uint32_t ssrc);
+  bool checkSSRCChange(uint32_t ssrc);
+  void changeSpatialLayerOnKeyframeReceived(std::shared_ptr<dataPacket> packet);
 
  private:
   std::shared_ptr<QualityManager> quality_manager_;
@@ -46,12 +47,16 @@ class QualityFilterHandler: public Handler, public std::enable_shared_from_this<
   bool enabled_;
   bool initialized_;
   bool receiving_multiple_ssrc_;
+  bool changing_spatial_layer_;
   int target_spatial_layer_;
+  int future_spatial_layer_;
   int target_temporal_layer_;
   uint32_t video_sink_ssrc_;
   uint32_t video_source_ssrc_;
   uint32_t last_ssrc_received_;
   uint32_t max_video_bw_;
+  uint32_t last_timestamp_sent_;
+  uint32_t timestamp_offset_;
 };
 }  // namespace erizo
 

diff --git a/erizo/src/erizo/rtp/RtpUtils.cpp b/erizo/src/erizo/rtp/RtpUtils.cpp
@@ -47,6 +47,8 @@ void RtpUtils::forEachRRBlock(std::shared_ptr<dataPacket> packet, std::function<
     int total_length = 0;
     int currentBlock = 0;
 
+    f(chead);
+
     do {
       moving_buffer += rtcp_length;
       chead = reinterpret_cast<RtcpHeader*>(moving_buffer);

diff --git a/erizo/src/erizo/rtp/SequenceNumberTranslator.cpp b/erizo/src/erizo/rtp/SequenceNumberTranslator.cpp
@@ -5,8 +5,8 @@
 
 namespace erizo {
 
-constexpr uint16_t kMaxSequenceNumberInBuffer = 511;  // = 65536 / 128 and more than 10s of audio @50fps
-constexpr uint16_t kMaxDistance = 200;
+constexpr uint16_t kMaxSequenceNumberInBuffer = 1023;  // = 65536 / 128 and more than 10s of audio @50fps
+constexpr uint16_t kMaxDistance = 500;
 
 DEFINE_LOGGER(SequenceNumberTranslator, "rtp.SequenceNumberTranslator");
 
@@ -26,14 +26,17 @@ void SequenceNumberTranslator::add(SequenceNumber sequence_number) {
 
 uint16_t SequenceNumberTranslator::fill(const uint16_t &first,
                                         const uint16_t &last) {
-  const SequenceNumber& first_sequence_number = get(first);
-  uint16_t output_sequence_number = first_sequence_number.output;
+  if (last < first) {
+    fill(first, 65535);
+    fill(0, last);
+  }
+  const SequenceNumber& previous_sequence_number = get(first - 1);
+  uint16_t output_sequence_number = previous_sequence_number.output;
 
-  if (first_sequence_number.type != SequenceNumberType::Skip) {
+  if (previous_sequence_number.type != SequenceNumberType::Skip) {
     output_sequence_number++;
   }
-
-  for (uint16_t sequence_number = first_sequence_number.input + 1;
+  for (uint16_t sequence_number = first;
        RtpUtils::sequenceNumberLessThan(sequence_number, last);
        sequence_number++) {
     add(SequenceNumber{sequence_number, output_sequence_number++, SequenceNumberType::Valid});
@@ -46,8 +49,16 @@ SequenceNumber& SequenceNumberTranslator::internalGet(uint16_t input_sequence_nu
   return in_out_buffer_[input_sequence_number % kMaxSequenceNumberInBuffer];
 }
 
+SequenceNumber& SequenceNumberTranslator::internalReverse(uint16_t output_sequence_number) {
+  return out_in_buffer_[output_sequence_number % kMaxSequenceNumberInBuffer];
+}
+
 SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number) const {
-  return in_out_buffer_[input_sequence_number % kMaxSequenceNumberInBuffer];
+  const SequenceNumber &result = in_out_buffer_[input_sequence_number % kMaxSequenceNumberInBuffer];
+  if (result.input == input_sequence_number) {
+    return result;
+  }
+  return SequenceNumber{0, 0, SequenceNumberType::Skip};
 }
 
 SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number, bool skip) {
@@ -67,20 +78,22 @@ SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number, boo
   }
 
   if (RtpUtils::sequenceNumberLessThan(last_input_sequence_number_, input_sequence_number)) {
-    uint16_t output_sequence_number = fill(last_input_sequence_number_, input_sequence_number);
+    uint16_t output_sequence_number = fill(last_input_sequence_number_ + 1, input_sequence_number);
 
     SequenceNumberType type = skip ? SequenceNumberType::Skip : SequenceNumberType::Valid;
 
     add(SequenceNumber{input_sequence_number, output_sequence_number, type});
     last_input_sequence_number_ = input_sequence_number;
-    if (last_input_sequence_number_ - first_input_sequence_number_ > kMaxDistance) {
+    if (last_input_sequence_number_ - kMaxDistance > first_input_sequence_number_) {
       first_input_sequence_number_ = last_input_sequence_number_ - kMaxDistance;
     }
-    return get(input_sequence_number);
+    SequenceNumber info = get(input_sequence_number);
+    return info;
   } else {
     SequenceNumber& result = internalGet(input_sequence_number);
     if (result.type == SequenceNumberType::Valid) {
       SequenceNumberType type = skip ? SequenceNumberType::Discard : SequenceNumberType::Valid;
+      internalReverse(result.output).type = type;
       result.type = type;
     }
     return result;
@@ -89,17 +102,21 @@ SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number, boo
 
 SequenceNumber SequenceNumberTranslator::reverse(uint16_t output_sequence_number) const {
   SequenceNumber result = out_in_buffer_[output_sequence_number % kMaxSequenceNumberInBuffer];
-  if (RtpUtils::sequenceNumberLessThan(result.input, first_input_sequence_number_) ||
+  if (result.output != output_sequence_number ||
+      RtpUtils::sequenceNumberLessThan(result.input, first_input_sequence_number_) ||
       RtpUtils::sequenceNumberLessThan(last_input_sequence_number_, result.input)) {
     return SequenceNumber{0, output_sequence_number, SequenceNumberType::Discard};
   }
   return result;
 }
 
 void SequenceNumberTranslator::reset() {
+  if (!initialized_) {
+    return;
+  }
   initialized_ = false;
   SequenceNumber &last = internalGet(last_input_sequence_number_);
-  initial_output_sequence_number_ = last.output;
+  initial_output_sequence_number_ = last.output + 1;
   first_input_sequence_number_ = 0;
   last_input_sequence_number_ = 0;
   reset_ = true;

diff --git a/erizo/src/erizo/rtp/SequenceNumberTranslator.h b/erizo/src/erizo/rtp/SequenceNumberTranslator.h
@@ -40,6 +40,7 @@ class SequenceNumberTranslator: public Service, public std::enable_shared_from_t
   void add(SequenceNumber sequence_number);
   uint16_t fill(const uint16_t &first, const uint16_t &last);
   SequenceNumber& internalGet(uint16_t input_sequence_number);
+  SequenceNumber& internalReverse(uint16_t output_sequence_number);
 
  private:
   std::vector<SequenceNumber> in_out_buffer_;