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

8.3 RTP matching rules and "packet_receiver" (bad word) #344

Closed
ibc opened this Issue Jan 13, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@ibc
Copy link
Contributor

ibc commented Jan 13, 2016

Section 8.3 mentions packet_receiver several times but, in fact, it means RTCRtpReceiver and I consider it confusing for the reader. IMHO it should use RTCRtpReceiver or RTP receiver rather than packet_receiver.

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Jan 13, 2016

packet_receiver denotes an attribute whose value is the RTCRtpReceiver object to which the incoming RTP packet should be delivered. Since the attribute receiver is used earlier in Section 8.3 in a different context (relating to setting up the ssrc_table, muxId_table and pt_table), we didn't want to reuse that attribute name.

Would rtp_receiver work better?

@ibc

This comment has been minimized.

Copy link
Contributor

ibc commented Jan 13, 2016

Would "rtp_receiver" work better?

LGTM

Also, not sure now if the spec mentions "attribute" but if so I think that "key" would fit better (an object/map/hash has "key/value" pairs rather than "attribute/value").

@ibc

This comment has been minimized.

Copy link
Contributor

ibc commented Jan 14, 2016

Let me a question: this issue is fixed in master, should it remain open?

@aboba

This comment has been minimized.

Copy link
Contributor

aboba commented Jan 15, 2016

Once we have a PR in master we mark it as "PR exists". Next step is to close it if there is positive feedback on the PR (or at least no objections for a while).

@ibc

This comment has been minimized.

Copy link
Contributor

ibc commented Jan 15, 2016

The PR is merged, that's why I expect this issue to be closed now.

@aboba aboba closed this Jan 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment