kyzer-davis / srtp-assurance-rfc-draft

A template repository for Internet-Draft management
0 stars 2 forks source link

Consolidate references to late binding, fix nits #20

Closed danwing closed 9 months ago

danwing commented 10 months ago

…on, fixed several nits introduced with -02

kyzer-davis commented 10 months ago

I think all the changes make sense. Thanks @danwing, let me just cross post this to the other Authors before I merge it down.

danwing commented 10 months ago

Cool.

I could not figure out the "must have a 0" requirement for the value of the ROC/SSRC/Seq. As in, does that refer to the "0" of "0x" (I think it doesn't, but the text isn't clear). And if I have a 32 bits that are all 1, does it mean I have to provide a leading 0 before all my F's? That also doesn't make sense. Also, does it mean 0 has to be written as "0x00" (with a leading zero)?

In any event, I couldn't figure out a reason for the wording. I would tighten that up and adjust ABNF to require the leading zero if you really need it. Or remove the wording.

Also, is this the only thing encoded in SDP that uses hex? I haven't checked all the various RFCs that define SDP, but this does seem an outlier that it wants hex instead of decimals.

kyzer-davis commented 9 months ago

Sorry for the delay @danwing, I wanted to explicitly define that the conveyed SDP value was a hex value with the 0x prefix.

I chose the hex as the way to represent these for the following reasons:

Honestly I am fine with hex or integer. I am a bit biased in that most things I view, where I need to decode some SRTP media capture for triage, involve something that dumps me a hex value, so that is what I transposed to the SDP. So personally it would be easier to compare across log files and copy/paste into other debugging tools I have :)


The "leading 0s may be omitted and the alphanumeric hex may be upper or lowercase but at least one 0 must be present." was meant to illustrate that 0x000000 can be represented as 0x0 but something like 0x00845FED can be changed to 0x845fed Both are in Figure 4. I see the error and something better would be "one hex character" in place of "one 0". Thus:

leading 0s may be omitted and the alphanumeric hex may be upper or lowercase but at least one 0 must be present.

To:

leading 0s may be omitted and the alphanumeric hex may be upper or lowercase but at least one hex character must be present.

danwing commented 9 months ago

SSRC is signaled in SDP using decimal yet Wireshark shows SSRC as hex; it seems most folks can cope? But I am not aware of anything in SDP that is signaled in hex using the "0x" prefix, so it seems out of norm with the rest of SDP.

kyzer-davis commented 9 months ago

Yeah, this is really just for the humans. The code and other implementations will convert it to whatever is required after parsing the SDP.

let me merge this PR and discuss hex encoding vs some other for the visual display on the mailer.