SDP filtering issues with sdpops module

[Ozzyboshi]

In order to filter some specific codecs from the SDP I am using this functions from the sdpops module

• sdp_keep_codecs_by_name()
• sdp_remove_codecs_by_name()
• sdp_keep_codecs_by_id()
• sdp_remove_codecs_by_id()

and since they use the same code base for filtering, the problem I am going to describe affects all of them.

Let's say I have this data inside my SDP

...
a=rtcp-xr:rcvr-rtt=all:10000 stat-summary=loss,dup,jitt,TTL voip-metrics
m=audio 7078 RTP/AVPF 96 97 98 0 8 3 9 18 101 99 100
a=rtpmap:96 opus/48000/2
a=fmtp:96 useinbandfec=1
a=rtpmap:97 speex/16000
a=fmtp:97 vbr=on
a=rtpmap:98 speex/8000
a=fmtp:98 vbr=on
a=fmtp:18 annexb=yes
a=rtpmap:101 telephone-event/48000
a=rtpmap:99 telephone-event/16000
a=rtpmap:100 telephone-event/8000
a=rtcp-fb:* trr-int 1000
a=rtcp-fb:* ccm tmmbr
m=video 9078 RTP/AVPF 96 97
a=rtpmap:96 VP8/90000
a=rtpmap:97 H264/90000
a=fmtp:97 profile-level-id=42801F
a=rtcp-fb:* trr-int 1000
a=rtcp-fb:* ccm tmmbr
a=rtcp-fb:96 nack pli
a=rtcp-fb:96 nack sli
a=rtcp-fb:96 ack rpsi
a=rtcp-fb:96 ccm fir
a=rtcp-fb:97 nack pli
a=rtcp-fb:97 ccm fir

The sdp_keep_codecs_by_id (and I assume all other analog filter functions) correctly detects 2 SDP sessions, one for the audio and one for the video.

Now, let's say we want to allow only PCMA,PCMU,GSM,telephone-event ( id 8,0,3,100,99,101 ), this means that 96 and 97 codecs have to be stripped.

For this reason the functions sdp_remove_str_codec_id_attrs() and sdp_remove_str_codec_id() are executed passing the correct id we want to filter.
The sdp_remove_str_codec_id_attrs() will then remove the corresponding lines from the SDP but ONLY IF relative to rtpmap and fmtp.
This means that a=rtcp* lines will not be filtered even if they are relative to a filtered codec and this is not what the user expects.

Interestingly the sdp_remove_str_codec_id_attrs() function deletes the lines only if there is a match with the sdp_payload_attr linked list.

kamailio/src/core/parser/sdp/sdp.h

Line 90 in 758c85a

struct sdp_payload_attr *payload_attr;

This linked list is built at core level and only exposes rtp_payload and fmtp_string as described here.

kamailio/src/core/parser/sdp/sdp.h

Line 39 in 758c85a

typedef struct sdp_payload_attr {

So, in other words, the sdpops module has no clue of "a=rtcp-fb*" attributes and cannot be implemented to filter them in a neat way.

Another issue arises for the "m=video 9078 RTP/AVPF 96 97" line.
Here, since we are filtering 96 and 97, the line remains without ids, maybe it would be better to delete the whole line?

The final result relative to the video session, after applying my filter, is the following

m=video 23088 RTP/AVPF
a=rtcp-fb:* trr-int 1000
a=rtcp-fb:* ccm tmmbr
a=rtcp-fb:96 nack pli
a=rtcp-fb:96 nack sli
a=rtcp-fb:96 ack rpsi
a=rtcp-fb:96 ccm fir
a=rtcp-fb:97 nack pli
a=rtcp-fb:97 ccm fir

[miconda]

The code needs to be updated to be able to catch a=rtcp-fb:, probably it was not a common attribute when the sdp parser was written and not covered at that time (iirc, it was @ovidiusas writing the sdp parser).

The solution right now is to use subst_body() function to remove lines from sdp matching a=rtcp-fb:ID, replacing the ID with the corresponding codec numeric ids:

• https://www.kamailio.org/docs/modules/stable/modules/textops.html#textops.f.subst_body

If you do not want video media stream, there is a function to remove it all together:

• https://www.kamailio.org/docs/modules/stable/modules/sdpops.html#sdpops.f.sdp_with_media

[miconda]

Actually instead of subst_body() from textops, which I suggested in the previous comment, the sdpops module offers a more convenient function to remove lines from sdp that match a prefix:

• https://www.kamailio.org/docs/modules/stable/modules/sdpops.html#sdpops.f.remove_line_by_prefix

[Ozzyboshi]

thanks a lot @miconda and yes... for now I will try to write a workaround using remove_line_by_prefix inside my kamailio config files, thx for your suggestions.

[miconda]

It should be fixed by the commit referenced above. I did some basic tests, but if still fails, reopen.