More flexible make_<format>_sdp_parameters functions#205
Conversation
…explicitly in the make_<format>_sdp_parameters functions: ST 2110-21 Type Parameter for video/mux, packet time for ST 2110-30 audio and VPID Code for ST 2110-40 ancillary data
| params.vpid_code = vpid_code ? *vpid_code : 0; | ||
|
|
||
| return{ sender.at(nmos::fields::label).as_string(), params, 100, media_stream_ids, details::make_ts_refclk(node, source, sender, ptp_domain) }; | ||
| return{ sender.at(nmos::fields::label).as_string(), params, payload_type ? *payload_type : 100, media_stream_ids, details::make_ts_refclk(node, source, sender, ptp_domain) }; |
There was a problem hiding this comment.
Perhaps a comment explaining away the magic 100, similar to the comment in lines 220 and 221 below? Similarly for the the magic 96 and 97 above
There was a problem hiding this comment.
Good idea... Whereas ST 2022-8 recommends 98, I'm not sure where the 96, 97, 100, come from. Any idea, @jonathan-r-thorpe?
There was a problem hiding this comment.
Well, they're all dynamic payload types and one as worth as the other. At the end it really doesn't matter. My point is that when a user of nmos-cpp library wants to specify a certain payload type the call to make_sdp_parameters shouldn't override the previously chosen default.
There was a problem hiding this comment.
How can it be "previously chosen"? This function returns a new sdp_parameters object.
There was a problem hiding this comment.
There could be a default chosen by the user of the library. Now the payload would change because it is returned from make_sdp_parameters.
There was a problem hiding this comment.
It couldn't change, as the user previously never communicated the default they chose to the library. 😕
Or at least only by setting the value into the sdp_parameters returned by make_sdp_parameters.
With this PR, they can now provide it up front, but really there's not a lot of difference.
|
Yes, this would help, although we currently use make_sdp_parameters and then afterwards set payload type and ptp domain. Weo would then need to use make_audio_sdp_parameters( instead, which is okay. Adding optional parameters seems a good idea to me. |
|
Relocated |
* 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
4 participants
@aholzinger, would this resolve #202 to your satisfaction?