Skip to content

Refactor (and a few breaking interface changes) of sdp_utils#225

Merged
lo-simon merged 10 commits into
sony:masterfrom
garethsb:fmtp
Jan 14, 2022
Merged

Refactor (and a few breaking interface changes) of sdp_utils#225
lo-simon merged 10 commits into
sony:masterfrom
garethsb:fmtp

Conversation

@garethsb
Copy link
Copy Markdown
Contributor

@garethsb garethsb commented Jan 10, 2022

Refactor (and a few breaking interface changes) of nmos/sdp_utils.{cpp,h} to tease apart format-agnostic SDP creation, parsing and validation from format-specific functionality.

This is intended to simplify future addition of support for video/jxsv and other media types mentioned in #213, whether within nmos-cpp or in application code for specific use cases.

Details

  1. sdp_parameters is now format-agnostic.
    It no longer includes any format (media type/subtype) specific fields, like video.width.
    Instead, the format-specific parameters for the a=fmtp: line are serialized to a vector of pairs of string in this object. (It can't be a map because some keys may appear multiple times, like DID_SDID.)
    Other common (RFC4566-defined) attributes that are likely to be used, like a=ptime:, a=maxptime: and a=framerate: are added to the format-agnostic struct, as is direct support for one bandwidth b= line.
  2. There are now free functions to get and set typed format-specific parameters from/to the fmtp, rtpmap and other member variables.
    E.g. audio sample rate and channel count from the m= line, packet time from a=ptime:, and in the future, JPEG XS bit rate from the b= line.
    These are make_<media type/subtype>_sdp_parameters but get_<media type/subtype>_parameters rather than parse_ because there are other members of sdp_parameters which are not reflected into the <media type/subtype>_parameters objects.

Breaking interface changes

  • Constructors of sdp_parameters that took sdp_parameters::video_t, audio_t, etc. are deprecated (so not a breaking change, yet!).
    Equivalent constructors for new media types should not be added in the future. The free function approach should be adopted. See below...
  • Instead of sdp.video, sdp.audio, etc. application code must now use get_video_raw_parameters(sdp), get_audio_L_parameters(sdp), etc.
  • sdp_parameters::audio_t::sample_rate is now a uint64_t rather than an nmos::rational to correspond more closely with the SDP spec and roundtrip better

Adding a new media type

  1. Create a struct <media type/subtype>_parameters with members for the necessary format parameters.
    You may not want to include everything mentioned in the IANA registration, but that's a good place to start.
    See e.g. video_raw_parameters.
  2. Add make_<media type/subtype>_parameters which constructs the struct from info in the IS-04 resources and possibly commonly required additional info not carried in IS-04 resources (e.g. packet time for audio).
    See e.g. make_video_raw_parameters.
  3. Add make_<media type/subtype>_sdp_parameters which constructs the format-agnostic sdp_parameters from the struct (and the non-transport, non-format-specific, parameters such as the SDP session name).
    This function can usually be very simple, for example building the rtpmap and fmtp from the <media type/subtype>_parameters.
    Also add an equivalent overload of make_sdp_parameters; should be a one-liner.
    See e.g. make_video_raw_sdp_parameters.
  4. Add get_<media type/subtype>_parameters which constructs the struct from sdp_parameters.
    This function can usually be very simple, for example extracting the values for the <media type/subtype>_parameters from the rtpmap and fmtp.
    See e.g. get_video_raw_parameters.
  5. Support creation of SDP files of the new format based on the IS-04 resources.
    Within the nmos-cpp library this would be done by updating the nmos::make_sdp_parameters overload to use the new make_<media type/subtype>_sdp_parameters function.
    This nmos-cpp library function obviously couldn't be updated by application code, but since this function is only called from application code, such applications would use their own function for the application-specific media types instead.
  6. Support the new format for validation of SDP files for receiver capabilities.
    Within the nmos-cpp library, this would be done by updating nmos::validate_sdp_parameters.
    This nmos-cpp library function couldn't be updated by application code. It is called by a library function, nmos::parse_rtp_transport_file, but since that function isn't used if the application provides an nmos::transport_file_parser. An application can use nmos::details::parse_rtp_transport_file which now provides a way for application code to easily plug in the application-specific SDP parameters validation.

… apart format-agnostic SDP creation, parsing and validation from format-specific functionality
@garethsb
Copy link
Copy Markdown
Contributor Author

garethsb commented Jan 11, 2022

Next steps

  1. Add the description of how to add a new media type to the docs?

  2. A PR to add e.g. video/jxsv (when the NMOS mapping, BCP-006-01 is fully spec'ed...) and/or a PR to show how e.g. nmos-cpp-node application code could add support for its own media type.

  3. Another PR to extend the existing formats with the missing format-specific parameters etc. either in nmos-cpp itself, or as another part of the application code example in nmos-cpp-node

  • video/raw (when used as per ST 2110-20/-21)
    • Required parameters
      • PM, SSN (currently written with hard-coded values and not read)
    • Optional parameters
      • RANGE
      • MAXUDP
      • PAR
      • TROFF (also for video/SMPTE2022-6 when used as per ST 2022-8)
      • CMAX
  • video/SMPTE2022-6
    • Other
      • Frame rate as a=framerate:
  • audio/L
    • Other
      • Maximum packet time in an a=maxptime: attribute (maybe it's good enough it's in the sdp_parameters now? just inconsistent with packet_time in sdp_parameters::audio_t/audio_L_parameters)

@garethsb garethsb marked this pull request as ready for review January 11, 2022 11:00
…d add make_sdp_parameters overloads for clear extensibility
@garethsb
Copy link
Copy Markdown
Contributor Author

@aholzinger, after our discussions around #202, you may be interested in this also.

@garethsb
Copy link
Copy Markdown
Contributor Author

@peterbrightwell, @David-P-B, @pedro-alves-ferreira, you may also be interested in this.

@lo-simon lo-simon merged commit c36e1d7 into sony:master Jan 14, 2022
@garethsb garethsb deleted the fmtp branch January 15, 2022 11:17
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.

2 participants