[Home]

Summary:ASTERISK-27151: chan_sip.c/add_sdp() systematically adding m=video line
Reporter:TLP Research (tlp.research.dpt)Labels:
Date Opened:2017-07-24 02:26:59Date Closed:
Priority:MajorRegression?
Status:Open/NewComponents:Channels/chan_sip/General
Versions:13.17.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:The following code in function {{add_sdp()}} in {{chan_sip.c}} is buggy:

{code}
/* Check if we need video in this call */
if ((ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO)) && !p->novideo) {
if (doing_directmedia && !ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO)) {
ast_debug(2, "This call needs video offers, but caller probably did not offer it!\n");
} else if (p->vrtp) {
needvideo = TRUE;
ast_debug(2, "This call needs video offers!\n");
} else {
ast_debug(2, "This call needs video offers, but there's no video support enabled!\n");
}
}
{code}

The reason is that when the following condition is evaluated:
{{if (doing_directmedia && !ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO))}}

the value returned by:
{{ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO)}}

must necessarily be true (otherwise we would have skipped the block) causing the condition to be always false.

This has the effect of falling through the {{else if}} block systematically (when video is supported) and setting {{needvideo}} to true.

This means that even when no video is offered by the caller (i.e. no {{m=video}} in the SDP of the INVITE message sent from caller to Asterisk), {{needvideo}} will still be set to true. One side effect of this bug is causing the {{m=video}} line to be appended to the SDP later on in the code of the {{add_sdp()}} function, even when no such line is necessary.

Since {{add_sdp()}} is called from many functions, specifically {{transmit_invite()}} and {{transmit_reinvite_with_sdp()}}, one higher-level effect of this bug is causing the callee to enable video even when the caller is requesting only audio, making it impossible to issue an audio-only call if {{videosupport}} is set to {{yes}} in {{sip.conf}} and the callee supports video.

Our suggested fix (which we didn't test yet) would be to replace:
{{if (doing_directmedia && !ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO))}}
with:
{{if (doing_directmedia && !ast_format_cap_has_type(p->caps, AST_MEDIA_TYPE_VIDEO))}}


Thank you
Comments:By: Asterisk Team (asteriskteam) 2017-07-24 02:27:00.319-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

By: Benjamin Keith Ford (bford) 2017-07-24 16:26:33.820-0500

[~tlp.research.dpt], would you be willing to work on, test, and submit a patch \[1] for this? chan_sip is under extended support \[2], so the fastest way to a resolution is through your contribution.

\[1]: https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage
\[2]: https://wiki.asterisk.org/wiki/display/AST/Asterisk+Module+Support+States