Skip to content

Commit f3ce189

Browse files
vasilvvcopybara-github
vasilvv
authored and
copybara-github
committed
Support the "has first object in the subgroup" bit in the subgroup header.
PiperOrigin-RevId: 927552535
1 parent 6b6a9b7 commit f3ce189

15 files changed

Lines changed: 215 additions & 89 deletions

‎quiche/quic/moqt/moqt_framer_test.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,8 @@ class MoqtFramerSimpleTest : public quic::test::QuicTest {
265265
};
266266

267267
TEST_F(MoqtFramerSimpleTest, GroupMiddler) {
268-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(1, 1, true, false);
268+
MoqtDataStreamType type =
269+
MoqtDataStreamType::Subgroup(1, 1, true, false, true);
269270
auto header = std::make_unique<StreamHeaderSubgroupMessage>(type);
270271
auto buffer1 = SerializeObject(
271272
framer_, std::get<MoqtObject>(header->structured_data()), "foo", type, 0);
@@ -324,13 +325,15 @@ TEST_F(MoqtFramerSimpleTest, BadObjectInput) {
324325
std::string(kDefaultExtensionBlob.data(), kDefaultExtensionBlob.size()),
325326
/*object_status=*/MoqtObjectStatus::kObjectDoesNotExist,
326327
/*subgroup_id=*/8,
328+
/*first_object_in_subgroup=*/true,
327329
/*payload_length=*/3,
328330
};
329331
quiche::QuicheBuffer buffer;
330332
std::optional<PublishedObjectMetadata> previous;
331333
EXPECT_QUIC_BUG(
332334
buffer = framer_.SerializeObjectHeader(
333-
object, MoqtDataStreamType::Subgroup(8, 0, false, false), previous),
335+
object, MoqtDataStreamType::Subgroup(8, 0, false, false, true),
336+
previous),
334337
"Object metadata is invalid");
335338
EXPECT_TRUE(buffer.empty());
336339
}
@@ -345,6 +348,7 @@ TEST_F(MoqtFramerSimpleTest, BadDatagramInput) {
345348
std::string(kDefaultExtensionBlob),
346349
/*object_status=*/MoqtObjectStatus::kNormal,
347350
/*subgroup_id=*/std::nullopt,
351+
/*first_object_in_subgroup=*/std::nullopt,
348352
/*payload_length=*/3,
349353
};
350354
quiche::QuicheBuffer buffer;

‎quiche/quic/moqt/moqt_messages.h‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class QUICHE_EXPORT MoqtDataStreamType {
4444
static constexpr uint64_t kExtensions = 0x01;
4545
static constexpr uint64_t kEndOfGroup = 0x08;
4646
static constexpr uint64_t kDefaultPriority = 0x20;
47+
static constexpr uint64_t kHasFirstObject = 0x40;
4748
// These two cannot simultaneously be true;
4849
static constexpr uint64_t kFirstObjectId = 0x02;
4950
static constexpr uint64_t kSubgroupId = 0x04;
@@ -59,7 +60,7 @@ class QUICHE_EXPORT MoqtDataStreamType {
5960
return std::nullopt;
6061
}
6162
if (value > (kSubgroup | kExtensions | kEndOfGroup | kDefaultPriority |
62-
kFirstObjectId | kSubgroupId)) {
63+
kFirstObjectId | kSubgroupId | kHasFirstObject)) {
6364
// Reserved bits.
6465
return std::nullopt;
6566
}
@@ -70,11 +71,9 @@ class QUICHE_EXPORT MoqtDataStreamType {
7071
}
7172
static MoqtDataStreamType Fetch() { return MoqtDataStreamType(kFetch); }
7273
static MoqtDataStreamType Padding() { return MoqtDataStreamType(kPadding); }
73-
static MoqtDataStreamType Subgroup(uint64_t subgroup_id,
74-
uint64_t first_object_id,
75-
bool no_extension_headers,
76-
bool default_priority,
77-
bool end_of_group = false) {
74+
static MoqtDataStreamType Subgroup(
75+
uint64_t subgroup_id, uint64_t first_object_id, bool no_extension_headers,
76+
bool default_priority, bool has_first_object, bool end_of_group = false) {
7877
uint64_t value = kSubgroup;
7978
if (!no_extension_headers) {
8079
value |= kExtensions;
@@ -85,6 +84,9 @@ class QUICHE_EXPORT MoqtDataStreamType {
8584
if (default_priority) {
8685
value |= kDefaultPriority;
8786
}
87+
if (has_first_object) {
88+
value |= kHasFirstObject;
89+
}
8890
if (subgroup_id == 0) {
8991
return MoqtDataStreamType(value);
9092
}
@@ -117,6 +119,9 @@ class QUICHE_EXPORT MoqtDataStreamType {
117119
bool HasDefaultPriority() const {
118120
return IsSubgroup() && (value_ & kDefaultPriority);
119121
}
122+
bool HasFirstObject() const {
123+
return IsSubgroup() && (value_ & kHasFirstObject);
124+
}
120125

121126
uint64_t value() const { return value_; }
122127
MoqtDataStreamType& operator=(const MoqtDataStreamType& other) = default;
@@ -252,7 +257,8 @@ struct QUICHE_EXPORT MoqtObject {
252257
MoqtPriority publisher_priority;
253258
std::string extension_headers; // Raw, unparsed extension headers.
254259
MoqtObjectStatus object_status;
255-
std::optional<uint64_t> subgroup_id; // Only for subgroup objects.
260+
std::optional<uint64_t> subgroup_id; // Only for subgroup objects.
261+
std::optional<bool> first_object_in_subgroup; // Only for subgroup objects.
256262
uint64_t payload_length;
257263
};
258264

‎quiche/quic/moqt/moqt_object.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ struct PublishedObjectMetadata {
2929
std::string extensions;
3030
MoqtObjectStatus status = MoqtObjectStatus::kNormal;
3131
MoqtPriority publisher_priority = kDefaultPublisherPriority;
32+
// `first_object_in_subgroup` is only available in objects communicated via
33+
// a subscription. It is not available for the fetched objects; however, the
34+
// subscriber can guarantee that all subgroups have a start object by setting
35+
// the FETCH range appropriately.
36+
std::optional<bool> first_object_in_subgroup;
3237
// The length of the entire payload, which might include data that is not
3338
// present in an encompassing PublishedObject or CachedObject.
3439
uint64_t payload_length;

‎quiche/quic/moqt/moqt_outgoing_queue.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ void MoqtOutgoingQueue::AddRawObject(MoqtObjectStatus status,
7979
"",
8080
status,
8181
default_publisher_priority(),
82+
queue_.back().empty(),
8283
payload.length(),
8384
clock_->ApproximateNow()};
8485
queue_.back().push_back(

‎quiche/quic/moqt/moqt_outgoing_queue_test.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ class TestMoqtOutgoingQueue : public MoqtOutgoingQueue,
6262
AnyOf(MoqtObjectStatus::kNormal,
6363
MoqtObjectStatus::kEndOfGroup,
6464
MoqtObjectStatus::kEndOfTrack)))));
65+
EXPECT_EQ(object->metadata.first_object_in_subgroup, sequence.object == 0);
6566
if (object->metadata.status == MoqtObjectStatus::kNormal) {
6667
PublishObject(object->metadata.location.group,
6768
object->metadata.location.object,

‎quiche/quic/moqt/moqt_parser.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1285,6 +1285,10 @@ MoqtDataParser::NextInput MoqtDataParser::AdvanceParserState() {
12851285
if (num_objects_read_ == 0 && type_.SubgroupIsFirstObjectId()) {
12861286
metadata_.subgroup_id = metadata_.object_id;
12871287
}
1288+
if (type_.IsSubgroup()) {
1289+
metadata_.first_object_in_subgroup =
1290+
type_.HasFirstObject() && num_objects_read_ == 0;
1291+
}
12881292
if (type_.AreExtensionHeadersPresent()) {
12891293
return kExtensionSize;
12901294
}

‎quiche/quic/moqt/moqt_parser_test.cc‎

Lines changed: 73 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,8 @@ TEST_F(MoqtMessageSpecificTest, ThreePartObject) {
463463
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
464464
MoqtParserTestVisitor data_visitor;
465465
MoqtDataParser parser(&stream, &data_visitor);
466-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(1, 1, true, false);
466+
MoqtDataStreamType type =
467+
MoqtDataStreamType::Subgroup(1, 1, true, false, true);
467468
auto message = std::make_unique<StreamHeaderSubgroupMessage>(type);
468469
EXPECT_TRUE(message->SetPayloadLength(14));
469470
message->set_wire_image_size(message->total_message_size() - 11);
@@ -499,7 +500,8 @@ TEST_F(MoqtMessageSpecificTest, ThreePartObjectFirstIncomplete) {
499500
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
500501
MoqtParserTestVisitor data_visitor;
501502
MoqtDataParser parser(&stream, &data_visitor);
502-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(2, 1, false, false);
503+
MoqtDataStreamType type =
504+
MoqtDataStreamType::Subgroup(2, 1, false, false, true);
503505
auto message = std::make_unique<StreamHeaderSubgroupMessage>(type);
504506
EXPECT_TRUE(message->SetPayloadLength(payload_length));
505507

@@ -533,7 +535,8 @@ TEST_F(MoqtMessageSpecificTest, ObjectSplitInExtension) {
533535
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
534536
MoqtParserTestVisitor data_visitor;
535537
MoqtDataParser parser(&stream, &data_visitor);
536-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(2, 1, false, false);
538+
MoqtDataStreamType type =
539+
MoqtDataStreamType::Subgroup(2, 1, false, false, true);
537540
auto message = std::make_unique<StreamHeaderSubgroupMessage>(type);
538541

539542
// first part
@@ -557,7 +560,8 @@ TEST_F(MoqtMessageSpecificTest, StreamHeaderSubgroupFollowOn) {
557560
MoqtParserTestVisitor data_visitor;
558561
MoqtDataParser parser(&stream, &data_visitor);
559562
// first part
560-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(0, 1, false, false);
563+
MoqtDataStreamType type =
564+
MoqtDataStreamType::Subgroup(0, 1, false, false, true);
561565
auto message1 = std::make_unique<StreamHeaderSubgroupMessage>(type);
562566
stream.Receive(message1->PacketSample(), false);
563567
parser.ReadAllData();
@@ -583,7 +587,8 @@ TEST_F(MoqtMessageSpecificTest, StreamHeaderSubgroupFollowOnExpandedVarInts) {
583587
MoqtParserTestVisitor data_visitor;
584588
MoqtDataParser parser(&stream, &data_visitor);
585589
// first part
586-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(0, 1, false, false);
590+
MoqtDataStreamType type =
591+
MoqtDataStreamType::Subgroup(0, 1, false, false, true);
587592
auto message1 = std::make_unique<StreamHeaderSubgroupMessage>(type);
588593
message1->ExpandVarints();
589594
stream.Receive(message1->PacketSample(), false);
@@ -954,7 +959,8 @@ TEST_F(MoqtMessageSpecificTest, FinMidDataPayload) {
954959
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
955960
MoqtParserTestVisitor data_visitor;
956961
MoqtDataParser parser(&stream, &data_visitor);
957-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(0, 1, true, false);
962+
MoqtDataStreamType type =
963+
MoqtDataStreamType::Subgroup(0, 1, true, false, true);
958964
auto message = std::make_unique<StreamHeaderSubgroupMessage>(type);
959965
stream.Receive(
960966
message->PacketSample().substr(0, message->total_message_size() - 1),
@@ -972,7 +978,8 @@ TEST_F(MoqtMessageSpecificTest, FinMidExtension) {
972978
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
973979
MoqtParserTestVisitor data_visitor;
974980
MoqtDataParser parser(&stream, &data_visitor);
975-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(0, 1, false, false);
981+
MoqtDataStreamType type =
982+
MoqtDataStreamType::Subgroup(0, 1, false, false, true);
976983
auto message = std::make_unique<StreamHeaderSubgroupMessage>(type);
977984
// Read up to the extension body and then FIN.
978985
stream.Receive(message->PacketSample().substr(0, 7), true);
@@ -989,7 +996,8 @@ TEST_F(MoqtMessageSpecificTest, PartialPayloadThenFin) {
989996
webtransport::test::InMemoryStream stream(/*stream_id=*/0);
990997
MoqtParserTestVisitor data_visitor;
991998
MoqtDataParser parser(&stream, &data_visitor);
992-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(1, 1, false, false);
999+
MoqtDataStreamType type =
1000+
MoqtDataStreamType::Subgroup(1, 1, false, false, true);
9931001
auto message = std::make_unique<StreamHeaderSubgroupMessage>(type);
9941002
stream.Receive(
9951003
message->PacketSample().substr(0, message->total_message_size() - 1),
@@ -1600,7 +1608,8 @@ class MoqtDataParserStateMachineTest : public quic::test::QuicTest {
16001608
};
16011609

16021610
TEST_F(MoqtDataParserStateMachineTest, ReadAll) {
1603-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(0, 1, false, false);
1611+
MoqtDataStreamType type =
1612+
MoqtDataStreamType::Subgroup(0, 1, false, false, true);
16041613
stream_.Receive(StreamHeaderSubgroupMessage(type).PacketSample());
16051614
stream_.Receive(StreamMiddlerSubgroupMessage(type).PacketSample());
16061615
parser_.ReadAllData();
@@ -1614,7 +1623,8 @@ TEST_F(MoqtDataParserStateMachineTest, ReadAll) {
16141623
}
16151624

16161625
TEST_F(MoqtDataParserStateMachineTest, ReadObjects) {
1617-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(0, 1, true, false);
1626+
MoqtDataStreamType type =
1627+
MoqtDataStreamType::Subgroup(0, 1, true, false, true);
16181628
stream_.Receive(StreamHeaderSubgroupMessage(type).PacketSample());
16191629
stream_.Receive(StreamMiddlerSubgroupMessage(type).PacketSample(),
16201630
/*fin=*/true);
@@ -1629,7 +1639,8 @@ TEST_F(MoqtDataParserStateMachineTest, ReadObjects) {
16291639
}
16301640

16311641
TEST_F(MoqtDataParserStateMachineTest, ReadTypeThenObjects) {
1632-
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(1, 1, false, false);
1642+
MoqtDataStreamType type =
1643+
MoqtDataStreamType::Subgroup(1, 1, false, false, true);
16331644
stream_.Receive(StreamHeaderSubgroupMessage(type).PacketSample());
16341645
stream_.Receive(StreamMiddlerSubgroupMessage(type).PacketSample(),
16351646
/*fin=*/true);
@@ -1742,7 +1753,8 @@ TEST_F(MoqtDataParserStateMachineTest, IgnoresEndRangeIndicators) {
17421753

17431754
TEST_F(MoqtDataParserStateMachineTest, IntegerOverflowObjectId) {
17441755
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(
1745-
0, 1, /*no_extension_headers=*/true, /*default_priority=*/false);
1756+
0, 1, /*no_extension_headers=*/true, /*default_priority=*/false,
1757+
/*has_first_object=*/true);
17461758
stream_.Receive(StreamHeaderSubgroupMessage(type).PacketSample());
17471759
char buffer[32];
17481760
quic::QuicDataWriter writer(sizeof(buffer), buffer);
@@ -1757,4 +1769,53 @@ TEST_F(MoqtDataParserStateMachineTest, IntegerOverflowObjectId) {
17571769
"Integer overflow when parsing object ID");
17581770
}
17591771

1772+
TEST_F(MoqtDataParserStateMachineTest, SubgroupHasFirstObjectTrue) {
1773+
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(
1774+
0, 1, /*no_extension_headers=*/true, /*default_priority=*/false,
1775+
/*has_first_object=*/true);
1776+
stream_.Receive(StreamHeaderSubgroupMessage(type).PacketSample());
1777+
stream_.Receive(StreamMiddlerSubgroupMessage(type).PacketSample(),
1778+
/*fin=*/true);
1779+
parser_.ReadAtMostOneObject();
1780+
ASSERT_EQ(visitor_.messages_received(), 1);
1781+
ASSERT_TRUE(visitor_.last_message().has_value());
1782+
EXPECT_EQ(visitor_.last_message()->first_object_in_subgroup, true);
1783+
parser_.ReadAtMostOneObject();
1784+
ASSERT_EQ(visitor_.messages_received(), 2);
1785+
ASSERT_TRUE(visitor_.last_message().has_value());
1786+
EXPECT_EQ(visitor_.last_message()->first_object_in_subgroup, false);
1787+
EXPECT_EQ(visitor_.parsing_error(), std::nullopt);
1788+
EXPECT_TRUE(visitor_.fin_received());
1789+
}
1790+
1791+
TEST_F(MoqtDataParserStateMachineTest, SubgroupHasFirstObjectFalse) {
1792+
MoqtDataStreamType type = MoqtDataStreamType::Subgroup(
1793+
0, 1, /*no_extension_headers=*/true, /*default_priority=*/false,
1794+
/*has_first_object=*/false);
1795+
stream_.Receive(StreamHeaderSubgroupMessage(type).PacketSample());
1796+
stream_.Receive(StreamMiddlerSubgroupMessage(type).PacketSample(),
1797+
/*fin=*/true);
1798+
parser_.ReadAtMostOneObject();
1799+
ASSERT_EQ(visitor_.messages_received(), 1);
1800+
ASSERT_TRUE(visitor_.last_message().has_value());
1801+
EXPECT_EQ(visitor_.last_message()->first_object_in_subgroup, false);
1802+
parser_.ReadAtMostOneObject();
1803+
ASSERT_EQ(visitor_.messages_received(), 2);
1804+
ASSERT_TRUE(visitor_.last_message().has_value());
1805+
EXPECT_EQ(visitor_.last_message()->first_object_in_subgroup, false);
1806+
EXPECT_EQ(visitor_.parsing_error(), std::nullopt);
1807+
EXPECT_TRUE(visitor_.fin_received());
1808+
}
1809+
1810+
TEST_F(MoqtDataParserStateMachineTest, FetchFirstObjectMissing) {
1811+
StreamHeaderFetchMessage header;
1812+
stream_.Receive(header.PacketSample());
1813+
parser_.ReadStreamType();
1814+
ASSERT_EQ(visitor_.messages_received(), 0);
1815+
parser_.ReadAtMostOneObject();
1816+
ASSERT_EQ(visitor_.messages_received(), 1);
1817+
ASSERT_TRUE(visitor_.last_message().has_value());
1818+
EXPECT_FALSE(visitor_.last_message()->first_object_in_subgroup.has_value());
1819+
}
1820+
17601821
} // namespace moqt::test

0 commit comments

Comments
 (0)