Rename fields in MoQT SUBSCRIBE_OK to match draft-11. PiperOrigin-RevId: 757871040
diff --git a/quiche/quic/moqt/moqt_framer.cc b/quiche/quic/moqt/moqt_framer.cc index 21588c9..7578166 100644 --- a/quiche/quic/moqt/moqt_framer.cc +++ b/quiche/quic/moqt/moqt_framer.cc
@@ -482,17 +482,17 @@ << "Serializing invalid MoQT parameters"; return quiche::QuicheBuffer(); } - if (message.largest_id.has_value()) { + if (message.largest_location.has_value()) { return SerializeControlMessage( - MoqtMessageType::kSubscribeOk, WireVarInt62(message.subscribe_id), + MoqtMessageType::kSubscribeOk, WireVarInt62(message.request_id), WireVarInt62(message.expires.ToMilliseconds()), WireDeliveryOrder(message.group_order), WireUint8(1), - WireVarInt62(message.largest_id->group), - WireVarInt62(message.largest_id->object), + WireVarInt62(message.largest_location->group), + WireVarInt62(message.largest_location->object), WireKeyValuePairList(parameters)); } return SerializeControlMessage( - MoqtMessageType::kSubscribeOk, WireVarInt62(message.subscribe_id), + MoqtMessageType::kSubscribeOk, WireVarInt62(message.request_id), WireVarInt62(message.expires.ToMilliseconds()), WireDeliveryOrder(message.group_order), WireUint8(0), WireKeyValuePairList(parameters));
diff --git a/quiche/quic/moqt/moqt_live_relay_queue.cc b/quiche/quic/moqt/moqt_live_relay_queue.cc index caab1e5..c50caa5 100644 --- a/quiche/quic/moqt/moqt_live_relay_queue.cc +++ b/quiche/quic/moqt/moqt_live_relay_queue.cc
@@ -275,7 +275,7 @@ return MoqtTrackStatusCode::kInProgress; } -Location MoqtLiveRelayQueue::GetLargestSequence() const { +Location MoqtLiveRelayQueue::GetLargestLocation() const { return Location{next_sequence_.group, next_sequence_.object - 1}; }
diff --git a/quiche/quic/moqt/moqt_live_relay_queue.h b/quiche/quic/moqt/moqt_live_relay_queue.h index 3a5faf0..0ecb671 100644 --- a/quiche/quic/moqt/moqt_live_relay_queue.h +++ b/quiche/quic/moqt/moqt_live_relay_queue.h
@@ -92,7 +92,7 @@ listeners_.erase(listener); } absl::StatusOr<MoqtTrackStatusCode> GetTrackStatus() const override; - Location GetLargestSequence() const override; + Location GetLargestLocation() const override; MoqtForwardingPreference GetForwardingPreference() const override { return forwarding_preference_; }
diff --git a/quiche/quic/moqt/moqt_live_relay_queue_test.cc b/quiche/quic/moqt/moqt_live_relay_queue_test.cc index d5ed3ff..6115acb 100644 --- a/quiche/quic/moqt/moqt_live_relay_queue_test.cc +++ b/quiche/quic/moqt/moqt_live_relay_queue_test.cc
@@ -66,7 +66,7 @@ void GetObjectsFromPast(const SubscribeWindow& window) { std::vector<Location> objects = - GetCachedObjectsInRange(Location(0, 0), GetLargestSequence()); + GetCachedObjectsInRange(Location(0, 0), GetLargestLocation()); for (Location object : objects) { if (window.InWindow(object)) { OnNewObjectAvailable(object);
diff --git a/quiche/quic/moqt/moqt_messages.h b/quiche/quic/moqt/moqt_messages.h index 9ba07f8..fc525c2 100644 --- a/quiche/quic/moqt/moqt_messages.h +++ b/quiche/quic/moqt/moqt_messages.h
@@ -538,12 +538,12 @@ }; struct QUICHE_EXPORT MoqtSubscribeOk { - uint64_t subscribe_id; + uint64_t request_id; // The message uses ms, but expires is in us. quic::QuicTimeDelta expires = quic::QuicTimeDelta::FromMilliseconds(0); MoqtDeliveryOrder group_order; // If ContextExists on the wire is zero, largest_id has no value. - std::optional<Location> largest_id; + std::optional<Location> largest_location; VersionSpecificParameters parameters; };
diff --git a/quiche/quic/moqt/moqt_outgoing_queue.cc b/quiche/quic/moqt/moqt_outgoing_queue.cc index 0b57ca4..1c24cde 100644 --- a/quiche/quic/moqt/moqt_outgoing_queue.cc +++ b/quiche/quic/moqt/moqt_outgoing_queue.cc
@@ -118,10 +118,10 @@ return MoqtTrackStatusCode::kInProgress; } -Location MoqtOutgoingQueue::GetLargestSequence() const { +Location MoqtOutgoingQueue::GetLargestLocation() const { if (queue_.empty()) { - QUICHE_BUG(MoqtOutgoingQueue_GetLargestSequence_not_begun) - << "Calling GetLargestSequence() on a track that hasn't begun"; + QUICHE_BUG(MoqtOutgoingQueue_GetLargestLocation_not_begun) + << "Calling GetLargestLocation() on a track that hasn't begun"; return Location{0, 0}; } return Location{current_group_id_, queue_.back().size() - 1};
diff --git a/quiche/quic/moqt/moqt_outgoing_queue.h b/quiche/quic/moqt/moqt_outgoing_queue.h index af480ca..e8391be 100644 --- a/quiche/quic/moqt/moqt_outgoing_queue.h +++ b/quiche/quic/moqt/moqt_outgoing_queue.h
@@ -66,7 +66,7 @@ listeners_.erase(listener); } absl::StatusOr<MoqtTrackStatusCode> GetTrackStatus() const override; - Location GetLargestSequence() const override; + Location GetLargestLocation() const override; MoqtForwardingPreference GetForwardingPreference() const override { return forwarding_preference_; }
diff --git a/quiche/quic/moqt/moqt_outgoing_queue_test.cc b/quiche/quic/moqt/moqt_outgoing_queue_test.cc index 028be69..a648a42 100644 --- a/quiche/quic/moqt/moqt_outgoing_queue_test.cc +++ b/quiche/quic/moqt/moqt_outgoing_queue_test.cc
@@ -61,7 +61,7 @@ void GetObjectsFromPast(const SubscribeWindow& window) { std::vector<Location> objects = - GetCachedObjectsInRange(Location(0, 0), GetLargestSequence()); + GetCachedObjectsInRange(Location(0, 0), GetLargestLocation()); for (Location object : objects) { if (window.InWindow(object)) { OnNewObjectAvailable(object);
diff --git a/quiche/quic/moqt/moqt_parser.cc b/quiche/quic/moqt/moqt_parser.cc index e9a093b..c84fa9f 100644 --- a/quiche/quic/moqt/moqt_parser.cc +++ b/quiche/quic/moqt/moqt_parser.cc
@@ -472,7 +472,7 @@ uint64_t milliseconds; uint8_t group_order; uint8_t content_exists; - if (!reader.ReadVarInt62(&subscribe_ok.subscribe_id) || + if (!reader.ReadVarInt62(&subscribe_ok.request_id) || !reader.ReadVarInt62(&milliseconds) || !reader.ReadUInt8(&group_order) || !reader.ReadUInt8(&content_exists)) { return 0; @@ -488,9 +488,9 @@ subscribe_ok.expires = quic::QuicTimeDelta::FromMilliseconds(milliseconds); subscribe_ok.group_order = static_cast<MoqtDeliveryOrder>(group_order); if (content_exists) { - subscribe_ok.largest_id = Location(); - if (!reader.ReadVarInt62(&subscribe_ok.largest_id->group) || - !reader.ReadVarInt62(&subscribe_ok.largest_id->object)) { + subscribe_ok.largest_location = Location(); + if (!reader.ReadVarInt62(&subscribe_ok.largest_location->group) || + !reader.ReadVarInt62(&subscribe_ok.largest_location->object)) { return 0; } }
diff --git a/quiche/quic/moqt/moqt_publisher.h b/quiche/quic/moqt/moqt_publisher.h index 77d85d7..1123d93 100644 --- a/quiche/quic/moqt/moqt_publisher.h +++ b/quiche/quic/moqt/moqt_publisher.h
@@ -166,10 +166,10 @@ virtual absl::StatusOr<MoqtTrackStatusCode> GetTrackStatus() const = 0; - // Returns the largest sequence pair that has been published so far. + // Returns the largest (group, object) pair that has been published so far. // This method may only be called if // DoesTrackStatusImplyHavingData(GetTrackStatus()) is true. - virtual Location GetLargestSequence() const = 0; + virtual Location GetLargestLocation() const = 0; // Returns the forwarding preference of the track. // This method may only be called if
diff --git a/quiche/quic/moqt/moqt_session.cc b/quiche/quic/moqt/moqt_session.cc index 7ea870d..9a0b499 100644 --- a/quiche/quic/moqt/moqt_session.cc +++ b/quiche/quic/moqt/moqt_session.cc
@@ -988,10 +988,10 @@ void MoqtSession::ControlStream::OnSubscribeOkMessage( const MoqtSubscribeOk& message) { - RemoteTrack* track = session_->RemoteTrackById(message.subscribe_id); + RemoteTrack* track = session_->RemoteTrackById(message.request_id); if (track == nullptr) { QUIC_DLOG(INFO) << ENDPOINT << "Received the SUBSCRIBE_OK for " - << "subscribe_id = " << message.subscribe_id + << "subscribe_id = " << message.request_id << " but no track exists"; // Subscription state might have been destroyed for internal reasons. return; @@ -1001,25 +1001,25 @@ "Received SUBSCRIBE_OK for a FETCH"); return; } - if (message.largest_id.has_value()) { + if (message.largest_location.has_value()) { QUIC_DLOG(INFO) << ENDPOINT << "Received the SUBSCRIBE_OK for " - << "subscribe_id = " << message.subscribe_id << " " + << "subscribe_id = " << message.request_id << " " << track->full_track_name() - << " largest_id = " << *message.largest_id; + << " largest_id = " << *message.largest_location; } else { QUIC_DLOG(INFO) << ENDPOINT << "Received the SUBSCRIBE_OK for " - << "subscribe_id = " << message.subscribe_id << " " + << "subscribe_id = " << message.request_id << " " << track->full_track_name(); } SubscribeRemoteTrack* subscribe = static_cast<SubscribeRemoteTrack*>(track); subscribe->OnObjectOrOk(); // TODO(martinduke): Handle expires field. - if (message.largest_id.has_value()) { - subscribe->TruncateStart(message.largest_id->next()); + if (message.largest_location.has_value()) { + subscribe->TruncateStart(message.largest_location->next()); } if (subscribe->visitor() != nullptr) { - subscribe->visitor()->OnReply(track->full_track_name(), message.largest_id, - std::nullopt); + subscribe->visitor()->OnReply(track->full_track_name(), + message.largest_location, std::nullopt); } } @@ -1801,18 +1801,18 @@ }; void MoqtSession::PublishedSubscription::OnSubscribeAccepted() { - std::optional<Location> largest_id; + std::optional<Location> largest_location; ControlStream* stream = session_->GetControlStream(); if (PublisherHasData(*track_publisher_)) { - largest_id = track_publisher_->GetLargestSequence(); - QUICHE_CHECK(largest_id.has_value()); + largest_location = track_publisher_->GetLargestLocation(); + QUICHE_CHECK(largest_location.has_value()); if (forward_) { switch (filter_type_) { case MoqtFilterType::kLatestObject: - window_ = SubscribeWindow(largest_id->next()); + window_ = SubscribeWindow(largest_location->next()); break; case MoqtFilterType::kNextGroupStart: - window_ = SubscribeWindow(Location(largest_id->group + 1, 0)); + window_ = SubscribeWindow(Location(largest_location->group + 1, 0)); break; default: break; @@ -1824,9 +1824,9 @@ window_ = SubscribeWindow(Location(0, 0)); } MoqtSubscribeOk subscribe_ok; - subscribe_ok.subscribe_id = request_id_; + subscribe_ok.request_id = request_id_; subscribe_ok.group_order = track_publisher_->GetDeliveryOrder(); - subscribe_ok.largest_id = largest_id; + subscribe_ok.largest_location = largest_location; // TODO(martinduke): Support sending DELIVERY_TIMEOUT parameter as the // publisher. stream->SendOrBufferMessage(
diff --git a/quiche/quic/moqt/moqt_session_test.cc b/quiche/quic/moqt/moqt_session_test.cc index 7e1b8df..70c6a24 100644 --- a/quiche/quic/moqt/moqt_session_test.cc +++ b/quiche/quic/moqt/moqt_session_test.cc
@@ -99,7 +99,7 @@ .WillByDefault(Return(MoqtTrackStatusCode::kInProgress)); ON_CALL(*publisher, GetForwardingPreference()) .WillByDefault(Return(forwarding_preference)); - ON_CALL(*publisher, GetLargestSequence()) + ON_CALL(*publisher, GetLargestLocation()) .WillByDefault(Return(largest_sequence)); return publisher; } @@ -137,7 +137,7 @@ void SetLargestId(MockTrackPublisher* publisher, Location largest_id) { ON_CALL(*publisher, GetTrackStatus()) .WillByDefault(Return(MoqtTrackStatusCode::kInProgress)); - ON_CALL(*publisher, GetLargestSequence()).WillByDefault(Return(largest_id)); + ON_CALL(*publisher, GetLargestLocation()).WillByDefault(Return(largest_id)); } // The publisher receives SUBSCRIBE and synchronously announces it will @@ -161,7 +161,7 @@ /*expires=*/quic::QuicTimeDelta::FromMilliseconds(0), /*group_order=*/MoqtDeliveryOrder::kAscending, (*track_status == MoqtTrackStatusCode::kInProgress) - ? std::make_optional(publisher->GetLargestSequence()) + ? std::make_optional(publisher->GetLargestLocation()) : std::optional<Location>(), /*parameters=*/VersionSpecificParameters(), }; @@ -689,7 +689,7 @@ }; EXPECT_CALL(remote_track_visitor, OnReply(_, _, _)) .WillOnce([&](const FullTrackName& ftn, - std::optional<Location> /*largest_id*/, + std::optional<Location> /*largest_location*/, std::optional<absl::string_view> error_message) { EXPECT_EQ(ftn, FullTrackName("foo", "bar")); EXPECT_FALSE(error_message.has_value()); @@ -726,7 +726,7 @@ }; EXPECT_CALL(remote_track_visitor, OnReply(_, _, _)) .WillOnce([&](const FullTrackName& ftn, - std::optional<Location> /*largest_id*/, + std::optional<Location> /*largest_location*/, std::optional<absl::string_view> error_message) { EXPECT_EQ(ftn, FullTrackName("foo", "bar")); EXPECT_FALSE(error_message.has_value()); @@ -1087,7 +1087,7 @@ /*request_id=*/1, /*expires=*/quic::QuicTimeDelta::FromMilliseconds(0), /*group_order=*/MoqtDeliveryOrder::kAscending, - /*largest_id=*/std::nullopt, + /*largest_location=*/std::nullopt, }; webtransport::test::MockStream mock_control_stream; std::unique_ptr<MoqtControlParserVisitor> control_stream = @@ -3204,7 +3204,7 @@ /*request_id=*/0, /*expires=*/quic::QuicTimeDelta::FromMilliseconds(10000), /*group_order=*/MoqtDeliveryOrder::kAscending, - /*largest_id=*/std::nullopt, + /*largest_location=*/std::nullopt, /*parameters=*/VersionSpecificParameters(), }; stream_input->OnSubscribeOkMessage(ok); @@ -3262,7 +3262,7 @@ /*request_id=*/0, /*expires=*/quic::QuicTimeDelta::FromMilliseconds(10000), /*group_order=*/MoqtDeliveryOrder::kAscending, - /*largest_id=*/std::nullopt, + /*largest_location=*/std::nullopt, /*parameters=*/VersionSpecificParameters(), }; stream_input->OnSubscribeOkMessage(ok); @@ -3317,7 +3317,7 @@ /*request_id=*/0, /*expires=*/quic::QuicTimeDelta::FromMilliseconds(10000), /*group_order=*/MoqtDeliveryOrder::kAscending, - /*largest_id=*/std::nullopt, + /*largest_location=*/std::nullopt, /*parameters=*/VersionSpecificParameters(), }; stream_input->OnSubscribeOkMessage(ok);
diff --git a/quiche/quic/moqt/test_tools/mock_moqt_session.cc b/quiche/quic/moqt/test_tools/mock_moqt_session.cc index 5753aa8..d9c2e1b 100644 --- a/quiche/quic/moqt/test_tools/mock_moqt_session.cc +++ b/quiche/quic/moqt/test_tools/mock_moqt_session.cc
@@ -52,7 +52,7 @@ void OnSubscribeAccepted() override { visitor_->OnReply(name_, HasObjects() - ? std::make_optional(publisher_->GetLargestSequence()) + ? std::make_optional(publisher_->GetLargestLocation()) : std::nullopt, std::nullopt); } @@ -209,7 +209,7 @@ return Fetch(name, std::move(callback), Location(0, 0), 0, 0, priority, delivery_order, std::move(parameters)); } - Location largest = track_publisher->get()->GetLargestSequence(); + Location largest = track_publisher->get()->GetLargestLocation(); uint64_t start_group = largest.group >= num_previous_groups ? largest.group - num_previous_groups + 1 : 0;
diff --git a/quiche/quic/moqt/test_tools/moqt_test_message.h b/quiche/quic/moqt/test_tools/moqt_test_message.h index 31c2521..b30d63c 100644 --- a/quiche/quic/moqt/test_tools/moqt_test_message.h +++ b/quiche/quic/moqt/test_tools/moqt_test_message.h
@@ -556,7 +556,7 @@ bool EqualFieldValues(MessageStructuredData& values) const override { auto cast = std::get<MoqtSubscribeOk>(values); - if (cast.subscribe_id != subscribe_ok_.subscribe_id) { + if (cast.request_id != subscribe_ok_.request_id) { QUIC_LOG(INFO) << "SUBSCRIBE OK subscribe ID mismatch"; return false; } @@ -568,7 +568,7 @@ QUIC_LOG(INFO) << "SUBSCRIBE OK group order mismatch"; return false; } - if (cast.largest_id != subscribe_ok_.largest_id) { + if (cast.largest_location != subscribe_ok_.largest_location) { QUIC_LOG(INFO) << "SUBSCRIBE OK largest ID mismatch"; return false; } @@ -597,19 +597,19 @@ private: uint8_t raw_packet_[16] = { - 0x04, 0x00, 0x0d, 0x01, 0x03, // subscribe_id = 1, expires = 3 + 0x04, 0x00, 0x0d, 0x01, 0x03, // request_id = 1, expires = 3 0x02, 0x01, // group_order = 2, content exists - 0x0c, 0x14, // largest_group_id = 12, largest_object_id = 20, - 0x02, // 2 parameters - 0x02, 0x67, 0x10, // delivery_timeout = 10000 - 0x04, 0x67, 0x10, // max_cache_duration = 10000 + 0x0c, 0x14, // largest_location = (12, 20) + 0x02, // 2 parameters + 0x02, 0x67, 0x10, // delivery_timeout = 10000 + 0x04, 0x67, 0x10, // max_cache_duration = 10000 }; MoqtSubscribeOk subscribe_ok_ = { - /*subscribe_id=*/1, + /*request_id=*/1, /*expires=*/quic::QuicTimeDelta::FromMilliseconds(3), /*group_order=*/MoqtDeliveryOrder::kDescending, - /*largest_id=*/Location(12, 20), + /*largest_location=*/Location(12, 20), VersionSpecificParameters(quic::QuicTimeDelta::FromMilliseconds(10000), quic::QuicTimeDelta::FromMilliseconds(10000)), };
diff --git a/quiche/quic/moqt/tools/moqt_mock_visitor.h b/quiche/quic/moqt/tools/moqt_mock_visitor.h index eebd267..3b910ee 100644 --- a/quiche/quic/moqt/tools/moqt_mock_visitor.h +++ b/quiche/quic/moqt/tools/moqt_mock_visitor.h
@@ -74,7 +74,7 @@ (override)); MOCK_METHOD(absl::StatusOr<MoqtTrackStatusCode>, GetTrackStatus, (), (const, override)); - MOCK_METHOD(Location, GetLargestSequence, (), (const, override)); + MOCK_METHOD(Location, GetLargestLocation, (), (const, override)); MOCK_METHOD(MoqtForwardingPreference, GetForwardingPreference, (), (const, override)); MOCK_METHOD(MoqtPriority, GetPublisherPriority, (), (const, override));