Delete MoQT Track Forwarding preference and roll version to -07. PiperOrigin-RevId: 698888178
diff --git a/quiche/quic/moqt/moqt_framer.cc b/quiche/quic/moqt/moqt_framer.cc index fe8c47f..26a77dd 100644 --- a/quiche/quic/moqt/moqt_framer.cc +++ b/quiche/quic/moqt/moqt_framer.cc
@@ -277,15 +277,6 @@ } if (!is_first_in_stream) { switch (message_type) { - case MoqtDataStreamType::kStreamHeaderTrack: - return (message.payload_length == 0) - ? Serialize(WireVarInt62(message.group_id), - WireVarInt62(message.object_id), - WireVarInt62(message.payload_length), - WireVarInt62(message.object_status)) - : Serialize(WireVarInt62(message.group_id), - WireVarInt62(message.object_id), - WireVarInt62(message.payload_length)); case MoqtDataStreamType::kStreamHeaderSubgroup: return (message.payload_length == 0) ? Serialize(WireVarInt62(message.object_id), @@ -314,21 +305,6 @@ } } switch (message_type) { - case MoqtDataStreamType::kStreamHeaderTrack: - return (message.payload_length == 0) - ? Serialize(WireVarInt62(message_type), - WireVarInt62(message.track_alias), - WireUint8(message.publisher_priority), - WireVarInt62(message.group_id), - WireVarInt62(message.object_id), - WireVarInt62(message.payload_length), - WireVarInt62(message.object_status)) - : Serialize(WireVarInt62(message_type), - WireVarInt62(message.track_alias), - WireUint8(message.publisher_priority), - WireVarInt62(message.group_id), - WireVarInt62(message.object_id), - WireVarInt62(message.payload_length)); case MoqtDataStreamType::kStreamHeaderSubgroup: return (message.payload_length == 0) ? Serialize(WireVarInt62(message_type),
diff --git a/quiche/quic/moqt/moqt_framer_test.cc b/quiche/quic/moqt/moqt_framer_test.cc index 0a2387b..8f93e2c 100644 --- a/quiche/quic/moqt/moqt_framer_test.cc +++ b/quiche/quic/moqt/moqt_framer_test.cc
@@ -275,22 +275,6 @@ EXPECT_EQ(buffer2.AsStringView(), middler->PacketSample()); } -TEST_F(MoqtFramerSimpleTest, TrackMiddler) { - auto header = std::make_unique<StreamHeaderTrackMessage>(); - auto buffer1 = - SerializeObject(framer_, std::get<MoqtObject>(header->structured_data()), - "foo", MoqtDataStreamType::kStreamHeaderTrack, true); - EXPECT_EQ(buffer1.size(), header->total_message_size()); - EXPECT_EQ(buffer1.AsStringView(), header->PacketSample()); - - auto middler = std::make_unique<StreamMiddlerTrackMessage>(); - auto buffer2 = - SerializeObject(framer_, std::get<MoqtObject>(middler->structured_data()), - "bar", MoqtDataStreamType::kStreamHeaderTrack, false); - EXPECT_EQ(buffer2.size(), middler->total_message_size()); - EXPECT_EQ(buffer2.AsStringView(), middler->PacketSample()); -} - TEST_F(MoqtFramerSimpleTest, FetchMiddler) { auto header = std::make_unique<StreamHeaderFetchMessage>(); auto buffer1 = @@ -337,14 +321,6 @@ EXPECT_TRUE(buffer.empty()); object.subgroup_id = 8; - // kTrack must not have a subgroup_id. - object.forwarding_preference = MoqtForwardingPreference::kTrack; - EXPECT_QUIC_BUG(buffer = framer_.SerializeObjectHeader( - object, MoqtDataStreamType::kStreamHeaderTrack, false), - "Object metadata is invalid"); - EXPECT_TRUE(buffer.empty()); - object.forwarding_preference = MoqtForwardingPreference::kSubgroup; - // Non-normal status must have no payload. object.object_status = MoqtObjectStatus::kEndOfGroup; EXPECT_QUIC_BUG(buffer = framer_.SerializeObjectHeader(
diff --git a/quiche/quic/moqt/moqt_integration_test.cc b/quiche/quic/moqt/moqt_integration_test.cc index fe6cecf..9218552 100644 --- a/quiche/quic/moqt/moqt_integration_test.cc +++ b/quiche/quic/moqt/moqt_integration_test.cc
@@ -231,7 +231,7 @@ server_->session()->set_publisher(&publisher); for (MoqtForwardingPreference forwarding_preference : - {MoqtForwardingPreference::kTrack, MoqtForwardingPreference::kSubgroup, + {MoqtForwardingPreference::kSubgroup, MoqtForwardingPreference::kDatagram}) { SCOPED_TRACE(MoqtForwardingPreferenceToString(forwarding_preference)); MockRemoteTrackVisitor client_visitor; @@ -294,7 +294,7 @@ server_->session()->set_publisher(&publisher); for (MoqtForwardingPreference forwarding_preference : - {MoqtForwardingPreference::kTrack, MoqtForwardingPreference::kSubgroup, + {MoqtForwardingPreference::kSubgroup, MoqtForwardingPreference::kDatagram}) { SCOPED_TRACE(MoqtForwardingPreferenceToString(forwarding_preference)); MockRemoteTrackVisitor client_visitor;
diff --git a/quiche/quic/moqt/moqt_live_relay_queue.cc b/quiche/quic/moqt/moqt_live_relay_queue.cc index a81139e..f093014 100644 --- a/quiche/quic/moqt/moqt_live_relay_queue.cc +++ b/quiche/quic/moqt/moqt_live_relay_queue.cc
@@ -27,9 +27,6 @@ switch (forwarding_preference_) { case MoqtForwardingPreference::kDatagram: return false; - case MoqtForwardingPreference::kTrack: - // TODO(martinduke): Support if it doesn't go away. - return false; case MoqtForwardingPreference::kSubgroup: break; }
diff --git a/quiche/quic/moqt/moqt_messages.cc b/quiche/quic/moqt/moqt_messages.cc index ce6680c..e395190 100644 --- a/quiche/quic/moqt/moqt_messages.cc +++ b/quiche/quic/moqt/moqt_messages.cc
@@ -125,8 +125,6 @@ switch (type) { case MoqtDataStreamType::kObjectDatagram: return "OBJECT_PREFER_DATAGRAM"; - case MoqtDataStreamType::kStreamHeaderTrack: - return "STREAM_HEADER_TRACK"; case MoqtDataStreamType::kStreamHeaderSubgroup: return "STREAM_HEADER_SUBGROUP"; case MoqtDataStreamType::kStreamHeaderFetch: @@ -142,8 +140,6 @@ switch (preference) { case MoqtForwardingPreference::kDatagram: return "DATAGRAM"; - case MoqtForwardingPreference::kTrack: - return "TRACK"; case MoqtForwardingPreference::kSubgroup: return "SUBGROUP"; } @@ -156,12 +152,12 @@ switch (type) { case MoqtDataStreamType::kObjectDatagram: return MoqtForwardingPreference::kDatagram; - case MoqtDataStreamType::kStreamHeaderTrack: - return MoqtForwardingPreference::kTrack; case MoqtDataStreamType::kStreamHeaderSubgroup: return MoqtForwardingPreference::kSubgroup; case MoqtDataStreamType::kStreamHeaderFetch: - return MoqtForwardingPreference::kTrack; // This is a placeholder. + QUIC_BUG(quic_bug_forwarding_preference_for_fetch) + << "Forwarding preference for fetch is not supported"; + break; default: break; } @@ -175,8 +171,6 @@ switch (preference) { case MoqtForwardingPreference::kDatagram: return MoqtDataStreamType::kObjectDatagram; - case MoqtForwardingPreference::kTrack: - return MoqtDataStreamType::kStreamHeaderTrack; case MoqtForwardingPreference::kSubgroup: return MoqtDataStreamType::kStreamHeaderSubgroup; }
diff --git a/quiche/quic/moqt/moqt_messages.h b/quiche/quic/moqt/moqt_messages.h index 0b09438..680ebf0 100644 --- a/quiche/quic/moqt/moqt_messages.h +++ b/quiche/quic/moqt/moqt_messages.h
@@ -32,11 +32,11 @@ } enum class MoqtVersion : uint64_t { - kDraft06 = 0xff000006, + kDraft07 = 0xff000007, kUnrecognizedVersionForTests = 0xfe0000ff, }; -inline constexpr MoqtVersion kDefaultMoqtVersion = MoqtVersion::kDraft06; +inline constexpr MoqtVersion kDefaultMoqtVersion = MoqtVersion::kDraft07; inline constexpr uint64_t kDefaultInitialMaxSubscribeId = 100; struct QUICHE_EXPORT MoqtSessionParameters { @@ -66,7 +66,6 @@ enum class QUICHE_EXPORT MoqtDataStreamType : uint64_t { kObjectDatagram = 0x01, - kStreamHeaderTrack = 0x02, kStreamHeaderSubgroup = 0x04, kStreamHeaderFetch = 0x05, @@ -305,7 +304,6 @@ // These codes do not appear on the wire. enum class QUICHE_EXPORT MoqtForwardingPreference { - kTrack, kSubgroup, kDatagram, };
diff --git a/quiche/quic/moqt/moqt_parser.cc b/quiche/quic/moqt/moqt_parser.cc index eae5b48..f643767 100644 --- a/quiche/quic/moqt/moqt_parser.cc +++ b/quiche/quic/moqt/moqt_parser.cc
@@ -52,7 +52,6 @@ bool IsAllowedStreamType(uint64_t value) { constexpr std::array kAllowedStreamTypes = { MoqtDataStreamType::kStreamHeaderSubgroup, - MoqtDataStreamType::kStreamHeaderTrack, MoqtDataStreamType::kStreamHeaderFetch, MoqtDataStreamType::kPadding}; for (MoqtDataStreamType type : kAllowedStreamTypes) { if (static_cast<uint64_t>(type) == value) { @@ -67,8 +66,7 @@ if (!reader.ReadVarInt62(&object.track_alias)) { return 0; } - if (type != MoqtDataStreamType::kStreamHeaderTrack && - type != MoqtDataStreamType::kStreamHeaderFetch && + if (type != MoqtDataStreamType::kStreamHeaderFetch && !reader.ReadVarInt62(&object.group_id)) { return 0; } @@ -94,14 +92,12 @@ return 0; } object.object_status = IntegerToObjectStatus(status); - object.forwarding_preference = GetForwardingPreference(type); return reader.PreviouslyReadPayload().size(); } size_t ParseObjectSubheader(quic::QuicDataReader& reader, MoqtObject& object, MoqtDataStreamType type) { switch (type) { - case MoqtDataStreamType::kStreamHeaderTrack: case MoqtDataStreamType::kStreamHeaderFetch: if (!reader.ReadVarInt62(&object.group_id)) { return 0; @@ -1082,6 +1078,7 @@ if (processed_data == 0) { // Incomplete header return absl::string_view(); } + object_metadata.forwarding_preference = MoqtForwardingPreference::kDatagram; return reader.PeekRemainingPayload(); } @@ -1156,6 +1153,7 @@ if (bytes_read == 0) { return remainder; } + header.forwarding_preference = MoqtForwardingPreference::kSubgroup; metadata_ = header; continue; }
diff --git a/quiche/quic/moqt/moqt_parser_test.cc b/quiche/quic/moqt/moqt_parser_test.cc index a7c7c74..1c77875 100644 --- a/quiche/quic/moqt/moqt_parser_test.cc +++ b/quiche/quic/moqt/moqt_parser_test.cc
@@ -59,7 +59,6 @@ MoqtMessageType::kObjectAck, }; constexpr std::array kDataStreamTypes{ - MoqtDataStreamType::kStreamHeaderTrack, MoqtDataStreamType::kStreamHeaderSubgroup, MoqtDataStreamType::kStreamHeaderFetch, }; @@ -510,27 +509,6 @@ EXPECT_FALSE(visitor_.parsing_error_.has_value()); } -TEST_F(MoqtMessageSpecificTest, StreamHeaderTrackFollowOn) { - MoqtDataParser parser(&visitor_); - // first part - auto message1 = std::make_unique<StreamHeaderTrackMessage>(); - parser.ProcessData(message1->PacketSample(), false); - EXPECT_EQ(visitor_.messages_received_, 1); - EXPECT_TRUE(message1->EqualFieldValues(*visitor_.last_message_)); - EXPECT_TRUE(visitor_.end_of_message_); - EXPECT_EQ(visitor_.object_payload(), "foo"); - EXPECT_FALSE(visitor_.parsing_error_.has_value()); - // second part - visitor_.object_payloads_.clear(); - auto message2 = std::make_unique<StreamMiddlerTrackMessage>(); - parser.ProcessData(message2->PacketSample(), false); - EXPECT_EQ(visitor_.messages_received_, 2); - EXPECT_TRUE(message2->EqualFieldValues(*visitor_.last_message_)); - EXPECT_TRUE(visitor_.end_of_message_); - EXPECT_EQ(visitor_.object_payload(), "bar"); - EXPECT_FALSE(visitor_.parsing_error_.has_value()); -} - TEST_F(MoqtMessageSpecificTest, ClientSetupRoleIsInvalid) { MoqtControlParser parser(kRawQuic, visitor_); char setup[] = { @@ -892,7 +870,7 @@ TEST_F(MoqtMessageSpecificTest, PartialPayloadThenFin) { MoqtDataParser parser(&visitor_); - auto message = std::make_unique<StreamHeaderTrackMessage>(); + auto message = std::make_unique<StreamHeaderSubgroupMessage>(); parser.ProcessData( message->PacketSample().substr(0, message->total_message_size() - 1), false);
diff --git a/quiche/quic/moqt/moqt_session.cc b/quiche/quic/moqt/moqt_session.cc index ec7a867..93da87b 100644 --- a/quiche/quic/moqt/moqt_session.cc +++ b/quiche/quic/moqt/moqt_session.cc
@@ -1278,20 +1278,13 @@ MoqtPriority publisher_priority = track_publisher_->GetPublisherPriority(); MoqtDeliveryOrder delivery_order = subscriber_delivery_order().value_or( track_publisher_->GetDeliveryOrder()); - switch (forwarding_preference) { - case MoqtForwardingPreference::kTrack: - return SendOrderForStream(subscriber_priority_, publisher_priority, - /*group_id=*/0, delivery_order); - break; - case MoqtForwardingPreference::kSubgroup: - return SendOrderForStream(subscriber_priority_, publisher_priority, - sequence.group, sequence.subgroup, - delivery_order); - break; - case MoqtForwardingPreference::kDatagram: - QUICHE_NOTREACHED(); - return 0; + if (forwarding_preference == MoqtForwardingPreference::kDatagram) { + QUICHE_BUG(quic_bug_GetSendOrder_for_Datagram) + << "Datagram Track requesting SendOrder"; + return 0; } + return SendOrderForStream(subscriber_priority_, publisher_priority, + sequence.group, sequence.subgroup, delivery_order); } // Returns the highest send order in the subscription. @@ -1442,29 +1435,16 @@ MoqtForwardingPreference forwarding_preference = publisher.GetForwardingPreference(); UpdateSendOrder(subscription); - switch (forwarding_preference) { - case MoqtForwardingPreference::kTrack: - if (object->status == MoqtObjectStatus::kEndOfGroup || - object->status == MoqtObjectStatus::kGroupDoesNotExist) { - ++next_object_.group; - next_object_.object = 0; - } else { - next_object_.object = object->sequence.object + 1; - } - break; - - case MoqtForwardingPreference::kSubgroup: - next_object_.object = object->sequence.object + 1; - break; - - case MoqtForwardingPreference::kDatagram: - QUICHE_NOTREACHED(); - break; + if (forwarding_preference == MoqtForwardingPreference::kDatagram) { + QUICHE_BUG(quic_bug_SendObjects_for_Datagram) + << "Datagram Track requesting SendObjects"; + return; } + next_object_.object = object->sequence.object + 1; if (session_->WriteObjectToStream( stream_, subscription.track_alias(), *object, - GetMessageTypeForForwardingPreference(forwarding_preference), - !stream_header_written_, object->fin_after_this)) { + MoqtDataStreamType::kStreamHeaderSubgroup, !stream_header_written_, + object->fin_after_this)) { stream_header_written_ = true; subscription.OnObjectSent(object->sequence); }
diff --git a/quiche/quic/moqt/moqt_session_test.cc b/quiche/quic/moqt/moqt_session_test.cc index 5e40775..de654f5 100644 --- a/quiche/quic/moqt/moqt_session_test.cc +++ b/quiche/quic/moqt/moqt_session_test.cc
@@ -1160,7 +1160,7 @@ EXPECT_CALL(mock_stream, GetStreamId()) .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, true); - object.forwarding_preference = MoqtForwardingPreference::kTrack; + object.forwarding_preference = MoqtForwardingPreference::kDatagram; ++object.object_id; EXPECT_CALL(mock_session_, CloseSession(static_cast<uint64_t>(MoqtError::kProtocolViolation), @@ -1215,7 +1215,7 @@ MoqtSessionPeer::CreateRemoteTrack(&session_, ftn, &visitor, 2); // The track already exists, and has a different forwarding preference. MoqtSessionPeer::remote_track(&session_, 2) - .CheckForwardingPreference(MoqtForwardingPreference::kTrack); + .CheckForwardingPreference(MoqtForwardingPreference::kDatagram); // SUBSCRIBE_OK arrives MoqtSubscribeOk ok = { @@ -1696,8 +1696,8 @@ TEST_F(MoqtSessionTest, ReceiveUnsubscribe) { FullTrackName ftn("foo", "bar"); - auto track = - SetupPublisher(ftn, MoqtForwardingPreference::kTrack, FullSequence(4, 2)); + auto track = SetupPublisher(ftn, MoqtForwardingPreference::kSubgroup, + FullSequence(4, 2)); MoqtSessionPeer::AddSubscription(&session_, track, 0, 1, 3, 4); webtransport::test::MockStream mock_stream; std::unique_ptr<MoqtControlParserVisitor> stream_input = @@ -1800,7 +1800,7 @@ .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, true); ++object.object_id; - object.forwarding_preference = MoqtForwardingPreference::kTrack; + object.forwarding_preference = MoqtForwardingPreference::kDatagram; EXPECT_CALL(mock_session_, CloseSession(static_cast<uint64_t>(MoqtError::kProtocolViolation), "Forwarding preference changes mid-track")) @@ -2398,7 +2398,8 @@ MoqtSessionPeer::set_peer_role(&session_, MoqtRole::kSubscriber); FullTrackName ftn("foo", "bar"); MockLocalTrackVisitor track_visitor; - session_.AddLocalTrack(ftn, MoqtForwardingPreference::kTrack, &track_visitor); + session_.AddLocalTrack(ftn, MoqtForwardingPreference::kSubgroup, + &track_visitor); MoqtSessionPeer::AddSubscription(&session_, ftn, 0, 2, 5, 0); // Get the window, set the maximum delivered. LocalTrack* track = MoqtSessionPeer::local_track(&session_, ftn);
diff --git a/quiche/quic/moqt/moqt_subscribe_windows.cc b/quiche/quic/moqt/moqt_subscribe_windows.cc index 6e9c8e9..0bff04d 100644 --- a/quiche/quic/moqt/moqt_subscribe_windows.cc +++ b/quiche/quic/moqt/moqt_subscribe_windows.cc
@@ -25,9 +25,6 @@ ReducedSequenceIndex::ReducedSequenceIndex( FullSequence sequence, MoqtForwardingPreference preference) { switch (preference) { - case MoqtForwardingPreference::kTrack: - sequence_ = FullSequence(0, 0, 0); - break; case MoqtForwardingPreference::kSubgroup: sequence_ = FullSequence(sequence.group, sequence.subgroup, 0); break;
diff --git a/quiche/quic/moqt/moqt_subscribe_windows_test.cc b/quiche/quic/moqt/moqt_subscribe_windows_test.cc index 1fcc43d..b09adf4 100644 --- a/quiche/quic/moqt/moqt_subscribe_windows_test.cc +++ b/quiche/quic/moqt/moqt_subscribe_windows_test.cc
@@ -34,16 +34,6 @@ EXPECT_FALSE(window.InWindow(FullSequence(3, 12))); } -TEST_F(SubscribeWindowTest, AddQueryRemoveStreamIdTrack) { - SendStreamMap stream_map(MoqtForwardingPreference::kTrack); - stream_map.AddStream(FullSequence{4, 0}, 2); - EXPECT_QUIC_BUG(stream_map.AddStream(FullSequence{5, 2}, 6), - "Stream already added"); - EXPECT_EQ(stream_map.GetStreamForSequence(FullSequence(5, 2)), 2); - stream_map.RemoveStream(FullSequence{7, 2}, 2); - EXPECT_EQ(stream_map.GetStreamForSequence(FullSequence(4, 0)), std::nullopt); -} - TEST_F(SubscribeWindowTest, AddQueryRemoveStreamIdSubgroup) { SendStreamMap stream_map(MoqtForwardingPreference::kSubgroup); stream_map.AddStream(FullSequence{4, 0}, 2);
diff --git a/quiche/quic/moqt/test_tools/moqt_test_message.h b/quiche/quic/moqt/test_tools/moqt_test_message.h index c26261f..0fc7d7a 100644 --- a/quiche/quic/moqt/test_tools/moqt_test_message.h +++ b/quiche/quic/moqt/test_tools/moqt_test_message.h
@@ -210,7 +210,7 @@ /*object_id=*/6, /*publisher_priority=*/7, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kTrack, + /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/std::nullopt, /*payload_length=*/3, }; @@ -235,49 +235,6 @@ // Concatentation of the base header and the object-specific header. Follow-on // object headers are handled in a different class. -class QUICHE_NO_EXPORT StreamHeaderTrackMessage : public ObjectMessage { - public: - StreamHeaderTrackMessage() : ObjectMessage() { - SetWireImage(raw_packet_, sizeof(raw_packet_)); - object_.forwarding_preference = MoqtForwardingPreference::kTrack; - object_.payload_length = 3; - } - - void ExpandVarints() override { ExpandVarintsImpl("vv-vvv", false); } - - private: - // Some tests check that a FIN sent at the halfway point of a message results - // in an error. Without the unnecessary expanded varint 0x0405, the halfway - // point falls at the end of the Stream Header, which is legal. Expand the - // varint so that the FIN would be illegal. - uint8_t raw_packet_[9] = { - 0x02, // type field - 0x04, // varints - 0x07, // publisher priority - 0x05, 0x06, // object middler - 0x03, 0x66, 0x6f, 0x6f, // payload = "foo" - }; -}; - -// Used only for tests that process multiple objects on one stream. -class QUICHE_NO_EXPORT StreamMiddlerTrackMessage : public ObjectMessage { - public: - StreamMiddlerTrackMessage() : ObjectMessage() { - SetWireImage(raw_packet_, sizeof(raw_packet_)); - object_.forwarding_preference = MoqtForwardingPreference::kTrack; - object_.group_id = 9; - object_.object_id = 10; - } - - void ExpandVarints() override { ExpandVarintsImpl("vvv", false); } - - private: - uint8_t raw_packet_[6] = { - 0x09, 0x0a, // object middler - 0x03, 0x62, 0x61, 0x72, // payload = "bar" - }; -}; - class QUICHE_NO_EXPORT StreamHeaderSubgroupMessage : public ObjectMessage { public: StreamHeaderSubgroupMessage() : ObjectMessage() { @@ -362,7 +319,6 @@ public: StreamMiddlerFetchMessage() : ObjectMessage() { SetWireImage(raw_packet_, sizeof(raw_packet_)); - object_.forwarding_preference = MoqtForwardingPreference::kTrack; object_.subgroup_id = 8; object_.object_id = 9; } @@ -1669,8 +1625,6 @@ switch (type) { case MoqtDataStreamType::kObjectDatagram: return std::make_unique<ObjectDatagramMessage>(); - case MoqtDataStreamType::kStreamHeaderTrack: - return std::make_unique<StreamHeaderTrackMessage>(); case MoqtDataStreamType::kStreamHeaderSubgroup: return std::make_unique<StreamHeaderSubgroupMessage>(); case MoqtDataStreamType::kStreamHeaderFetch: