sony/nmos-cpp

nmos::details::get_format() and nmos::details::get_format_parameters() now reject -31 SDPs

duncwebb opened this issue · 6 comments

Good afternoon,
I'm currently in the process of transitioning to the latest code in master and there's a couple of changes in nmos/sdp_utils.cpp that causes -31 SDPs (with a media type of a=rtpmap:97 AM824/48000/16) to be rejected with an sdp_processing_error("unsupported media type/encoding name"). In the (admittedly old) version in the commit I'm moving from the nmos::details::get_format() and nmos::details::get_format_parameters() methods didn't check the media type identifier...

if (sdp::media_types::audio == sdp_params.media_type) return nmos::formats::audio;

is now:

if (sdp::media_types::audio == sdp_params.media_type && U("L") == sdp_params.rtpmap.encoding_name.substr(0, 1)) return nmos::formats::audio;

Is this intentional?

It would be great to have full support for audio/AM824 in nmos-cpp. It should be straightforward to add, but I think it should be treated separately than audio/Lnn as the required SDP format-specific parameters and NMOS resource attributes are slightly different?

Yes it would. I've managed to deal with it in our vendor code up until this point but the changes mean that the SDP is rejected before any of the callbacks to vendor code are made. I can patch nmos-cpp in our build system though I'll have a look at the other changes made in that commit for any potential gotchas.

@duncwebb the intention now is that new media types can be added in vendor code - those functions should only be called by validate_sdp_parameters which is just a default implementation called by nmos-cpp if application code doesn't override it. See the example code to see how things can be extended beyond the base support for video/raw, audio/L, video/smpte291 and video/SMPTE2022-6 to include video/jxsv. The same principle should be used to add support for audio/AM824 or any other media type:

nmos::transport_file_parser make_node_implementation_transport_file_parser()

Thanks Gareth, I'll have a look!

Yep, that makes sense - I'd overlooked that in my nmos::transport_file_parser handler I was calling validate_sdp_parameters() which in turn calls the changed method. I can change that to handle the SDP appropriately. Thanks again - I'll close this issue.

@duncwebb I'm glad that works for you. If you were able to share the differences you've encountered in handling -31 audio/AM824 vs. -30 SDP files, to be used as reference for implementation in the style of nmos/video_jxsv.h that would be great!