search

traud / asterisk-evs

Asterisk 13 transcoding module: 3GPP EVS
The Unlicense
23 stars 10 forks source link

Issue with Samsung #2

Closed mtryfoss closed 6 years ago

mtryfoss commented 6 years ago

Hello!

Samsung (at least Galaxy S7) do not accept "evs/16000" in lower-case. All documentation I've seen has EVS in upper-case. However, I do not know if that's required.

Adding this line to rtp_engine.c did the trick: set_next_mime_type(ast_format_evs, 0, "audio", "EVS", 16000);

traud commented 6 years ago

Puh. That is a severe software bug of Samsung, see RFC 4855, section 3, last paragraph:

[…] payload format (encoding) names defined in the RTP Profile are commonly shown in upper case. MIME subtypes are commonly shown in lower case. These names are case-insensitive in both places.

The same was already stated in RFC 3555 in the year 2003. Are you able to report this to Samsung?

My 3GPP EVS module ends up with lowercase, because I use ast_rtp_engine_load_format(.). Therefore, if you want to patch that until Samsung has solved this, you have to go for

Approach A – the same approach as within my AMR module: set_next_mime_type(ast_format_evs, 0, "audio", "EVS", 16000); add_static_payload(-1, ast_format_evs, 0); instead of ast_rtp_engine_load_format(ast_format_evs);

Note: I used that approach in the AMR module not on purpose. When I created the AMR module, I did not know about ast_rtp_engine_load_format(.). Now, I cannot change to that anymore, because it would change the parameter name in Asterisk: Currently in Asterisk, you have to specify amrwb (without hyphen) in your list of allowed codecs in your configuration file sip.conf. If I go for ast_rtp_engine_load_format(.), it would change to amr-wb with an hyphen.

Approach B – uppercase always: this was removed with commit 852e763…

mtryfoss commented 6 years ago

Thanks for your feedback!

We will try reporting this to Samsung through our technical contact.

traud commented 6 years ago

Re-opening this issue, so the answer of Samsung can be tracked. Furthermore, nothing prevents us to tackle this from the Asterisk side, for example by re-adding ast_str_to_upper(.) to ast_rtp_engine_load_format(.). This has to be done not here but within the Asterisk project.

traud commented 6 years ago

This has to be done not here but within the Asterisk project.

see ASTERISK-27689…

mtryfoss commented 6 years ago

Good idea! I'll let you know if we hear anything from Samsung regarding this.

traud commented 6 years ago

The workaround was accepted and is included since Asterisk 13.20. Please, re-test whether that helps. I am closing this issue here due to lack of activity. Samsung Electronics, this is a severe issue – it is a pity that you still have not learned how to deal with software bugs after trying to be in business for so long.