Implement announce using Subscriber interface #17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D68139012 |
|
This pull request was exported from Phabricator. Differential Revision: D68139012 |
afrind
added a commit
to afrind/moxygen
that referenced
this pull request
Jan 18, 2025
Summary: Pull Request resolved: facebookexperimental#17 By removing the last use of the control message queue, delete all control loops and control visitors Differential Revision: D68139012
afrind
force-pushed
the
export-D68139012
branch
from
41c0783 to
991eda1
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68139012 |
afrind
added a commit
to afrind/moxygen
that referenced
this pull request
Jan 18, 2025
Summary: Pull Request resolved: facebookexperimental#17 By removing the last use of the control message queue, delete all control loops and control visitors Differential Revision: D68139012
afrind
force-pushed
the
export-D68139012
branch
from
991eda1 to
a9515ed
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68139012 |
afrind
added a commit
to afrind/moxygen
that referenced
this pull request
Jan 19, 2025
Summary: Pull Request resolved: facebookexperimental#17 By removing the last use of the control message queue, delete all control loops and control visitors Differential Revision: D68139012
afrind
force-pushed
the
export-D68139012
branch
from
a9515ed to
d303ab4
Compare
Summary: This is the beginning of rewriting the control interfaces of MoQSession. The goal is to have the interface match the key verbs of the protocol. Currently: Publisher: trackStatus subscribe fetch subscribeAnnounces Subscriber: announce Both: `goaway With the exception of goaway, which has no response, each of these APIs is a coroutine. The return value is a "Handle" to the corresponding state that supports the operations allowed (eg: SubscribeHandle = subscribeUpdate, unsubscribe). The API is designed to be symmetric, such that MoQSession will implement these APIs for the caller, and will be given a callback to handle these APIs when they arrive via the network. Reviewed By: sharmafb Differential Revision: D67865127
|
This pull request was exported from Phabricator. Differential Revision: D68139012 |
afrind
force-pushed
the
export-D68139012
branch
from
d303ab4 to
7b9d0ca
Compare
afrind
added a commit
to afrind/moxygen
that referenced
this pull request
Jan 22, 2025
Summary: Pull Request resolved: facebookexperimental#17 By removing the last use of the control message queue, delete all control loops and control visitors Differential Revision: D68139012
…SubscribeAnnounces Summary: This changes the return type of `subscribe`, `fetch` and `subscribeAnnounces` to match the Publisher interface, and moves trackStatus and subscribeAnnounces handling from the ControlVisitor to a Publisher callback. Reviewed By: sharmafb Differential Revision: D67865163
Summary: These should work through the SubscriptionHandle rather than directly on the session. Reviewed By: sharmafb Differential Revision: D68170454
Summary: I don't know why I never knew you could forward declare an inner class in a .h file and then fully define it in a .cpp file, but it's my new favorite trick to keep interfaces small while retaining the implicit friend behavior. No functional changes. Reviewed By: DanielFay22 Differential Revision: D67996498
Summary: Remove subscribe from the ControlVisitor and implement the logic on the receive side to invoke publisherCallback_->subscribe after receiving SUBSCRIBE on the wire. The result from the publisher's callback determines whether SUBSCRIBE_OK or SUBSCRIBE_ERROR is written. Note a minor edge case is that it is now possible for the publisher to invoke `subscribeDone` before returning from the handler. This probably should go out after OK (though not MUST?), so cache it and resend if this happens. Class Relationships: `MoQSession` is-a Publisher `MoQSession::SubscribeTrackReceiveState` is-a `Publisher::SubscriptionHandle` (subscriber invokes subscribeUpdate/unsubscribe) `MoQSession::TrackPublisherImpl` has-a `Publisher::SubscriptionHandle` (supplied by the publisher/application, the session will invoke subscribeUpdate/unsubscribe after receiving these messages on the wire). Reviewed By: sharmafb Differential Revision: D67996507
Summary: Pull Request resolved: facebookexperimental#16 This one is the most straightforward Reviewed By: sharmafb Differential Revision: D68139006
Differential Revision: D68139010
Differential Revision: D68164730
Differential Revision: D68170465
Summary: Pull Request resolved: facebookexperimental#17 By removing the last use of the control message queue, delete all control loops and control visitors Differential Revision: D68139012
|
This pull request was exported from Phabricator. Differential Revision: D68139012 |
afrind
force-pushed
the
export-D68139012
branch
from
7b9d0ca to
0620e38
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: By removing the last use of the control message queue, delete all control loops and control visitors
Differential Revision: D68139012