sony / nmos-cpp

An NMOS (Networked Media Open Specifications) Registry and Node in C++ (IS-04, IS-05)
Apache License 2.0
142 stars 80 forks source link

Handling of PTP reference clocks. #125

Closed s13n closed 3 years ago

s13n commented 4 years ago

NMOS IS-04 requires that the GMID of the PTP reference clock be given with lowercase hex number notation. The corresponding SDP attribute a=ts-refclk has it the other way around, i.e. uppercase hex only. Somewhere a conversion must happen either way, but I don't see where.

Furthermore, nmos-cpp seems to assume that the a=ts-refclk attribute is always in the media section of the SDP file. Indeed that is what ST2110 (stupidly) demands. Other relevant standards don't have this restriction, for example AES67, and indeed the RFC 7273 they all depend on. One would have thought that the normal case is a common reference clock, so that putting it into the global section of the SDP file would be the most prevalent case, if it weren't for ST2110.

So I think nmos-cpp should at least recognise and properly handle either style when receiving an SDP. At the moment, it insists on ST2110 on input, too.

Please tell if you want me to produce a patch.

garethsb commented 4 years ago

NMOS IS-04 requires that the GMID of the PTP reference clock be given with lowercase hex number notation. The corresponding SDP attribute a=ts-refclk has it the other way around, i.e. uppercase hex only.

https://github.com/AMWA-TV/nmos-discovery-registration/issues/144

Somewhere a conversion must happen either way, but I don't see where.

E.g. https://github.com/sony/nmos-cpp/blob/7284ef84c07e1772cc4234ee20ed299571906bfb/Development/nmos/sdp_utils.cpp#L64

Furthermore, nmos-cpp seems to assume that the a=ts-refclk attribute is always in the media section of the SDP file. Indeed that is what ST2110 (stupidly) demands.

I'm well aware of the (insert adjective of your choice) additional demands added by ST 2110 beyond RFC 7273. I raised this previously with the ST 2110 team, and was told it was to make things "simpler". 🤷‍♂

So I think nmos-cpp should at least recognise and properly handle either style when receiving an SDP. At the moment, it insists on ST2110 on input, too.

Please tell if you want me to produce a patch.

Yep, updating nmos::get_session_description_sdp_parameters to use session-level values for ts-refclk and mediaclk as the default, as per RFC 7273 Section 4.8, would be nice.

s13n commented 4 years ago

Gosh, time to update my repository. Sorry to bother you with old stuff.

I raised this previously with the ST 2110 team, and was told it was to make things "simpler".

Ouch. That's not my concept of "simpler".

I will try to produce a patch for session-level ts-refclk and mediaclk shortly.

garethsb commented 4 years ago

Yep, updating nmos::get_session_description_sdp_parameters to use session-level values for ts-refclk and mediaclk as the default, as per RFC 7273 Section 4.8, would be nice.

See https://github.com/aholzinger/nmos-cpp/commit/80f41a8679a15f85e6a2cbd75e77600250bdfbcb