Skip to content

Conversation

@appsdesh
Copy link
Contributor

As the transmitter does not have any notion of the receiver uniqueness, it makes more sense to callout the stream ID to be unique per stream.

In the update stream event, aud is expected as a stream ID which is expected unique per stream as well.

Stream ID to be unique per stream
@FragLegs FragLegs self-requested a review August 30, 2023 23:36
Copy link
Contributor

@FragLegs FragLegs left a comment

Choose a reason for hiding this comment

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

This is more clear. Thanks!

Uniqueness per Transmitter
tulshi
tulshi previously approved these changes Sep 5, 2023

> **Transmitter-Supplied**, A string that uniquely identifies the stream. Stream
IDs MUST be unique per Reciever. This value is generated by the Transmitter
IDs MUST be unique per stream in the context of each transmitter. This value is generated by the Transmitter
Copy link
Contributor

Choose a reason for hiding this comment

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

"across all streams that are currently open in a Transmitter"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the following to handle "future" streams, rather than just the "currently open"

Transmitter-Supplied, A string that uniquely identifies the stream. A transmitter MUST generate a unique ID for each of it's streams.

@tulshi tulshi dismissed their stale review September 5, 2023 21:21

need minor text changes

Verbiage change
> **Transmitter-Supplied**, A string that uniquely identifies the stream. Stream
IDs MUST be unique per Reciever. This value is generated by the Transmitter
when the stream is created.
> **Transmitter-Supplied**, A string that uniquely identifies the stream. A transmitter MUST generate a unique ID for each of it's streams at the time of stream creation.
Copy link
Contributor

Choose a reason for hiding this comment

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

it's streams -> its open streams (no apostrophe, and specify open, since the rest of the sentence talks about the time of creation)

appsdesh added a commit that referenced this pull request Sep 14, 2023
Consolidating changes from PRs

1. #109
2. #108
3. #100
@appsdesh
Copy link
Contributor Author

Following up on #117

@appsdesh appsdesh closed this Sep 14, 2023
appsdesh added a commit that referenced this pull request Sep 22, 2023
Consolidating changes from PRs

1. #109
2. #108
3. #100
tulshi pushed a commit that referenced this pull request Sep 22, 2023
* Consolidating changes from PRs

Consolidating changes from PRs

1. #109
2. #108
3. #100

* Shayne's suggestion

Shayne's suggestion

* More places to fix MUST -> SHOULD

More places to fix MUST -> SHOULD
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.

4 participants