Skip to content

SDP fixes#201

Merged
lo-simon merged 3 commits into
sony:masterfrom
garethsb:sdp-fixes
Oct 4, 2021
Merged

SDP fixes#201
lo-simon merged 3 commits into
sony:masterfrom
garethsb:sdp-fixes

Conversation

@garethsb
Copy link
Copy Markdown
Contributor

@garethsb garethsb commented Oct 1, 2021

  1. Fix correct setting of source_ip in a Receiver's transport_params when parsing an SDP file which doesn't contain a source-filter. Previously it set it from the origin <unicast-address> but that's wrong - as shown by the example in the IS-05 spec for Behaviour - RTP Transport Type - Unicast.

  2. Following on from that, generate source-filters from unicast Senders' transport_params by default.

  3. To enable creation of the effective SDP files at a Receiver when rtp_enabled is explicitly overridden, support creation of SDP files from Receivers' transport_params. Previously only worked from Senders' transport_params, which have a slightly different set of parameters, e.g. destination_ip vs. multicast_ip/interface_ip.

  4. New test cases are added for most of the above improvements.

  5. Also support "session-level" ts-refclk and mediaclk which will resolve Handling of PTP reference clocks. #125. The source-filter attribute is still not handled at "session-level", but I've added more comments to explain some of the ways the source-filter handling could be improved (e.g. checking <dest-address> actually matches the connection address being considered).

  6. Fix generation of a=mediaclk:sender (which would previously have been rendered with a trailing =).

@garethsb
Copy link
Copy Markdown
Contributor Author

garethsb commented Oct 1, 2021

The nmos::get_session_description_transport_params function (which is used when parsing SDP files for IS-05 PATCH requests on Receivers) includes this comment:

        // For now, this function should handle the following cases identified in the documentation:
        // * Unicast
        // * Source Specific Multicast
        // * Any Source Multicast
        // * Operation with SMPTE 2022-7 - Separate Source Addresses
        // * Operation with SMPTE 2022-7 - Separate Destination Addresses

        // The following cases are not yet handled:
        // * Operation with SMPTE 2022-5
        // * Operation with SMPTE 2022-7 - Temporal Redundancy
        // * Operation with RTCP

I think this is basically true, however it's worth pointing out that not all of the other relevant information is extracted from the SDP file.

For example, when parsing an SDP file for ST 2022-7 with "Separate Destination Addresses", the <payload type> is only extracted into the sdp_parameters from the m=/a=rtpmap: lines of the first media description. Similarly, the ssrc attributes are not extracted when parsing an SDP File for ST 2022-7 with "Separate Source Addresses". This would prevent accurate roundtrip - creation of a new SDP file from the parsed transport_params and sdp_parameters.

On the creation side, nmos::make_session_description does not distinguish the different ST 2022-7 cases, so I'm pretty certain the "Separate Source Addresses" case does not generate the correct SDP file.

@garethsb
Copy link
Copy Markdown
Contributor Author

garethsb commented Oct 1, 2021

Note that ST 2110-10 has this to say about representation of ST 2022-7 in SDP:

8.3 SDP for Duplicate RTP Streams
Duplicate RTP streams meeting the requirements of SMPTE ST 2022-7 may be used for redundant transmission to achieve higher system availability. Senders which transmit these duplicated RTP streams, using the mechanisms of separate source addresses (IETF RFC 7104, Section 4.1) or separate destination addresses (IETF RFC 7104, section 4.2) shall signal the RTP duplication using the session level group attribute of IETF RFC 5888 and the duplication grouping DUP semantics of IETF RFC 7104.

The example for "Separate Source Addresses" in IS-05 is taken from RFC 7104 Section 4.1 and it uses ssrc and ssrc-groupattributes for synchronization source (SSRC) multiplexed RTP streams. That mechanism, which is defined by RFC 5576, isn't mentioned in ST 2110-10, which may explain why I've never seen it in the wild...

The ST 2110-10 Annex B SDP Example (Informative) is of the "Separate Destination Addresses" case, and uses the same RTP Payload ID for both streams, which may explain why I've never seen different ones outside of the example in RFC 7104 Section 4.2 which is reproduced in IS-05.

@garethsb
Copy link
Copy Markdown
Contributor Author

garethsb commented Oct 1, 2021

  1. Following on from that, generate source-filters from unicast Senders' transport_params by default.

The default behaviour at the Sender side is now that source-filter is written for both unicast (new) and multicast (as before).

I wonder whether this should be tightened up to only include the source-filter for multicast if the address is in the source-specific multicast block, 232/8 (232.0.0.0-232.255.255.255) defined by RFC 5771 and registered in the IANA IPv4 Multicast Address Space Registry. The new source_filters optional flag could be used to force it off in that range or on in other ranges.

On the other hand, I think source filters are valid whatever address range the connection address is in, so maybe stick with the status quo? 🤷‍♂️ See e.g. RFC4570 Section 4.

The comment on the function doesn't match the impl so one or other needs to change 🙂
https://github.com/sony/nmos-cpp/pull/201/files#diff-5df26230d2c70cf295fa2d37f469bbffb2f4302f2de693aafd3b24efb6f44a04R30
Fixed in 9a0174b.

Copy link
Copy Markdown
Collaborator

@lo-simon lo-simon left a comment

Choose a reason for hiding this comment

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

LGTM

…n parsing an SDP file which doesn't contain a source-filter

Generate source-filters from unicast Senders' transport_params by default
Support creation of SDP files from Receivers' transport_params
Add optional flag to omit generation of source-filters
Support "session-level" ts-refclk and mediaclk (but not source-filter)
Fix generation of a=mediaclk:sender
Unit tests for some of these improvements :-/
@lo-simon lo-simon merged commit 4fba3ab into sony:master Oct 4, 2021
@garethsb garethsb deleted the sdp-fixes branch April 22, 2022 07:43
garethsb added a commit to garethsb/nmos-cpp that referenced this pull request Apr 28, 2022
* for build and dependencies, e.g. sony#197, sony#198, sony#207, sony#211, sony#215, sony#229, sony#230, sony#235, sony#243
* for SDP parser/generator, e.g. sony#201, sony#205, sony#219, sony#241, sony#242, sony#244
* for RQL, e.g. sony#224
* for CI tests, e.g. sony#218, sony#231, sony#239, sony#250
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.

Handling of PTP reference clocks.

2 participants