unispeech / unimrcp

Open source cross-platform implementation of MRCP protocol
http://www.unimrcp.org
Apache License 2.0
372 stars 163 forks source link

UniMRCP-Client ignores asymmetric Payload Type negotation #248

Closed schlagert closed 5 years ago

schlagert commented 5 years ago

I just found that the UniMRCP client does not correctly handle asymmetric payload types that were otherwise correctly negotiated using SDP. The client (library) does ignore the fact, that the server sent a matching codec with a different payload type.

How to reproduce? Configure an UniMRCP-based server with

<codecs own-preference="true">L16/96/8000</codecs>

Configure an UniMRCP-based client (e.g. asrclient) with

<codecs>L16/97/8000</codecs>

When trying to run a scenario, the SDP found in the client logs will totally ignore and therefore miss payload type 96.

achaloyan commented 5 years ago

This is an interesting case. I tried to replicate the setup per your description.

What I see is the client offers

a=rtpmap:97 L16/8000

Even though the server seems to respond with

a=rtpmap:96 L16/8000

the client actually receives

a=rtpmap:97 L16/8000

The problem is actually originated on the server. UniMRCP server passes the payload type 96 to Sofia-SIP uas as an answer to the received offer. However, the Sofia-SIP uas uses the offered 97 based on an internal logic implemented in the SOA module.

schlagert commented 5 years ago

Ok, then this is probably located somewhere in modules/mrcp-sofiasip/ or should this issue be moved to the sofia-sip repository?

schlagert commented 5 years ago

I just realized that the problem is independent from the own-preference configuration. It is sufficient to configure client and server with conflicting payload types...

achaloyan commented 5 years ago

The configuration parameter own-preference is actually made for selection of a codec from the two codec lists: offered by the client and configured for the server.

This particular problem, I believe, applies to Sofia-SIP itself, as there is no such an API available to alter the behavior from a user application.

However, I have absolutely no time to dig further into this problem to identify the root case. Let's leave this for later, unless you are willing to investigate the problem further.

schlagert commented 5 years ago

I already invested a half day on Friday to investigate. Unfortunately, there is no easy fix and from what I've read, the FreeSwitch team works around this by disabling SOA all together.

I will see if I can invest some more hours next week because the issue is quite nasty. It totally breaks the negotiation of dynamic payload types in all UniMRCP-based servers unless people are willing to configure their clients to use the server payload types.

schlagert commented 5 years ago

Seems like there might be an easy fix available when looking the Sofia-SIP module in FreeSWITCH... followup in #249.

achaloyan commented 5 years ago

Well. Thanks, but this would be a quite radical change. It is not a matter of interoperability between UniMRCP client and server only. There are various other platforms that I personally may not currently have access to re-validate all the possible cases.

schlagert commented 5 years ago

I know it looks scary but keep in mind that MRCPv2-based dynamic payload type negotiation is not working anyways (unless you luckily/accidentially configured the same payload types).

Since I'm working with SIP for quite a long time now, I know about the complexity of the SDP Offer/Answer model. Disabling SOA might break some use cases, e.g. UPDATE-handling, black hole media, etc. As you mentioned, this needs to be tested against clients using these features. Maybe someone from the community could help out with testing such cases.

Furthermore, I've noticed that disabling SOA breaks the SDP origin uniqueness and does also not handle SDP versioning. However, the SDP origin is also not unique when using MRCPv1 where this is not considered a problem. Another example for this behaviour would be the LumenVox MRCPv2 client which does also not send a unique SDP origin (session id and version both 0).

I'm integrating third party MRCP services into our media server and this is the second time I could not make a UniMRCP-based product working with our (admittedly UniMRCP-based) client. Luckily, I had a closer look at the SDP this time...

schlagert commented 5 years ago

This is the corresponding FreeSWITCH-Bug.

achaloyan commented 5 years ago

Your concerns are clear to me.

I started my career with SIP/RTP development in 2001, much earlier than I got to know what MRCP is, and am familiar with every bit of RFC 3264. It is true that dynamic RTP payload types are used widely in the SIP world. However, the same is not true for SIP/MRCPv2, where dynamic RTP payload types are used mostly for telephone-event, and very rarely for anything else.

Your are right that the negotiation of dynamic payload types is broken. However, the workaround is as easy as specifying the same payload type on both sides. Note, that as opposed to a generic SIP user agent, where it is not clear what the other peer is, in the great majority of MRCP deployments, [Uni]MRCP server is installed with a certain IVR platform, be it Genesys, Avaya, Cisco, or UniMRCP client based IVR.

As for issues you have encountered with UniMRCP, I have personally trying to help you as much as I can, mostly because your and everyone else's input to the project is valuable to me.

Moving back to the issue, I still believe it would be unwise from my side to apply this change as is, which does not mean there is anything wrong with your proposed change. However, if you could disable SOA based on a configuration parameter, then that would be a much safer approach. I can take a look at this myself too, but cannot guarantee any timelines.

schlagert commented 5 years ago

Sorry, for coming across rude in my comment! I didn't mean to offend you in any way, so please accept my apologies for that. Furthermore, I didn't have any doubt about your competence in the field, having written this project proves this more than enough. UniMRCP is a great and important project and I especially honor its high quality standards along with your dedication to help. This also includes the necessity to reject contributions that don't meet these standards.

I agree with you about the role of dynamic payload types in the MRCP world and I probably shouldn't have made such a big deal about it. I have/had to integrate some smaller players in the recognition field which rely on uncompressed L16 and thus use dynamic payload types.

I think a configuration parameter for disabling SOA is a good compromise and I would be glad to adapt my PR accordingly as soon as I can spare the time for it.

achaloyan commented 5 years ago

Thanks for your understanding. No worries.

schlagert commented 5 years ago

If anybody discovers similar problems and cannot adapt payload types, the configuration in #249 may be used as a workaround.