Skip to content

Commit cb7f0c2

Browse files
martindukecopybara-github
martinduke
authored and
copybara-github
committed
Parameterize remaining MOQT message types.
Also FETCH Group Order semantics have changed. The FETCH message dictates GroupOrder and this MUST be followed. Part of draft-16 update. PiperOrigin-RevId: 869355391
1 parent 7cf0592 commit cb7f0c2

27 files changed

+293
-965
lines changed

‎quiche/quic/moqt/moqt_framer.cc‎

Lines changed: 10 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -353,30 +353,6 @@ KeyValuePairList MessageParameters::ToKeyValuePairList() const {
353353
return list;
354354
}
355355

356-
KeyValuePairList VersionSpecificParameters::ToKeyValuePairList() const {
357-
KeyValuePairList out;
358-
if (delivery_timeout != quic::QuicTimeDelta::Infinite()) {
359-
out.insert(
360-
static_cast<uint64_t>(VersionSpecificParameter::kDeliveryTimeout),
361-
static_cast<uint64_t>(delivery_timeout.ToMilliseconds()));
362-
}
363-
for (const AuthToken& token : authorization_tokens) {
364-
out.insert(
365-
static_cast<uint64_t>(VersionSpecificParameter::kAuthorizationToken),
366-
SerializeAuthToken(token).AsStringView());
367-
}
368-
if (max_cache_duration != quic::QuicTimeDelta::Infinite()) {
369-
out.insert(
370-
static_cast<uint64_t>(VersionSpecificParameter::kMaxCacheDuration),
371-
static_cast<uint64_t>(max_cache_duration.ToMilliseconds()));
372-
}
373-
if (oack_window_size.has_value()) {
374-
out.insert(static_cast<uint64_t>(VersionSpecificParameter::kOackWindowSize),
375-
static_cast<uint64_t>(oack_window_size->ToMicroseconds()));
376-
}
377-
return out;
378-
}
379-
380356
quiche::QuicheBuffer MoqtFramer::SerializeObjectHeader(
381357
const MoqtObject& message, MoqtDataStreamType message_type,
382358
std::optional<uint64_t> previous_object_in_stream) {
@@ -649,18 +625,11 @@ quiche::QuicheBuffer MoqtFramer::SerializeFetch(const MoqtFetch& message) {
649625
return quiche::QuicheBuffer();
650626
}
651627
}
652-
KeyValuePairList parameters;
653-
if (!FillAndValidateVersionSpecificParameters(
654-
MoqtMessageType::kFetch, message.parameters, parameters)) {
655-
return quiche::QuicheBuffer();
656-
};
657628
if (std::holds_alternative<StandaloneFetch>(message.fetch)) {
658629
const StandaloneFetch& standalone_fetch =
659630
std::get<StandaloneFetch>(message.fetch);
660631
return SerializeControlMessage(
661632
MoqtMessageType::kFetch, WireVarInt62(message.request_id),
662-
WireUint8(message.subscriber_priority),
663-
WireDeliveryOrder(message.group_order),
664633
WireVarInt62(FetchType::kStandalone),
665634
WireFullTrackName(standalone_fetch.full_track_name),
666635
WireVarInt62(standalone_fetch.start_location.group),
@@ -669,7 +638,7 @@ quiche::QuicheBuffer MoqtFramer::SerializeFetch(const MoqtFetch& message) {
669638
WireVarInt62(standalone_fetch.end_location.object == kMaxObjectId
670639
? 0
671640
: standalone_fetch.end_location.object + 1),
672-
WireKeyValuePairList(parameters));
641+
WireKeyValuePairList(message.parameters.ToKeyValuePairList()));
673642
}
674643
uint64_t request_id, joining_start;
675644
if (std::holds_alternative<JoiningFetchRelative>(message.fetch)) {
@@ -685,26 +654,21 @@ quiche::QuicheBuffer MoqtFramer::SerializeFetch(const MoqtFetch& message) {
685654
}
686655
return SerializeControlMessage(
687656
MoqtMessageType::kFetch, WireVarInt62(message.request_id),
688-
WireUint8(message.subscriber_priority),
689-
WireDeliveryOrder(message.group_order),
690657
WireVarInt62(message.fetch.index() + 1), WireVarInt62(request_id),
691-
WireVarInt62(joining_start), WireKeyValuePairList(parameters));
658+
WireVarInt62(joining_start),
659+
WireKeyValuePairList(message.parameters.ToKeyValuePairList()));
692660
}
693661

694662
quiche::QuicheBuffer MoqtFramer::SerializeFetchOk(const MoqtFetchOk& message) {
695-
KeyValuePairList parameters;
696-
if (!FillAndValidateVersionSpecificParameters(
697-
MoqtMessageType::kFetchOk, message.parameters, parameters)) {
698-
return quiche::QuicheBuffer();
699-
};
700663
return SerializeControlMessage(
701664
MoqtMessageType::kFetchOk, WireVarInt62(message.request_id),
702-
WireDeliveryOrder(message.group_order), WireBoolean(message.end_of_track),
665+
WireBoolean(message.end_of_track),
703666
WireVarInt62(message.end_location.group),
704667
WireVarInt62(message.end_location.object == kMaxObjectId
705668
? 0
706669
: (message.end_location.object + 1)),
707-
WireKeyValuePairList(parameters));
670+
WireKeyValuePairList(message.parameters.ToKeyValuePairList()),
671+
WireKeyValuePairList(message.extensions, false));
708672
}
709673

710674
quiche::QuicheBuffer MoqtFramer::SerializeFetchCancel(
@@ -720,73 +684,19 @@ quiche::QuicheBuffer MoqtFramer::SerializeRequestsBlocked(
720684
}
721685

722686
quiche::QuicheBuffer MoqtFramer::SerializePublish(const MoqtPublish& message) {
723-
KeyValuePairList parameters;
724-
if (!FillAndValidateVersionSpecificParameters(
725-
MoqtMessageType::kPublish, message.parameters, parameters)) {
726-
return quiche::QuicheBuffer();
727-
};
728-
std::optional<uint64_t> group, object;
729-
if (message.largest_location.has_value()) {
730-
group = message.largest_location->group;
731-
object = message.largest_location->object;
732-
}
733687
return SerializeControlMessage(
734688
MoqtMessageType::kPublish, WireVarInt62(message.request_id),
735689
WireFullTrackName(message.full_track_name),
736-
WireVarInt62(message.track_alias), WireDeliveryOrder(message.group_order),
737-
WireBoolean(message.largest_location.has_value()),
738-
WireOptional<WireVarInt62>(group), WireOptional<WireVarInt62>(object),
739-
WireBoolean(message.forward), WireKeyValuePairList(parameters));
690+
WireVarInt62(message.track_alias),
691+
WireKeyValuePairList(message.parameters.ToKeyValuePairList()),
692+
WireKeyValuePairList(message.extensions, false));
740693
}
741694

742695
quiche::QuicheBuffer MoqtFramer::SerializePublishOk(
743696
const MoqtPublishOk& message) {
744-
KeyValuePairList parameters;
745-
if (!FillAndValidateVersionSpecificParameters(
746-
MoqtMessageType::kPublishOk, message.parameters, parameters)) {
747-
return quiche::QuicheBuffer();
748-
};
749-
std::optional<uint64_t> start_group, start_object, end_group;
750-
switch (message.filter_type) {
751-
case MoqtFilterType::kNextGroupStart:
752-
case MoqtFilterType::kLargestObject:
753-
break;
754-
case MoqtFilterType::kAbsoluteStart:
755-
case MoqtFilterType::kAbsoluteRange:
756-
if (!message.start.has_value()) {
757-
QUICHE_BUG(QUICHE_BUG_invalid_filter_type)
758-
<< "Serializing invalid MoQT filter type";
759-
return quiche::QuicheBuffer();
760-
}
761-
start_group = message.start->group;
762-
start_object = message.start->object;
763-
if (message.filter_type == MoqtFilterType::kAbsoluteStart) {
764-
break;
765-
}
766-
if (!message.end_group.has_value()) {
767-
QUICHE_BUG(QUICHE_BUG_invalid_filter_type)
768-
<< "Serializing invalid MoQT filter type";
769-
return quiche::QuicheBuffer();
770-
}
771-
end_group = message.end_group;
772-
if (*end_group < *start_group) {
773-
QUICHE_BUG(QUICHE_BUG_invalid_filter_type)
774-
<< "End group is less than start group";
775-
return quiche::QuicheBuffer();
776-
}
777-
break;
778-
default:
779-
QUICHE_BUG(QUICHE_BUG_invalid_filter_type)
780-
<< "Serializing invalid MoQT filter type";
781-
return quiche::QuicheBuffer();
782-
}
783697
return SerializeControlMessage(
784698
MoqtMessageType::kPublishOk, WireVarInt62(message.request_id),
785-
WireBoolean(message.forward), WireUint8(message.subscriber_priority),
786-
WireDeliveryOrder(message.group_order), WireVarInt62(message.filter_type),
787-
WireOptional<WireVarInt62>(start_group),
788-
WireOptional<WireVarInt62>(start_object),
789-
WireOptional<WireVarInt62>(end_group), WireKeyValuePairList(parameters));
699+
WireKeyValuePairList(message.parameters.ToKeyValuePairList()));
790700
}
791701

792702
quiche::QuicheBuffer MoqtFramer::SerializeObjectAck(
@@ -812,18 +722,6 @@ bool MoqtFramer::FillAndValidateSetupParameters(
812722
return true;
813723
}
814724

815-
bool MoqtFramer::FillAndValidateVersionSpecificParameters(
816-
MoqtMessageType message_type, const VersionSpecificParameters& parameters,
817-
KeyValuePairList& out) {
818-
if (!VersionSpecificParametersAllowedByMessage(parameters, message_type)) {
819-
QUICHE_BUG(QUICHE_BUG_invalid_parameters)
820-
<< "Invalid parameters for " << MoqtMessageTypeToString(message_type);
821-
return false;
822-
}
823-
out = parameters.ToKeyValuePairList();
824-
return true;
825-
}
826-
827725
// static
828726
bool MoqtFramer::ValidateObjectMetadata(const MoqtObject& object,
829727
bool is_datagram) {

‎quiche/quic/moqt/moqt_framer.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,6 @@ class QUICHE_EXPORT MoqtFramer {
8585
bool FillAndValidateSetupParameters(MoqtMessageType message_type,
8686
const SetupParameters& parameters,
8787
KeyValuePairList& out);
88-
bool FillAndValidateVersionSpecificParameters(
89-
MoqtMessageType message_type, const VersionSpecificParameters& parameters,
90-
KeyValuePairList& out);
9188
// Returns true if the metadata is internally consistent.
9289
static bool ValidateObjectMetadata(const MoqtObject& object,
9390
bool is_datagram);

‎quiche/quic/moqt/moqt_framer_test.cc‎

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -379,70 +379,15 @@ TEST_F(MoqtFramerSimpleTest, AllSubscribeInputs) {
379379
}
380380
}
381381

382-
TEST_F(MoqtFramerSimpleTest, PublishOkEndBeforeStart) {
383-
MoqtPublishOk publish_ok = {
384-
/*request_id=*/1,
385-
/*forward=*/true,
386-
/*subscriber_priority=*/2,
387-
/*group_order=*/MoqtDeliveryOrder::kAscending,
388-
/*filter_type=*/MoqtFilterType::kAbsoluteRange,
389-
/*start=*/Location{1, 2},
390-
/*end_group=*/0,
391-
/*parameters=*/VersionSpecificParameters(),
392-
};
393-
quiche::QuicheBuffer buffer;
394-
EXPECT_QUICHE_BUG(buffer = framer_.SerializePublishOk(publish_ok),
395-
"End group is less than start group");
396-
EXPECT_EQ(buffer.size(), 0);
397-
}
398-
399-
TEST_F(MoqtFramerSimpleTest, PublishOkMissingEndGroup) {
400-
MoqtPublishOk publish_ok = {
401-
/*request_id=*/1,
402-
/*forward=*/true,
403-
/*subscriber_priority=*/2,
404-
/*group_order=*/MoqtDeliveryOrder::kAscending,
405-
/*filter_type=*/MoqtFilterType::kAbsoluteRange,
406-
/*start=*/Location{1, 2},
407-
/*end_group=*/std::nullopt,
408-
/*parameters=*/VersionSpecificParameters(),
409-
};
410-
quiche::QuicheBuffer buffer;
411-
EXPECT_QUICHE_BUG(buffer = framer_.SerializePublishOk(publish_ok),
412-
"Serializing invalid MoQT filter type");
413-
EXPECT_EQ(buffer.size(), 0);
414-
}
415-
416-
TEST_F(MoqtFramerSimpleTest, PublishOkMissingStart) {
417-
MoqtPublishOk publish_ok = {
418-
/*request_id=*/1,
419-
/*forward=*/true,
420-
/*subscriber_priority=*/2,
421-
/*group_order=*/MoqtDeliveryOrder::kAscending,
422-
/*filter_type=*/MoqtFilterType::kAbsoluteStart,
423-
/*start=*/std::nullopt,
424-
/*end_group=*/std::nullopt,
425-
/*parameters=*/VersionSpecificParameters(),
426-
};
427-
quiche::QuicheBuffer buffer;
428-
EXPECT_QUICHE_BUG(buffer = framer_.SerializePublishOk(publish_ok),
429-
"Serializing invalid MoQT filter type");
430-
EXPECT_EQ(buffer.size(), 0);
431-
}
432-
433382
TEST_F(MoqtFramerSimpleTest, FetchEndBeforeStart) {
434383
MoqtFetch fetch = {
435384
/*request_id=*/1,
436-
/*subscriber_priority=*/2,
437-
/*group_order=*/MoqtDeliveryOrder::kAscending,
438-
/*fetch=*/
439385
StandaloneFetch{
440386
FullTrackName("foo", "bar"),
441387
/*start_location=*/Location{1, 2},
442388
/*end_location=*/Location{1, 1},
443389
},
444-
/*parameters=*/
445-
VersionSpecificParameters(AuthTokenType::kOutOfBand, "baz"),
390+
MessageParameters(),
446391
};
447392
quiche::QuicheBuffer buffer;
448393
EXPECT_QUIC_BUG(buffer = framer_.SerializeFetch(fetch),
@@ -458,10 +403,10 @@ TEST_F(MoqtFramerSimpleTest, FetchEndBeforeStart) {
458403
TEST_F(MoqtFramerSimpleTest, FetchOkWholeGroup) {
459404
MoqtFetchOk fetch_ok = {
460405
/*request_id=*/1,
461-
MoqtDeliveryOrder::kAscending,
462406
/*end_of_track=*/false,
463407
/*end_location=*/Location{4, kMaxObjectId},
464-
VersionSpecificParameters(),
408+
MessageParameters(),
409+
TrackExtensions(),
465410
};
466411
quiche::QuicheBuffer buffer = framer_.SerializeFetchOk(fetch_ok);
467412
// Check that object ID is zero.

‎quiche/quic/moqt/moqt_integration_test.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,7 @@ TEST_F(MoqtIntegrationTest, FetchItemsFromPast) {
480480
EXPECT_TRUE(client_->session()->Fetch(
481481
full_track_name,
482482
[&](std::unique_ptr<MoqtFetchTask> task) { fetch = std::move(task); },
483-
Location{0, 0}, 99, std::nullopt, 128, std::nullopt,
484-
VersionSpecificParameters()));
483+
Location{0, 0}, 99, std::nullopt, MessageParameters()));
485484
// Run until we get FETCH_OK.
486485
bool success = test_harness_.RunUntilWithDefaultTimeout(
487486
[&]() { return fetch != nullptr; });

‎quiche/quic/moqt/moqt_key_value_pair.h‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -337,43 +337,6 @@ class TrackExtensions : public KeyValuePairList {
337337
std::optional<uint64_t> max_value) const;
338338
};
339339

340-
// Version specific parameters.
341-
// TODO(martinduke): Replace with MessageParameters and delete when all
342-
// messages are migrated.
343-
enum class QUICHE_EXPORT VersionSpecificParameter : uint64_t {
344-
kDeliveryTimeout = 0x2,
345-
kAuthorizationToken = 0x3,
346-
kMaxCacheDuration = 0x4,
347-
348-
// QUICHE-specific extensions.
349-
kOackWindowSize = 0xbbf1438,
350-
};
351-
struct VersionSpecificParameters {
352-
VersionSpecificParameters() = default;
353-
// Likely parameter combinations.
354-
VersionSpecificParameters(quic::QuicTimeDelta delivery_timeout,
355-
quic::QuicTimeDelta max_cache_duration)
356-
: delivery_timeout(delivery_timeout),
357-
max_cache_duration(max_cache_duration) {}
358-
VersionSpecificParameters(AuthTokenType token_type, absl::string_view token) {
359-
authorization_tokens.emplace_back(token_type, token);
360-
}
361-
VersionSpecificParameters(quic::QuicTimeDelta delivery_timeout,
362-
AuthTokenType token_type, absl::string_view token)
363-
: delivery_timeout(delivery_timeout) {
364-
authorization_tokens.emplace_back(token_type, token);
365-
}
366-
367-
std::vector<AuthToken> authorization_tokens;
368-
quic::QuicTimeDelta delivery_timeout = quic::QuicTimeDelta::Infinite();
369-
quic::QuicTimeDelta max_cache_duration = quic::QuicTimeDelta::Infinite();
370-
std::optional<quic::QuicTimeDelta> oack_window_size;
371-
372-
bool operator==(const VersionSpecificParameters& other) const = default;
373-
KeyValuePairList ToKeyValuePairList() const;
374-
MoqtError FromKeyValuePairList(const KeyValuePairList& list);
375-
};
376-
377340
// TODO(martinduke): Extension Headers (MOQT draft-16 Sec 11)
378341

379342
} // namespace moqt

‎quiche/quic/moqt/moqt_messages.cc‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ MoqtError SetupParametersAllowedByMessage(const SetupParameters& parameters,
6262
return MoqtError::kNoError;
6363
}
6464

65+
// Parameter types are not enforced by message in draft-16, but apparently this
66+
// is coming back later.
67+
#if 0
6568
const std::array<MoqtMessageType, 9> kAllowsAuthorization = {
6669
MoqtMessageType::kClientSetup,
6770
MoqtMessageType::kServerSetup,
@@ -77,11 +80,8 @@ const std::array<MoqtMessageType, 7> kAllowsDeliveryTimeout = {
7780
MoqtMessageType::kPublish, MoqtMessageType::kPublishOk,
7881
MoqtMessageType::kSubscribe, MoqtMessageType::kSubscribeOk,
7982
MoqtMessageType::kRequestUpdate};
80-
const std::array<MoqtMessageType, 4> kAllowsMaxCacheDuration = {
81-
MoqtMessageType::kSubscribeOk, MoqtMessageType::kRequestOk,
82-
MoqtMessageType::kFetchOk, MoqtMessageType::kPublish};
83-
bool VersionSpecificParametersAllowedByMessage(
84-
const VersionSpecificParameters& parameters, MoqtMessageType message_type) {
83+
bool MessageParametersAllowedByMessage(
84+
const MessageParameters& parameters, MoqtMessageType message_type) {
8585
if (!parameters.authorization_tokens.empty() &&
8686
!absl::c_linear_search(kAllowsAuthorization, message_type)) {
8787
return false;
@@ -90,12 +90,9 @@ bool VersionSpecificParametersAllowedByMessage(
9090
!absl::c_linear_search(kAllowsDeliveryTimeout, message_type)) {
9191
return false;
9292
}
93-
if (parameters.max_cache_duration != quic::QuicTimeDelta::Infinite() &&
94-
!absl::c_linear_search(kAllowsMaxCacheDuration, message_type)) {
95-
return false;
96-
}
9793
return true;
9894
}
95+
#endif
9996

10097
std::string MoqtMessageTypeToString(const MoqtMessageType message_type) {
10198
switch (message_type) {

0 commit comments

Comments
 (0)