Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUBSCRIBE_NAMESPACE #498

Merged
merged 6 commits into from
Sep 4, 2024
Merged

SUBSCRIBE_NAMESPACE #498

merged 6 commits into from
Sep 4, 2024

Conversation

afrind
Copy link
Collaborator

@afrind afrind commented Aug 5, 2024

This message suite allows subscribers to express interest in ANNOUNCE/UNANNOUNCE for set of namespaces that match a prefix. To accomplish safe prefix matching, Track Namesapce is redefined from a single byte array to a N-Tuple of byte arrays.

Fixes #484
Closes #253

This message suite allows subscribers to express interest in ANNOUNCE/UNANNOUNCE for set of namespaces that match a prefix.  To accomplish safe prefix matching, Track Namesapce is redefined from a single byte array to a N-Tuple of byte arrays.

Fixes: #484
@@ -325,6 +331,20 @@ In MOQT, every track has a track name and a track namespace associated
with it. A track name identifies an individual track within the
namespace.

Track namespace is an N-tuple of bytes where N can be between 1 and 32. The
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if we need to limit the number of tuples. but we should instead have limit on the overall length.

Copy link
Collaborator

@suhasHere suhasHere Aug 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we need this to be "Ordered N-tuple" . From the definition of tuple from prog lang, it is sequence , but not sure if we need to make it clear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The limit on the number of tuples allows the relay to bound the complexity of a tree-structure used to implement tuple prefix matching.

Ordered tuple seems more specific, I'll update it.

* Track Namespace Prefix: An N-Tuple of byte fields which are matched against
track namespaces known to the publisher. For example, if the publisher is a
relay that has received ANNOUNCE messages for namespaces ("example.com",
"meeting", "123", "participant=100") and ("example.com", "meeting", "123",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this example is bit misleading but the intent is fine. May be better way to say is
("example.com", "meeting/123", "participant/100")

The mix of a=b, vs split info is confusing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll restructure the example to make it more clear.

Copy link
Collaborator

@suhasHere suhasHere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afrind Huge thanks for this PR. This is coming along pretty well.
I do have few clarification questions and few details that, if added, will make it easy to reason about.

Copy link
Collaborator Author

@afrind afrind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review.

@@ -325,6 +331,20 @@ In MOQT, every track has a track name and a track namespace associated
with it. A track name identifies an individual track within the
namespace.

Track namespace is an N-tuple of bytes where N can be between 1 and 32. The
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The limit on the number of tuples allows the relay to bound the complexity of a tree-structure used to implement tuple prefix matching.

Ordered tuple seems more specific, I'll update it.

draft-ietf-moq-transport.md Show resolved Hide resolved
draft-ietf-moq-transport.md Show resolved Hide resolved
* Track Namespace Prefix: An N-Tuple of byte fields which are matched against
track namespaces known to the publisher. For example, if the publisher is a
relay that has received ANNOUNCE messages for namespaces ("example.com",
"meeting", "123", "participant=100") and ("example.com", "meeting", "123",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll restructure the example to make it more clear.

draft-ietf-moq-transport.md Show resolved Hide resolved
draft-ietf-moq-transport.md Show resolved Hide resolved
@wilaw
Copy link
Contributor

wilaw commented Aug 7, 2024

Philosophical question - we now have a N-32 tuple of namespace components and then a track name that hangs off the end. What's the utility in that? Could not the track name just be one of the namespace components?

Can we simplify MOQT by getting rid of the concept of namespace and instead having a N-32 tuple track name?

Copy link
Collaborator Author

@afrind afrind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding Will's question:

Can we simplify MOQT by getting rid of the concept of namespace and instead having a N-32 tuple track name?

This also crossed my mind. At present, ANNOUNCE only announces namespace and SUBSCRIBE still requires a FULL match on namespace to route through a relay, so the distinction between namespace and name is serving a purpose. I guess we could say the last component is always the track name. I was hesitant in any case to completely undo track naming in this PR.

draft-ietf-moq-transport.md Show resolved Hide resolved
draft-ietf-moq-transport.md Show resolved Hide resolved
@suhasHere
Copy link
Collaborator

This is a good start. I am sure we will fix aspects of it as we explore futher, but overall LGTM

@afrind afrind mentioned this pull request Aug 14, 2024
Copy link
Collaborator

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments, but overall LGTM

draft-ietf-moq-transport.md Outdated Show resolved Hide resolved
draft-ietf-moq-transport.md Show resolved Hide resolved
Co-authored-by: ianswett <ianswett@users.noreply.github.com>
Copy link
Contributor

@fluffy fluffy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the direction of this and it makes a bunch of things much easier to do.


: Indicates that x is a tuple, consisting of a variable length integer encoded as
described in ({{?RFC9000, Section 16}}), followed by that many variable length
tuple fields, each of which are encoded as (b) above.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny nit but I don't love this syntax for this. It would be hard for someone reading this out of the blue to figure this out. But agree with the encoding , just wish there was less confusing syntax to do this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this is non-obvious. Should we burn some characters and make it "x (tup)" or "x (tuple)"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try it with x (tuple) and see what folks think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah, I like that x(tuple) proposal. ( and of course I don't care deeply if we keep it like it is, either way works for me )

draft-ietf-moq-transport.md Show resolved Hide resolved
@fluffy
Copy link
Contributor

fluffy commented Aug 23, 2024

It would be good to also bring in the changes proposed in #508

@afrind
Copy link
Collaborator Author

afrind commented Aug 23, 2024

It would be good to also bring in the changes proposed in #508

I will try to take a stab at that but I'm worried the scope is going to blow up this PR, and we'd do better to handle that in a follow up. I mentioned this briefly in the interim, but this PR adds significant functionality that is currently not possible so I am biased towards an iterative approach.

@ianswett
Copy link
Collaborator

To be clear, I'm happy to land this and then evaluate other improvements like #508 subsequently.

@suhasHere
Copy link
Collaborator

just 2 cents, #508 addresses important design aspects which i feel would be good to consider bringing in this PR as it applies not just to announces but subscribes in general. It would be good to evaulate this PR in the context of #508

@fluffy
Copy link
Contributor

fluffy commented Aug 26, 2024

Looking at #508 later after lands seems fine to me

@fluffy
Copy link
Contributor

fluffy commented Aug 29, 2024

I hope we can get agenda time to talk about this one at the next call. This add critical functionally to moq that I would really like to have included even if we still needed time and implementation experience to get all the details sorted.

Copy link
Collaborator Author

@afrind afrind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe all comments have been addressed, except "should we make this a special track instead".

draft-ietf-moq-transport.md Show resolved Hide resolved
@@ -378,6 +384,20 @@ In MOQT, every track has a track name and a track namespace associated
with it. A track name identifies an individual track within the
namespace.

Track namespace is an ordered N-tuple of bytes where N can be between 1 and 32.
The structured nature of Track Namespace allows relays and applications to
manipulate prefixes of a namespace. Track name is a sequence of bytes.
Copy link
Collaborator

@ianswett ianswett Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
manipulate prefixes of a namespace. Track name is a sequence of bytes.
match on prefixes of a namespace. Track name is a sequence of bytes.

Or maybe there are other types of manipulation?

@ianswett ianswett merged commit 73d71a6 into main Sep 4, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discovery of publishers via a relay instead of catalog Wildcard SUBSCRIBE
5 participants