Skip to content

Commit f571c6f

Browse files
afrindmeta-codesync[bot]
afrind
authored and
meta-codesync[bot]
committed
Clean up MoQTestClient verification
Summary: When using two subgroups, the next expected object is really per subgroup, since that data can arrive out of order. Verification of received track data really needs a major overhaul, in particular because stream-per-object and datagram can have reordering, etc. Not engaging in that now. Reviewed By: sharmafb Differential Revision: D86428304 fbshipit-source-id: a4b6cc1ba975cd9e78435f88c1216ec89ed9259c
1 parent 6fc76c1 commit f571c6f

2 files changed

Lines changed: 81 additions & 32 deletions

File tree

‎moxygen/moqtest/MoQTestClient.cpp‎

Lines changed: 76 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,12 @@ ObjectReceiverCallback::FlowControlState MoQTestClient::onObject(
157157
}
158158

159159
// Adjust the expected data (If Still recieving data, leave unblocked)
160-
return adjustExpected(params_) == AdjustedExpectedResult::STILL_RECEIVING_DATA
161-
? ObjectReceiverCallback::FlowControlState::UNBLOCKED
162-
: ObjectReceiverCallback::FlowControlState::BLOCKED;
160+
auto result = adjustExpected(params_, &objHeader);
161+
if (result == AdjustedExpectedResult::STILL_RECEIVING_DATA) {
162+
return ObjectReceiverCallback::FlowControlState::UNBLOCKED;
163+
} else {
164+
return ObjectReceiverCallback::FlowControlState::BLOCKED;
165+
}
163166
}
164167

165168
void MoQTestClient::onObjectStatus(
@@ -190,7 +193,8 @@ void MoQTestClient::onObjectStatus(
190193
}
191194

192195
// Adjust the expected data
193-
if (adjustExpected(params_) == AdjustedExpectedResult::RECEIVED_ALL_DATA) {
196+
if (adjustExpected(params_, &objHeader) ==
197+
AdjustedExpectedResult::RECEIVED_ALL_DATA) {
194198
XLOG(DBG1)
195199
<< "MoQTest DEBUGGING: onObjectStatus: No more data to be expected";
196200
}
@@ -219,7 +223,8 @@ void MoQTestClient::onSubscribeDone(const SubscribeDone& done) {
219223
}
220224
}
221225
if (params_.forwardingPreference != ForwardingPreference::DATAGRAM &&
222-
adjustExpected(params_) == AdjustedExpectedResult::STILL_RECEIVING_DATA) {
226+
adjustExpected(params_, nullptr) ==
227+
AdjustedExpectedResult::STILL_RECEIVING_DATA) {
223228
XLOG(ERR)
224229
<< "MoQTest verification result: FAILURE! reason: SubscribeDone recieved while objects are still expected";
225230
subHandle_->unsubscribe();
@@ -235,7 +240,8 @@ bool MoQTestClient::validateSubscribedData(
235240
// Validate Group, Object Id, SubGroup (and End of Group Markers if
236241
// applicable)
237242
XLOG(DBG1) << "MoQTest DEBUGGING: Expected Group=" << expectedGroup_
238-
<< " Expected ObjectId=" << expectedObjectId_;
243+
<< " Expected ObjectId="
244+
<< subgroupToExpectedObjId_[header.subgroup];
239245
XLOG(DBG1) << "MoQTest DEBUGGING: Object Group=" << header.group
240246
<< " end of group markers=" << params_.sendEndOfGroupMarkers
241247
<< " expected end of group markers=" << expectEndOfGroup_;
@@ -247,7 +253,8 @@ bool MoQTestClient::validateSubscribedData(
247253
return false;
248254
}
249255

250-
if (params_.forwardingPreference != ForwardingPreference::DATAGRAM &&
256+
if (params_.forwardingPreference ==
257+
ForwardingPreference::ONE_SUBGROUP_PER_GROUP &&
251258
header.subgroup != expectedSubgroup_) {
252259
XLOG(ERR)
253260
<< "MoQTest verification result: FAILURE! reason: SubGroup Mismatch: Actual="
@@ -262,11 +269,35 @@ bool MoQTestClient::validateSubscribedData(
262269
}
263270
}
264271

272+
// Validate subgroup ID according to forwarding preference
273+
if ((params_.forwardingPreference ==
274+
ForwardingPreference::ONE_SUBGROUP_PER_GROUP &&
275+
header.subgroup != 0) ||
276+
(params_.forwardingPreference ==
277+
ForwardingPreference::TWO_SUBGROUPS_PER_GROUP &&
278+
header.subgroup > 1)) {
279+
XLOG(ERR)
280+
<< "MoQTest verification result: FAILURE! reason: SubGroup Mismatch: Actual="
281+
<< header.subgroup << " Expected="
282+
<< (params_.forwardingPreference ==
283+
ForwardingPreference::ONE_SUBGROUP_PER_GROUP
284+
? "0"
285+
: (params_.forwardingPreference ==
286+
ForwardingPreference::TWO_SUBGROUPS_PER_GROUP
287+
? "0 or 1"
288+
: "N/A"));
289+
return false;
290+
}
291+
265292
if (params_.forwardingPreference != ForwardingPreference::DATAGRAM &&
266-
header.id != expectedObjectId_) {
293+
params_.forwardingPreference !=
294+
ForwardingPreference::ONE_SUBGROUP_PER_OBJECT &&
295+
header.id != subgroupToExpectedObjId_[header.subgroup]) {
267296
XLOG(ERR)
268297
<< "MoQTest verification result: FAILURE! reason: Object Id Mismatch: Actual="
269-
<< header.id << " Expected=" << expectedObjectId_;
298+
<< header.id
299+
<< " Expected=" << subgroupToExpectedObjId_[header.subgroup]
300+
<< " (Subgroup=" << header.subgroup << ")";
270301
return false;
271302
}
272303

@@ -307,11 +338,11 @@ AdjustedExpectedResult MoQTestClient::adjustExpectedForOneSubgroupPerGroup(
307338
MoQTestParameters& params) {
308339
// Adjust Expected Group and ObjectId
309340
if (expectedGroup_ < params.lastGroupInTrack &&
310-
expectedObjectId_ == params.lastObjectInTrack) {
341+
subgroupToExpectedObjId_[0] == params.lastObjectInTrack) {
311342
expectedGroup_ += params.groupIncrement;
312-
expectedObjectId_ = params.startObject;
313-
} else if (expectedObjectId_ < params.lastObjectInTrack) {
314-
expectedObjectId_ += params.objectIncrement;
343+
subgroupToExpectedObjId_[0] = params.startObject;
344+
} else if (subgroupToExpectedObjId_[0] < params.lastObjectInTrack) {
345+
subgroupToExpectedObjId_[0] += params.objectIncrement;
315346
} else {
316347
return AdjustedExpectedResult::RECEIVED_ALL_DATA;
317348
}
@@ -322,34 +353,41 @@ AdjustedExpectedResult MoQTestClient::adjustExpectedForOneSubgroupPerObject(
322353
MoQTestParameters& params) {
323354
// Adjust Expected Group, ObjectId and Subgroup
324355
if (expectedGroup_ < params.lastGroupInTrack &&
325-
expectedObjectId_ == params.lastObjectInTrack) {
356+
subgroupToExpectedObjId_[0] == params.lastObjectInTrack) {
326357
// Increment Group, Reset ObjectId and Subgroup
327358
expectedGroup_ += params.groupIncrement;
328-
expectedObjectId_ = params.startObject;
359+
subgroupToExpectedObjId_[0] = params.startObject;
329360
expectedSubgroup_ = 0;
330-
} else if (expectedObjectId_ < params.lastObjectInTrack) {
361+
} else if (subgroupToExpectedObjId_[0] < params.lastObjectInTrack) {
331362
// Increment ObjectId and Subgroup
332-
expectedObjectId_ += params.objectIncrement;
333-
expectedSubgroup_++;
363+
subgroupToExpectedObjId_[0] += params.objectIncrement;
364+
expectedSubgroup_ += params.objectIncrement;
334365
} else {
335366
return AdjustedExpectedResult::RECEIVED_ALL_DATA;
336367
}
337368
return AdjustedExpectedResult::STILL_RECEIVING_DATA;
338369
}
339370

340371
AdjustedExpectedResult MoQTestClient::adjustExpectedForTwoSubgroupsPerGroup(
372+
const ObjectHeader* header,
341373
MoQTestParameters& params) {
374+
auto subgroup =
375+
header ? header->subgroup : ((params.lastObjectInTrack & 1) ? 1 : 0);
342376
// Adjust Expected Group, ObjectId and Subgroup
343377
if (expectedGroup_ < params.lastGroupInTrack &&
344-
expectedObjectId_ == params.lastObjectInTrack) {
378+
subgroupToExpectedObjId_[subgroup] >= params.lastObjectInTrack) {
345379
// Increment Group, Reset ObjectId and Subgroup
346380
expectedGroup_ += params.groupIncrement;
347-
expectedObjectId_ = params.startObject;
348-
expectedSubgroup_ = 0;
349-
} else if (expectedObjectId_ < params.lastObjectInTrack) {
350-
// Increment ObjectId, Switch Subgroup between 0 and 1
351-
expectedObjectId_ += params.objectIncrement;
352-
expectedSubgroup_ = 1 - expectedSubgroup_;
381+
subgroupToExpectedObjId_[params.startObject & 1] = params.startObject;
382+
subgroupToExpectedObjId_[!(params.startObject & 1)] =
383+
params.startObject + params.objectIncrement;
384+
} else if (subgroupToExpectedObjId_[subgroup] < params.lastObjectInTrack) {
385+
// Increment ObjectId for this subgroup. If increment is odd, increment
386+
// twice
387+
subgroupToExpectedObjId_[subgroup] += params.objectIncrement;
388+
if (params.objectIncrement % 2 == 1) {
389+
subgroupToExpectedObjId_[subgroup] += params.objectIncrement;
390+
}
353391
} else {
354392
return AdjustedExpectedResult::RECEIVED_ALL_DATA;
355393
}
@@ -360,9 +398,9 @@ AdjustedExpectedResult MoQTestClient::adjustExpectedForDatagram(
360398
MoQTestParameters& params) {
361399
// Adjust Object Count
362400
datagramObjects_++;
363-
// Only Complete if expectedGroup_ and expectedObjectId_ are at the end
401+
// Only Complete if expectedGroup_ and subgroupToExpectedObjId_ are at the end
364402
if (expectedGroup_ == params_.lastGroupInTrack &&
365-
expectedObjectId_ == params_.lastObjectInTrack) {
403+
subgroupToExpectedObjId_[0] == params_.lastObjectInTrack) {
366404
return AdjustedExpectedResult::RECEIVED_ALL_DATA;
367405
}
368406
return AdjustedExpectedResult::STILL_RECEIVING_DATA;
@@ -429,7 +467,14 @@ folly::Expected<folly::Unit, ExtensionError> MoQTestClient::validateExtensions(
429467
void MoQTestClient::initializeExpecteds(MoQTestParameters& params) {
430468
params_ = params;
431469
expectedGroup_ = params.startGroup;
432-
expectedObjectId_ = params.startObject;
470+
if (params.forwardingPreference ==
471+
ForwardingPreference::TWO_SUBGROUPS_PER_GROUP) {
472+
subgroupToExpectedObjId_[params.startObject & 1] = params.startObject;
473+
subgroupToExpectedObjId_[!(params.startObject & 1)] =
474+
params.startObject + params.objectIncrement;
475+
} else {
476+
subgroupToExpectedObjId_[0] = params.startObject;
477+
}
433478
expectedSubgroup_ = 0;
434479
expectEndOfGroup_ = params.sendEndOfGroupMarkers;
435480

@@ -438,7 +483,8 @@ void MoQTestClient::initializeExpecteds(MoQTestParameters& params) {
438483
}
439484

440485
AdjustedExpectedResult MoQTestClient::adjustExpected(
441-
MoQTestParameters& params) {
486+
MoQTestParameters& params,
487+
const ObjectHeader* header) {
442488
switch (params_.forwardingPreference) {
443489
case (ForwardingPreference::ONE_SUBGROUP_PER_GROUP): {
444490
return adjustExpectedForOneSubgroupPerGroup(params);
@@ -449,7 +495,7 @@ AdjustedExpectedResult MoQTestClient::adjustExpected(
449495
break;
450496
}
451497
case (ForwardingPreference::TWO_SUBGROUPS_PER_GROUP): {
452-
return adjustExpectedForTwoSubgroupsPerGroup(params);
498+
return adjustExpectedForTwoSubgroupsPerGroup(header, params);
453499
break;
454500
}
455501
case (ForwardingPreference::DATAGRAM): {

‎moxygen/moqtest/MoQTestClient.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ class MoQTestClient {
130130
// expected data)
131131
uint64_t expectedGroup_{};
132132
uint64_t expectedSubgroup_{};
133-
uint64_t expectedObjectId_{};
133+
std::array<uint64_t, 2> subgroupToExpectedObjId_{};
134134

135135
// Holds if current request expects end of group markers
136136
bool expectEndOfGroup_{};
@@ -152,12 +152,15 @@ class MoQTestClient {
152152
const std::vector<Extension>& extensions,
153153
MoQTestParameters* params);
154154

155-
AdjustedExpectedResult adjustExpected(MoQTestParameters& params);
155+
AdjustedExpectedResult adjustExpected(
156+
MoQTestParameters& params,
157+
const ObjectHeader* header);
156158
AdjustedExpectedResult adjustExpectedForOneSubgroupPerGroup(
157159
MoQTestParameters& params);
158160
AdjustedExpectedResult adjustExpectedForOneSubgroupPerObject(
159161
MoQTestParameters& params);
160162
AdjustedExpectedResult adjustExpectedForTwoSubgroupsPerGroup(
163+
const ObjectHeader* header,
161164
MoQTestParameters& params);
162165
AdjustedExpectedResult adjustExpectedForDatagram(MoQTestParameters& params);
163166
bool validateDatagramObjects(const ObjectHeader& header);

0 commit comments

Comments
 (0)