Skip to content

Commit 9878707

Browse files
jcaguejcague
authored
Fix wrong reset in SeqNum Translator (lynckia#817)
1 parent 1bce5b3 commit 9878707

File tree

5 files changed

+94
-25
lines changed

5 files changed

+94
-25
lines changed

‎erizo/src/erizo/rtp/QualityFilterHandler.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,5 +205,6 @@ void QualityFilterHandler::notifyUpdate() {
205205

206206
video_sink_ssrc_ = connection_->getVideoSinkSSRC();
207207
video_source_ssrc_ = connection_->getVideoSourceSSRC();
208+
initialized_ = true;
208209
}
209210
} // namespace erizo

‎erizo/src/erizo/rtp/RtpPaddingGeneratorHandler.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ DEFINE_LOGGER(RtpPaddingGeneratorHandler, "rtp.RtpPaddingGeneratorHandler");
1414
constexpr duration kStatsPeriod = std::chrono::milliseconds(100);
1515
constexpr uint8_t kMaxPaddingSize = 255;
1616
constexpr uint64_t kMinMarkerRate = 3;
17-
constexpr uint8_t kMaxFractionLostAllowed = .05 * 255; // 5% of packet losts
1817
constexpr uint64_t kInitialBitrate = 300000;
1918

2019
RtpPaddingGeneratorHandler::RtpPaddingGeneratorHandler(std::shared_ptr<erizo::Clock> the_clock) :

‎erizo/src/erizo/rtp/SequenceNumberTranslator.cpp

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ SequenceNumberTranslator::SequenceNumberTranslator()
1515
out_in_buffer_{kMaxSequenceNumberInBuffer},
1616
first_input_sequence_number_{0},
1717
last_input_sequence_number_{0},
18-
initial_output_sequence_number_{0},
18+
last_output_sequence_number_{0},
1919
offset_{0},
2020
initialized_{false}, reset_{false} {
2121
}
@@ -63,29 +63,33 @@ SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number) con
6363
}
6464

6565
SequenceNumber SequenceNumberTranslator::generate() {
66-
uint16_t output_sequence_number = offset_++;
67-
if (initialized_) {
68-
SequenceNumber &last = internalGet(last_input_sequence_number_);
69-
output_sequence_number = last.output + offset_;
70-
} else {
71-
initial_output_sequence_number_ = offset_;
72-
}
66+
uint16_t output_sequence_number = ++offset_ + last_output_sequence_number_;
7367
SequenceNumber sequence_number{0, output_sequence_number, SequenceNumberType::Generated};
7468
out_in_buffer_[sequence_number.output % kMaxSequenceNumberInBuffer] = sequence_number;
7569

7670
return sequence_number;
7771
}
7872

73+
void SequenceNumberTranslator::updateLastOutputSequenceNumber(bool skip, uint16_t output_sequence_number) {
74+
if (!skip && RtpUtils::sequenceNumberLessThan(last_output_sequence_number_, output_sequence_number)) {
75+
last_output_sequence_number_ = output_sequence_number;
76+
}
77+
}
78+
7979
SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number, bool skip) {
8080
if (!initialized_) {
8181
SequenceNumberType type = skip ? SequenceNumberType::Skip : SequenceNumberType::Valid;
82-
uint16_t output_sequence_number = (reset_ || offset_ > 0) ? initial_output_sequence_number_ : input_sequence_number;
82+
uint16_t output_sequence_number = (reset_ || offset_ > 0) ?
83+
last_output_sequence_number_ + offset_ + 1 : input_sequence_number;
8384
add(SequenceNumber{input_sequence_number, output_sequence_number, type});
84-
last_input_sequence_number_ = input_sequence_number;
85-
first_input_sequence_number_ = input_sequence_number;
86-
initialized_ = true;
87-
reset_ = false;
88-
offset_ = 0;
85+
updateLastOutputSequenceNumber(skip, output_sequence_number);
86+
if (!skip) {
87+
last_input_sequence_number_ = input_sequence_number;
88+
first_input_sequence_number_ = input_sequence_number;
89+
initialized_ = true;
90+
reset_ = false;
91+
offset_ = 0;
92+
}
8993
return get(input_sequence_number);
9094
}
9195

@@ -98,6 +102,7 @@ SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number, boo
98102

99103
SequenceNumberType type = skip ? SequenceNumberType::Skip : SequenceNumberType::Valid;
100104

105+
updateLastOutputSequenceNumber(skip, output_sequence_number);
101106
add(SequenceNumber{input_sequence_number, output_sequence_number, type});
102107
last_input_sequence_number_ = input_sequence_number;
103108
if (RtpUtils::sequenceNumberLessThan(first_input_sequence_number_, last_input_sequence_number_ - kMaxDistance)) {
@@ -113,6 +118,7 @@ SequenceNumber SequenceNumberTranslator::get(uint16_t input_sequence_number, boo
113118
internalReverse(result.output).type = type;
114119
result.type = type;
115120
}
121+
updateLastOutputSequenceNumber(skip, result.output);
116122
return result;
117123
}
118124
}
@@ -132,11 +138,8 @@ void SequenceNumberTranslator::reset() {
132138
return;
133139
}
134140
initialized_ = false;
135-
SequenceNumber &last = internalGet(last_input_sequence_number_);
136-
initial_output_sequence_number_ = last.output + 1;
137141
first_input_sequence_number_ = 0;
138142
last_input_sequence_number_ = 0;
139-
reset_ = true;
140143
in_out_buffer_ = std::vector<SequenceNumber>(kMaxSequenceNumberInBuffer);
141144
out_in_buffer_ = std::vector<SequenceNumber>(kMaxSequenceNumberInBuffer);
142145
}

‎erizo/src/erizo/rtp/SequenceNumberTranslator.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,14 @@ class SequenceNumberTranslator: public Service, public std::enable_shared_from_t
4343
uint16_t fill(const uint16_t &first, const uint16_t &last);
4444
SequenceNumber& internalGet(uint16_t input_sequence_number);
4545
SequenceNumber& internalReverse(uint16_t output_sequence_number);
46+
void updateLastOutputSequenceNumber(bool skip, uint16_t output_sequence_number);
4647

4748
private:
4849
std::vector<SequenceNumber> in_out_buffer_;
4950
std::vector<SequenceNumber> out_in_buffer_;
5051
uint16_t first_input_sequence_number_;
5152
uint16_t last_input_sequence_number_;
52-
uint16_t initial_output_sequence_number_;
53+
uint16_t last_output_sequence_number_;
5354
uint16_t offset_;
5455
bool initialized_;
5556
bool reset_;

‎erizo/src/test/rtp/SequenceNumberTranslatorTest.cpp

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ using erizo::SequenceNumberType;
2626
enum class PacketState {
2727
Forward = 0,
2828
Skip = 1,
29-
Generate = 2
29+
Generate = 2,
30+
Reset = 3
3031
};
3132

3233
struct Packet {
@@ -56,7 +57,10 @@ TEST_P(SequenceNumberTranslatorTest, shouldReturnRightOutputSequenceNumbers) {
5657
for (Packet packet : queue) {
5758
bool skip = packet.state == PacketState::Skip;
5859
SequenceNumber output;
59-
if (packet.state == PacketState::Generate) {
60+
if (packet.state == PacketState::Reset) {
61+
translator.reset();
62+
return;
63+
} else if (packet.state == PacketState::Generate) {
6064
output = translator.generate();
6165
} else {
6266
output = translator.get(packet.sequence_number, skip);
@@ -144,13 +148,74 @@ INSTANTIATE_TEST_CASE_P(
144148
{ 0, PacketState::Skip, 0, SequenceNumberType::Discard},
145149
{ 2, PacketState::Forward, 2, SequenceNumberType::Valid}}),
146150

151+
// Reset after having received skipped packets
152+
std::vector<Packet>({{ 5059, PacketState::Skip, 5059, SequenceNumberType::Skip},
153+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
154+
{ 1032, PacketState::Skip, 1032, SequenceNumberType::Skip},
155+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
156+
{ 23537, PacketState::Forward, 23537, SequenceNumberType::Valid}}),
157+
158+
159+
// Reset after having received skipped packets
160+
std::vector<Packet>({{ 5059, PacketState::Skip, 5059, SequenceNumberType::Skip},
161+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
162+
{ 23537, PacketState::Forward, 23537, SequenceNumberType::Valid}}),
163+
164+
// Reset after having received skipped packets
165+
std::vector<Packet>({{ 5058, PacketState::Forward, 5058, SequenceNumberType::Valid},
166+
{ 5059, PacketState::Skip, 5059, SequenceNumberType::Skip},
167+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
168+
{ 23537, PacketState::Forward, 5059, SequenceNumberType::Valid}}),
169+
170+
// input expected_output
171+
std::vector<Packet>({{ 0, PacketState::Generate, 1, SequenceNumberType::Generated},
172+
{ 6, PacketState::Forward, 2, SequenceNumberType::Valid},
173+
{ 7, PacketState::Forward, 3, SequenceNumberType::Valid},
174+
{ 8, PacketState::Forward, 4, SequenceNumberType::Valid}}),
175+
176+
// input expected_output
177+
std::vector<Packet>({{ 6, PacketState::Forward, 6, SequenceNumberType::Valid},
178+
{ 10, PacketState::Skip, 10, SequenceNumberType::Skip},
179+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
180+
{ 301, PacketState::Skip, 301, SequenceNumberType::Skip},
181+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
182+
{ 901, PacketState::Forward, 7, SequenceNumberType::Valid}}),
147183

148184
// input expected_output
149-
std::vector<Packet>({{ 0, PacketState::Generate, 0, SequenceNumberType::Generated},
150-
{ 6, PacketState::Forward, 1, SequenceNumberType::Valid},
151-
{ 7, PacketState::Forward, 2, SequenceNumberType::Valid},
152-
{ 8, PacketState::Forward, 3, SequenceNumberType::Valid}}),
185+
std::vector<Packet>({{ 6, PacketState::Forward, 6, SequenceNumberType::Valid},
186+
{ 10, PacketState::Skip, 10, SequenceNumberType::Skip},
187+
{ 9, PacketState::Forward, 9, SequenceNumberType::Valid},
188+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
189+
{ 301, PacketState::Skip, 301, SequenceNumberType::Skip},
190+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
191+
{ 901, PacketState::Forward, 10, SequenceNumberType::Valid}}),
153192

193+
// input expected_output
194+
std::vector<Packet>({{ 6, PacketState::Forward, 6, SequenceNumberType::Valid},
195+
{ 10, PacketState::Skip, 10, SequenceNumberType::Skip},
196+
{ 0, PacketState::Generate, 7, SequenceNumberType::Generated},
197+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
198+
{ 301, PacketState::Skip, 301, SequenceNumberType::Skip},
199+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
200+
{ 901, PacketState::Forward, 8, SequenceNumberType::Valid}}),
201+
202+
// input expected_output
203+
std::vector<Packet>({{ 5059, PacketState::Skip, 5059, SequenceNumberType::Skip},
204+
{ 30, PacketState::Forward, 30, SequenceNumberType::Valid},
205+
{ 0, PacketState::Generate, 31, SequenceNumberType::Generated},
206+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
207+
{ 0, PacketState::Generate, 32, SequenceNumberType::Generated},
208+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
209+
{ 6, PacketState::Forward, 34, SequenceNumberType::Valid}}),
210+
211+
// input expected_output
212+
std::vector<Packet>({{ 5059, PacketState::Skip, 5059, SequenceNumberType::Skip},
213+
{ 30, PacketState::Forward, 30, SequenceNumberType::Valid},
214+
{ 0, PacketState::Generate, 31, SequenceNumberType::Generated},
215+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
216+
{ 0, PacketState::Generate, 32, SequenceNumberType::Generated},
217+
{ 0, PacketState::Reset, 0, SequenceNumberType::Skip},
218+
{ 6, PacketState::Forward, 34, SequenceNumberType::Valid}}),
154219

155220
// input expected_output
156221
std::vector<Packet>({{ 5, PacketState::Forward, 5, SequenceNumberType::Valid},

0 commit comments

Comments
 (0)