sony / nmos-cpp

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

smpte 2022-6 sender sdp file fails nmos-testing is05 test_41 #172

Closed orjan-neti closed 3 years ago

orjan-neti commented 3 years ago

I get fail on test_41 of test suite IS05 Connection management api with the following error:

SDPoker error for Sender f58d8039-eb08-5e57-830d-5fe55f74dcf8 transport file: Found 1 error(s) in SDP file: 1: Line 10: For stream 1, found an 'fmtp' attribute that is not an acceptable pattern."

The generated sdp file looks like:

v=0
o=- 3822977266 3822977266 IN IP4 10.42.5.1
s=.mts.5.mtp.tx.1
t=0 0
m=video 46052 RTP/AVP 98
c=IN IP4 192.0.2.1
a=ts-refclk:localmac=00-10-5B-27-70-B9
a=mediaclk:direct=0
a=rtpmap:98 SMPTE2022-6/27000000
a=fmtp:98 

I have looked a bit at the sdp generating code at sdp_utils.cpp:623 and it looks like it acts on info in the "video.tp"-field of sdp_paramters struct, but as far as I can see make_mux_sdp_parameters() only assigns value to ".mux.tp".

Any input on this is appreciated.

edit: our code are using nmos-cpp ver c38f782c4d6ff7a622ae1248064fcb59984d1c03

garethsb commented 3 years ago

That's a bug. Thanks very much for the report, we haven't had many ST 2022-6 users. I'll get that fixed ASAP.

orjan-neti commented 3 years ago

Thank you very much Gareth!

garethsb commented 3 years ago

Hi, Örjan, I've pushed a quick fix in 283e4cc. If you're able to let me know if this works for you, that would be great. Let's add updating the example nmos-cpp-node/node_implementation.cpp to allow config to switch from ST 2110-x0 separate video/audio/data to ST 2022-6 mux, so this could be tested with the example Node.

orjan-neti commented 3 years ago

Hello Gareth, I'm afraid that after the fix nmos::make_mux_session_description now throws an exception. format_specific_parameters is not an array, so push_back throws.

Initializing format_specific_parameters to an empty array seem to solve that.

        auto format_specific_parameters = web::json::value::array();
        if (!sdp_params.mux.tp.name.empty()) web::json::push_back(format_specific_parameters, sdp::named_value(sdp::fields::type_parameter, sdp_params.mux.tp.name));

I have to admit that I'm not very conversant in sdp, so I can't judge the resulting output, but at least nmos-testing seemed happy with my hastily hacked nmos-cpp-node mux sender test.

garethsb commented 3 years ago

@orjan-neti, that's embarrassing, I do apologise. I have fixed that bug, and prevented the same mistake recurring by adding some code that turns it into a compile-time error. And as penance, I'm adding support for ST 2022-6 mux 'ports' in the nmos-cpp-node example app, and checking that this passes all the AMWA test suites.

garethsb commented 3 years ago

Results of the test suites run in CI now look good.

orjan-neti commented 3 years ago

Yes, I can confirm that it resolves my problem as well. Many thanks Gareth!

garethsb commented 3 years ago

Thanks again for the report!