Remove forwarding_preference from MoqtObject. This is a track property, so reporting the FP of an incoming Track by other means deletes a bunch of error cases. It also eliminates the ambiguity of what to report for FETCHed objects. PiperOrigin-RevId: 698891569
diff --git a/quiche/quic/moqt/moqt_framer.cc b/quiche/quic/moqt/moqt_framer.cc index 26a77dd..f4bbe47 100644 --- a/quiche/quic/moqt/moqt_framer.cc +++ b/quiche/quic/moqt/moqt_framer.cc
@@ -352,11 +352,6 @@ << "Object metadata is invalid"; return quiche::QuicheBuffer(); } - if (message.forwarding_preference != MoqtForwardingPreference::kDatagram) { - QUIC_BUG(quic_bug_serialize_object_datagram_02) - << "Only datagrams use SerializeObjectDatagram()"; - return quiche::QuicheBuffer(); - } if (message.payload_length != payload.length()) { QUIC_BUG(quic_bug_serialize_object_datagram_03) << "Payload length does not match payload";
diff --git a/quiche/quic/moqt/moqt_framer_test.cc b/quiche/quic/moqt/moqt_framer_test.cc index 8f93e2c..1a17f77 100644 --- a/quiche/quic/moqt/moqt_framer_test.cc +++ b/quiche/quic/moqt/moqt_framer_test.cc
@@ -89,7 +89,7 @@ MoqtObject adjusted_message = message; adjusted_message.payload_length = payload.size(); quiche::QuicheBuffer header = - (message.forwarding_preference == MoqtForwardingPreference::kDatagram) + (stream_type == MoqtDataStreamType::kObjectDatagram) ? framer.SerializeObjectDatagram(adjusted_message, payload) : framer.SerializeObjectHeader(adjusted_message, stream_type, is_first_in_stream); @@ -299,7 +299,6 @@ /*object_id=*/6, /*publisher_priority=*/7, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/8, /*payload_length=*/3, }; @@ -338,7 +337,6 @@ /*object_id=*/6, /*publisher_priority=*/7, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kDatagram, /*subgroup_id=*/std::nullopt, /*payload_length=*/3, }; @@ -375,7 +373,6 @@ /*object_id=*/6, /*publisher_priority=*/7, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kDatagram, /*subgroup_id=*/std::nullopt, /*payload_length=*/3, };
diff --git a/quiche/quic/moqt/moqt_integration_test.cc b/quiche/quic/moqt/moqt_integration_test.cc index 9218552..c7630a5 100644 --- a/quiche/quic/moqt/moqt_integration_test.cc +++ b/quiche/quic/moqt/moqt_integration_test.cc
@@ -204,17 +204,15 @@ [](FullTrackName, std::optional<MoqtAnnounceErrorReason>) {}); bool received_object = false; - EXPECT_CALL(server_visitor, OnObjectFragment(_, _, _, _, _, _, _)) + EXPECT_CALL(server_visitor, OnObjectFragment(_, _, _, _, _, _)) .WillOnce([&](const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority /*publisher_priority*/, - MoqtObjectStatus status, - MoqtForwardingPreference forwarding_preference, - absl::string_view object, bool end_of_message) { + MoqtObjectStatus status, absl::string_view object, + bool end_of_message) { EXPECT_EQ(full_track_name, FullTrackName("test", "data")); EXPECT_EQ(sequence.group, 0u); EXPECT_EQ(sequence.object, 0u); EXPECT_EQ(status, MoqtObjectStatus::kNormal); - EXPECT_EQ(forwarding_preference, MoqtForwardingPreference::kSubgroup); EXPECT_EQ(object, "object data"); EXPECT_TRUE(end_of_message); received_object = true; @@ -249,13 +247,13 @@ client_->session()->SubscribeCurrentGroup(FullTrackName("test", name), &client_visitor); int received = 0; - EXPECT_CALL(client_visitor, OnObjectFragment(_, FullSequence{1, 0}, _, - MoqtObjectStatus::kNormal, _, - "object 4", true)) + EXPECT_CALL(client_visitor, + OnObjectFragment(_, FullSequence{1, 0}, _, + MoqtObjectStatus::kNormal, "object 4", true)) .WillOnce([&] { ++received; }); - EXPECT_CALL(client_visitor, OnObjectFragment(_, FullSequence{1, 1}, _, - MoqtObjectStatus::kNormal, _, - "object 5", true)) + EXPECT_CALL(client_visitor, + OnObjectFragment(_, FullSequence{1, 1}, _, + MoqtObjectStatus::kNormal, "object 5", true)) .WillOnce([&] { ++received; }); bool success = test_harness_.RunUntilWithDefaultTimeout( [&]() { return received >= 2; }); @@ -264,21 +262,21 @@ queue->AddObject(MemSliceFromString("object 6"), /*key=*/false); queue->AddObject(MemSliceFromString("object 7"), /*key=*/true); queue->AddObject(MemSliceFromString("object 8"), /*key=*/false); - EXPECT_CALL(client_visitor, OnObjectFragment(_, FullSequence{1, 2}, _, - MoqtObjectStatus::kNormal, _, - "object 6", true)) + EXPECT_CALL(client_visitor, + OnObjectFragment(_, FullSequence{1, 2}, _, + MoqtObjectStatus::kNormal, "object 6", true)) .WillOnce([&] { ++received; }); EXPECT_CALL(client_visitor, OnObjectFragment(_, FullSequence{1, 3}, _, - MoqtObjectStatus::kEndOfGroup, _, "", true)) + MoqtObjectStatus::kEndOfGroup, "", true)) .WillOnce([&] { ++received; }); - EXPECT_CALL(client_visitor, OnObjectFragment(_, FullSequence{2, 0}, _, - MoqtObjectStatus::kNormal, _, - "object 7", true)) + EXPECT_CALL(client_visitor, + OnObjectFragment(_, FullSequence{2, 0}, _, + MoqtObjectStatus::kNormal, "object 7", true)) .WillOnce([&] { ++received; }); - EXPECT_CALL(client_visitor, OnObjectFragment(_, FullSequence{2, 1}, _, - MoqtObjectStatus::kNormal, _, - "object 8", true)) + EXPECT_CALL(client_visitor, + OnObjectFragment(_, FullSequence{2, 1}, _, + MoqtObjectStatus::kNormal, "object 8", true)) .WillOnce([&] { ++received; }); success = test_harness_.RunUntilWithDefaultTimeout( [&]() { return received >= 6; }); @@ -312,35 +310,35 @@ int received = 0; // Those won't arrive since they have expired. EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{0, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{0, 0}, _, _, _, true)) .Times(0); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{0, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{0, 0}, _, _, _, true)) .Times(0); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{96, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{96, 0}, _, _, _, true)) .Times(0); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{96, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{96, 0}, _, _, _, true)) .Times(0); // Those are within the "last three groups" window. EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{97, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{97, 0}, _, _, _, true)) .WillOnce([&] { ++received; }); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{97, 1}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{97, 1}, _, _, _, true)) .WillOnce([&] { ++received; }); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{98, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{98, 0}, _, _, _, true)) .WillOnce([&] { ++received; }); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{98, 1}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{98, 1}, _, _, _, true)) .WillOnce([&] { ++received; }); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{99, 0}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{99, 0}, _, _, _, true)) .WillOnce([&] { ++received; }); EXPECT_CALL(client_visitor, - OnObjectFragment(_, FullSequence{99, 1}, _, _, _, _, true)) + OnObjectFragment(_, FullSequence{99, 1}, _, _, _, true)) .Times(0); // The current group should not be closed yet. bool success = test_harness_.RunUntilWithDefaultTimeout( [&]() { return received >= 5; });
diff --git a/quiche/quic/moqt/moqt_messages.h b/quiche/quic/moqt/moqt_messages.h index 680ebf0..6239299 100644 --- a/quiche/quic/moqt/moqt_messages.h +++ b/quiche/quic/moqt/moqt_messages.h
@@ -328,7 +328,6 @@ uint64_t object_id; MoqtPriority publisher_priority; MoqtObjectStatus object_status; - MoqtForwardingPreference forwarding_preference; std::optional<uint64_t> subgroup_id; uint64_t payload_length; };
diff --git a/quiche/quic/moqt/moqt_parser.cc b/quiche/quic/moqt/moqt_parser.cc index f643767..a5e0da0 100644 --- a/quiche/quic/moqt/moqt_parser.cc +++ b/quiche/quic/moqt/moqt_parser.cc
@@ -1078,7 +1078,6 @@ if (processed_data == 0) { // Incomplete header return absl::string_view(); } - object_metadata.forwarding_preference = MoqtForwardingPreference::kDatagram; return reader.PeekRemainingPayload(); } @@ -1153,7 +1152,6 @@ if (bytes_read == 0) { return remainder; } - header.forwarding_preference = MoqtForwardingPreference::kSubgroup; metadata_ = header; continue; }
diff --git a/quiche/quic/moqt/moqt_session.cc b/quiche/quic/moqt/moqt_session.cc index 93da87b..0bf32bf 100644 --- a/quiche/quic/moqt/moqt_session.cc +++ b/quiche/quic/moqt/moqt_session.cc
@@ -198,12 +198,12 @@ << message.object_id << " priority " << message.publisher_priority << " length " << payload.size(); - auto [full_track_name, visitor] = TrackPropertiesFromAlias(message); + auto [full_track_name, visitor] = + TrackPropertiesFromAlias(message, MoqtForwardingPreference::kDatagram); if (visitor != nullptr) { visitor->OnObjectFragment( full_track_name, FullSequence{message.group_id, 0, message.object_id}, - message.publisher_priority, message.object_status, - message.forwarding_preference, payload, true); + message.publisher_priority, message.object_status, payload, true); } } @@ -556,7 +556,9 @@ } std::pair<FullTrackName, RemoteTrack::Visitor*> -MoqtSession::TrackPropertiesFromAlias(const MoqtObject& message) { +MoqtSession::TrackPropertiesFromAlias( + const MoqtObject& message, + std::optional<MoqtForwardingPreference> forwarding_preference) { auto it = remote_tracks_.find(message.track_alias); if (it == remote_tracks_.end()) { ActiveSubscribe* subscribe = nullptr; @@ -574,21 +576,28 @@ {FullTrackName{}, nullptr}); } subscribe->received_object = true; - if (subscribe->forwarding_preference.has_value()) { - if (message.forwarding_preference != *subscribe->forwarding_preference) { - Error(MoqtError::kProtocolViolation, - "Forwarding preference changes mid-track"); - return std::pair<FullTrackName, RemoteTrack::Visitor*>( - {FullTrackName{}, nullptr}); + if (forwarding_preference.has_value()) { + if (subscribe->forwarding_preference.has_value()) { + if (*forwarding_preference != *subscribe->forwarding_preference) { + Error(MoqtError::kProtocolViolation, + "Forwarding preference changes mid-track"); + return std::pair<FullTrackName, RemoteTrack::Visitor*>( + {FullTrackName{}, nullptr}); + } + } else { + subscribe->forwarding_preference = *forwarding_preference; } } else { - subscribe->forwarding_preference = message.forwarding_preference; + QUICHE_BUG(quic_subscribe_no_forwarding preference) + << "Objects from a subscribe should know the forwarding preference"; } return std::make_pair(subscribe->message.full_track_name, subscribe->visitor); } RemoteTrack& track = it->second; - if (!track.CheckForwardingPreference(message.forwarding_preference)) { + // Update the forwarding preference if it is present. + if (forwarding_preference.has_value() && + !track.CheckForwardingPreference(*forwarding_preference)) { // Incorrect forwarding preference. Error(MoqtError::kProtocolViolation, "Forwarding preference changes mid-track"); @@ -1058,9 +1067,6 @@ << message.track_alias << " with sequence " << message.group_id << ":" << message.object_id << " priority " << message.publisher_priority - << " forwarding_preference " - << MoqtForwardingPreferenceToString( - message.forwarding_preference) << " length " << payload.size() << " length " << message.payload_length << (end_of_message ? "F" : ""); if (!session_->parameters_.deliver_partial_objects) { @@ -1078,14 +1084,15 @@ payload = absl::string_view(partial_object_); } } - auto [full_track_name, visitor] = session_->TrackPropertiesFromAlias(message); + auto [full_track_name, visitor] = session_->TrackPropertiesFromAlias( + message, MoqtForwardingPreference::kSubgroup); if (visitor != nullptr) { visitor->OnObjectFragment( full_track_name, FullSequence{message.group_id, message.subgroup_id.value_or(0), message.object_id}, - message.publisher_priority, message.object_status, - message.forwarding_preference, payload, end_of_message); + message.publisher_priority, message.object_status, payload, + end_of_message); } partial_object_.clear(); } @@ -1514,7 +1521,6 @@ header.object_id = object->sequence.object; header.publisher_priority = track_publisher_->GetPublisherPriority(); header.object_status = object->status; - header.forwarding_preference = MoqtForwardingPreference::kDatagram; header.subgroup_id = std::nullopt; header.payload_length = object->payload.length(); quiche::QuicheBuffer datagram = session_->framer_.SerializeObjectDatagram(
diff --git a/quiche/quic/moqt/moqt_session.h b/quiche/quic/moqt/moqt_session.h index ffae8d7..c4ae794 100644 --- a/quiche/quic/moqt/moqt_session.h +++ b/quiche/quic/moqt/moqt_session.h
@@ -509,9 +509,12 @@ [[nodiscard]] bool OpenDataStream(std::shared_ptr<PublishedFetch> fetch); // Get FullTrackName and visitor for a subscribe_id and track_alias. Returns - // an empty FullTrackName tuple and nullptr if not present. + // an empty FullTrackName tuple and nullptr if not present. If the caller has + // information about the track's forwarding preference, it can be passed via + // |forwarding_preference| so that it can be stored in RemoteTrack. std::pair<FullTrackName, RemoteTrack::Visitor*> TrackPropertiesFromAlias( - const MoqtObject& message); + const MoqtObject& message, + std::optional<MoqtForwardingPreference> forwarding_preference); // Checks that a subscribe ID from a SUBSCRIBE or FETCH is valid, and throws // a session error if is not.
diff --git a/quiche/quic/moqt/moqt_session_test.cc b/quiche/quic/moqt/moqt_session_test.cc index de654f5..844b4d5 100644 --- a/quiche/quic/moqt/moqt_session_test.cc +++ b/quiche/quic/moqt/moqt_session_test.cc
@@ -931,7 +931,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/8, }; @@ -939,7 +938,7 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _)).Times(1); EXPECT_CALL(mock_stream, GetStreamId()) .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, true); @@ -956,7 +955,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/16, }; @@ -964,7 +962,7 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _)).Times(1); EXPECT_CALL(mock_stream, GetStreamId()) .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, false); @@ -986,7 +984,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/16, }; @@ -994,7 +991,7 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session, &mock_stream); - EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _, _)).Times(2); + EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _)).Times(2); EXPECT_CALL(mock_stream, GetStreamId()) .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, false); @@ -1023,7 +1020,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/8, }; @@ -1031,10 +1027,9 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _, _)) + EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _)) .WillOnce([&](const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority publisher_priority, MoqtObjectStatus status, - MoqtForwardingPreference forwarding_preference, absl::string_view payload, bool end_of_message) { EXPECT_EQ(full_track_name, ftn); EXPECT_EQ(sequence.group, object.group_id); @@ -1080,7 +1075,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/8, }; @@ -1088,10 +1082,9 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor, OnObjectFragment(_, _, _, _, _, _, _)) + EXPECT_CALL(visitor, OnObjectFragment(_, _, _, _, _, _)) .WillOnce([&](const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority publisher_priority, MoqtObjectStatus status, - MoqtForwardingPreference forwarding_preference, absl::string_view payload, bool end_of_message) { EXPECT_EQ(full_track_name, ftn); EXPECT_EQ(sequence.group, object.group_id); @@ -1140,7 +1133,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/8, }; @@ -1148,10 +1140,9 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor, OnObjectFragment(_, _, _, _, _, _, _)) + EXPECT_CALL(visitor, OnObjectFragment(_, _, _, _, _, _)) .WillOnce([&](const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority publisher_priority, MoqtObjectStatus status, - MoqtForwardingPreference forwarding_preference, absl::string_view payload, bool end_of_message) { EXPECT_EQ(full_track_name, ftn); EXPECT_EQ(sequence.group, object.group_id); @@ -1160,13 +1151,13 @@ EXPECT_CALL(mock_stream, GetStreamId()) .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, true); - object.forwarding_preference = MoqtForwardingPreference::kDatagram; - ++object.object_id; + char datagram[] = {0x01, 0x02, 0x00, 0x00, 0x00, 0x08, 0x64, + 0x65, 0x61, 0x64, 0x62, 0x65, 0x65, 0x66}; EXPECT_CALL(mock_session_, CloseSession(static_cast<uint64_t>(MoqtError::kProtocolViolation), "Forwarding preference changes mid-track")) .Times(1); - object_stream->OnObjectMessage(object, payload, true); + session_.OnDatagramReceived(absl::string_view(datagram, sizeof(datagram))); } TEST_F(MoqtSessionTest, EarlyObjectForwardingDoesNotMatchTrack) { @@ -1191,7 +1182,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/8, }; @@ -1199,10 +1189,9 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor, OnObjectFragment(_, _, _, _, _, _, _)) + EXPECT_CALL(visitor, OnObjectFragment(_, _, _, _, _, _)) .WillOnce([&](const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority publisher_priority, MoqtObjectStatus status, - MoqtForwardingPreference forwarding_preference, absl::string_view payload, bool end_of_message) { EXPECT_EQ(full_track_name, ftn); EXPECT_EQ(sequence.group, object.group_id); @@ -1761,7 +1750,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kDatagram, /*subgroup_id=*/std::nullopt, /*payload_length=*/8, }; @@ -1770,8 +1758,8 @@ EXPECT_CALL( visitor_, OnObjectFragment(ftn, FullSequence{object.group_id, object.object_id}, - object.publisher_priority, object.object_status, - object.forwarding_preference, payload, true)) + object.publisher_priority, object.object_status, payload, + true)) .Times(1); session_.OnDatagramReceived(absl::string_view(datagram, sizeof(datagram))); } @@ -1787,7 +1775,6 @@ /*object_sequence=*/0, /*publisher_priority=*/0, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/0, /*payload_length=*/8, }; @@ -1795,17 +1782,17 @@ std::unique_ptr<MoqtDataParserVisitor> object_stream = MoqtSessionPeer::CreateIncomingDataStream(&session_, &mock_stream); - EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _, _)).Times(1); + EXPECT_CALL(visitor_, OnObjectFragment(_, _, _, _, _, _)).Times(1); EXPECT_CALL(mock_stream, GetStreamId()) .WillRepeatedly(Return(kIncomingUniStreamId)); object_stream->OnObjectMessage(object, payload, true); - ++object.object_id; - object.forwarding_preference = MoqtForwardingPreference::kDatagram; + char datagram[] = {0x01, 0x02, 0x00, 0x10, 0x00, 0x08, 0x64, + 0x65, 0x61, 0x64, 0x62, 0x65, 0x65, 0x66}; EXPECT_CALL(mock_session_, CloseSession(static_cast<uint64_t>(MoqtError::kProtocolViolation), "Forwarding preference changes mid-track")) .Times(1); - object_stream->OnObjectMessage(object, payload, true); + session_.OnDatagramReceived(absl::string_view(datagram, sizeof(datagram))); } TEST_F(MoqtSessionTest, AnnounceToPublisher) {
diff --git a/quiche/quic/moqt/moqt_track.h b/quiche/quic/moqt/moqt_track.h index 923fa37..0da047a 100644 --- a/quiche/quic/moqt/moqt_track.h +++ b/quiche/quic/moqt/moqt_track.h
@@ -40,7 +40,6 @@ virtual void OnObjectFragment( const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority publisher_priority, MoqtObjectStatus object_status, - MoqtForwardingPreference forwarding_preference, absl::string_view object, bool end_of_message) = 0; // TODO(martinduke): Add final sequence numbers }; @@ -62,6 +61,10 @@ // that the preference is consistent. bool CheckForwardingPreference(MoqtForwardingPreference preference); + std::optional<MoqtForwardingPreference> forwarding_preference() const { + return forwarding_preference_; + } + private: // TODO: There is no accounting for the number of outstanding subscribes, // because we can't match track names to individual subscribes.
diff --git a/quiche/quic/moqt/test_tools/moqt_test_message.h b/quiche/quic/moqt/test_tools/moqt_test_message.h index 0fc7d7a..9488f63 100644 --- a/quiche/quic/moqt/test_tools/moqt_test_message.h +++ b/quiche/quic/moqt/test_tools/moqt_test_message.h
@@ -184,10 +184,6 @@ QUIC_LOG(INFO) << "OBJECT Object Status mismatch"; return false; } - if (cast.forwarding_preference != object_.forwarding_preference) { - QUIC_LOG(INFO) << "OBJECT Object Send Order mismatch"; - return false; - } if (cast.subgroup_id != object_.subgroup_id) { QUIC_LOG(INFO) << "OBJECT Subgroup ID mismatch"; return false; @@ -210,7 +206,6 @@ /*object_id=*/6, /*publisher_priority=*/7, /*object_status=*/MoqtObjectStatus::kNormal, - /*forwarding_preference=*/MoqtForwardingPreference::kSubgroup, /*subgroup_id=*/std::nullopt, /*payload_length=*/3, }; @@ -220,7 +215,6 @@ public: ObjectDatagramMessage() : ObjectMessage() { SetWireImage(raw_packet_, sizeof(raw_packet_)); - object_.forwarding_preference = MoqtForwardingPreference::kDatagram; } void ExpandVarints() override { ExpandVarintsImpl("vvvv-v---", false); } @@ -239,7 +233,6 @@ public: StreamHeaderSubgroupMessage() : ObjectMessage() { SetWireImage(raw_packet_, sizeof(raw_packet_)); - object_.forwarding_preference = MoqtForwardingPreference::kSubgroup; object_.subgroup_id = 8; } @@ -270,7 +263,6 @@ public: StreamMiddlerSubgroupMessage() : ObjectMessage() { SetWireImage(raw_packet_, sizeof(raw_packet_)); - object_.forwarding_preference = MoqtForwardingPreference::kSubgroup; object_.subgroup_id = 8; object_.object_id = 9; }
diff --git a/quiche/quic/moqt/tools/chat_client.cc b/quiche/quic/moqt/tools/chat_client.cc index 13e3497..28c93c1 100644 --- a/quiche/quic/moqt/tools/chat_client.cc +++ b/quiche/quic/moqt/tools/chat_client.cc
@@ -126,7 +126,6 @@ void ChatClient::RemoteTrackVisitor::OnObjectFragment( const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority /*publisher_priority*/, MoqtObjectStatus /*status*/, - MoqtForwardingPreference /*forwarding_preference*/, absl::string_view object, bool end_of_message) { if (!end_of_message) { std::cerr << "Error: received partial message despite requesting "
diff --git a/quiche/quic/moqt/tools/chat_client.h b/quiche/quic/moqt/tools/chat_client.h index fe64809..024b78d 100644 --- a/quiche/quic/moqt/tools/chat_client.h +++ b/quiche/quic/moqt/tools/chat_client.h
@@ -101,7 +101,6 @@ FullSequence sequence, moqt::MoqtPriority publisher_priority, moqt::MoqtObjectStatus status, - moqt::MoqtForwardingPreference forwarding_preference, absl::string_view object, bool end_of_message) override;
diff --git a/quiche/quic/moqt/tools/chat_server.cc b/quiche/quic/moqt/tools/chat_server.cc index d098450..c4f0327 100644 --- a/quiche/quic/moqt/tools/chat_server.cc +++ b/quiche/quic/moqt/tools/chat_server.cc
@@ -93,7 +93,6 @@ void ChatServer::RemoteTrackVisitor::OnObjectFragment( const moqt::FullTrackName& full_track_name, moqt::FullSequence sequence, moqt::MoqtPriority /*publisher_priority*/, moqt::MoqtObjectStatus status, - moqt::MoqtForwardingPreference /*forwarding_preference*/, absl::string_view object, bool end_of_message) { if (!end_of_message) { std::cerr << "Error: received partial message despite requesting "
diff --git a/quiche/quic/moqt/tools/chat_server.h b/quiche/quic/moqt/tools/chat_server.h index f1b2ae1..1af6e5c 100644 --- a/quiche/quic/moqt/tools/chat_server.h +++ b/quiche/quic/moqt/tools/chat_server.h
@@ -45,7 +45,6 @@ const moqt::FullTrackName& full_track_name, FullSequence sequence, moqt::MoqtPriority /*publisher_priority*/, moqt::MoqtObjectStatus /*status*/, - moqt::MoqtForwardingPreference /*forwarding_preference*/, absl::string_view object, bool end_of_message) override; private:
diff --git a/quiche/quic/moqt/tools/moqt_ingestion_server_bin.cc b/quiche/quic/moqt/tools/moqt_ingestion_server_bin.cc index ddb51e6..f31fff4 100644 --- a/quiche/quic/moqt/tools/moqt_ingestion_server_bin.cc +++ b/quiche/quic/moqt/tools/moqt_ingestion_server_bin.cc
@@ -180,7 +180,6 @@ FullSequence sequence, MoqtPriority /*publisher_priority*/, MoqtObjectStatus /*status*/, - MoqtForwardingPreference /*forwarding_preference*/, absl::string_view object, bool /*end_of_message*/) override { std::string file_name = absl::StrCat(sequence.group, "-", sequence.object,
diff --git a/quiche/quic/moqt/tools/moqt_mock_visitor.h b/quiche/quic/moqt/tools/moqt_mock_visitor.h index 5a5a909..87e3541 100644 --- a/quiche/quic/moqt/tools/moqt_mock_visitor.h +++ b/quiche/quic/moqt/tools/moqt_mock_visitor.h
@@ -87,7 +87,6 @@ MOCK_METHOD(void, OnObjectFragment, (const FullTrackName& full_track_name, FullSequence sequence, MoqtPriority publisher_priority, MoqtObjectStatus status, - MoqtForwardingPreference forwarding_preference, absl::string_view object, bool end_of_message), (override)); };
diff --git a/quiche/quic/moqt/tools/moqt_simulator_bin.cc b/quiche/quic/moqt/tools/moqt_simulator_bin.cc index 2d260ca..39cf2c8 100644 --- a/quiche/quic/moqt/tools/moqt_simulator_bin.cc +++ b/quiche/quic/moqt/tools/moqt_simulator_bin.cc
@@ -205,7 +205,6 @@ FullSequence sequence, MoqtPriority /*publisher_priority*/, MoqtObjectStatus status, - MoqtForwardingPreference /*forwarding_preference*/, absl::string_view object, bool end_of_message) override { QUICHE_DCHECK(full_track_name == TrackName());