[svn-commits] mjordan: branch 13 r432195 - in /branches/13: channels/ main/ res/ res/ari/
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Tue Feb 24 15:58:39 CST 2015
Author: mjordan
Date: Tue Feb 24 15:58:35 2015
New Revision: 432195
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=432195
Log:
ARI/PJSIP: Apply requesting channel's format cap to created channels
This patch addresses the following problems:
* ari/resource_channels: In ARI, we currently create a format capability
structure of SLIN and apply it to the new channel being created. This was
originally done when the PBX core was used to create the channel, as there
was a condition where a newly created channel could be created without any
formats. Unfortunately, now that the Dial API is being used, this has two
drawbacks:
(a) SLIN, while it will ensure audio will flows, can cause a lot of
needless transcodings to occur, particularly when a Local channel is
created to the dialplan. When no format capabilities are available, the
Dial API handles this better by handing all audio formats to the requsted
channels. As such, we defer to that API to provide the format
capabilities.
(b) If a channel (requester) is causing this channel to be created, we
currently don't use its format capabilities as we are passing in our own.
However, the Dial API will use the requester channel's formats if none
are passed into it, and the requester channel exists and has format
capabilities. This is the "best" scenario, as it is the most likely to
create a media path that minimizes transcoding.
Fixing this simply entails removing the providing of the format capabilities
structure to the Dial API.
* chan_pjsip: Rather than blindly picking the first format in the format
capability structure - which actually *can* be a video or text format - we
select an audio format, and only pick the first format if that fails. That
minimizes the weird scenario where we attempt to transcode between video/audio.
* res_pjsip_sdp_rtp: Applied the joint capapbilites to the format structure.
Since ast_request already limits us down to one format capability once the
format capabilities are passed along, there's no reason to squelch it here.
* channel: Fixed a comment. The reason we have to minimize our requested
format capabilities down to a single format is due to Asterisk's inability
to convey the format to be used back "up" a channel chain. Consider the
following:
PJSIP/A => L;1 <=> L;2 => PJSIP/B
g,u,a g,u,a g,u,a u
That is, we have PJSIP/A dialing a Local channel, where the Local;2 dials
PJSIP/B. PJSIP/A has native format capabilities g722,ulaw,alaw; the Local
channel has inherited those format capabilities down the line; PJSIP/B
supports only ulaw. According to these format capabilities, ulaw is
acceptable and should be selected across all the channels, and no
transcoding should occur. However, there is no way to convey this: when L;2
and PJSIP/B are put into a bridge, we will select ulaw, but that is not
conveyed to PJSIP/A and L;1. Thus, we end up with:
PJSIP/A <=> L;1 <=> L;2 <=> PJSIP/B
g g X u u
Which causes g722 to be written to PJSIP/B.
Even if we can convey the 'ulaw' choice back up the chain (which through
some severe hacking in Local channels was accomplished), such that the chain
looks like:
PJSIP/A <=> L;1 <=> L;2 <=> PJSIP/B
u u u u
We have no way to tell PJSIP/A's *channel driver* to Answer in the SDP back
with only 'ulaw'. This results in all the channel structures being set up
correctly, but PJSIP/A *still* sending g722 and causing the chain to fall
apart.
There's a lot of difficulty just in setting this up, as there are numerous
race conditions in the act of bridging, and no clean mechanism to pass the
selected format backwards down an established channel chain. As such, the
best that can be done at this point in time is clarifying the comment.
Review: https://reviewboard.asterisk.org/r/4434/
ASTERISK-24812 #close
Reported by: Matt Jordan
Modified:
branches/13/channels/chan_pjsip.c
branches/13/main/channel.c
branches/13/res/ari/resource_channels.c
branches/13/res/res_pjsip_sdp_rtp.c
Modified: branches/13/channels/chan_pjsip.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/channels/chan_pjsip.c?view=diff&rev=432195&r1=432194&r2=432195
==============================================================================
--- branches/13/channels/chan_pjsip.c (original)
+++ branches/13/channels/chan_pjsip.c Tue Feb 24 15:58:35 2015
@@ -418,12 +418,13 @@
ast_channel_nativeformats_set(chan, caps);
if (!ast_format_cap_empty(caps)) {
- /*
- * XXX Probably should pick the first audio codec instead
- * of simply the first codec. The first codec may be video.
- */
- struct ast_format *fmt = ast_format_cap_get_format(caps, 0);
-
+ struct ast_format *fmt;
+
+ fmt = ast_format_cap_get_best_by_type(caps, AST_MEDIA_TYPE_AUDIO);
+ if (!fmt) {
+ /* Since our capabilities aren't empty, this will succeed */
+ fmt = ast_format_cap_get_format(caps, 0);
+ }
ast_channel_set_writeformat(chan, fmt);
ast_channel_set_rawwriteformat(chan, fmt);
ast_channel_set_readformat(chan, fmt);
Modified: branches/13/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/main/channel.c?view=diff&rev=432195&r1=432194&r2=432195
==============================================================================
--- branches/13/main/channel.c (original)
+++ branches/13/main/channel.c Tue Feb 24 15:58:35 2015
@@ -5883,9 +5883,10 @@
return NULL;
/* XXX Only the audio format calculated as being the best for translation
- * purposes is used for the request. This needs to be re-evaluated. It may be
- * a better choice to send all the audio formats capable of being translated
- * during the request and allow the channel drivers to pick the best one. */
+ * purposes is used for the request. This is because we don't have the ability
+ * to signal to the initiator which one of their codecs that was offered is
+ * the one that was selected, particularly in a chain of Local channels.
+ */
joint_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
if (!joint_cap) {
return NULL;
Modified: branches/13/res/ari/resource_channels.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/ari/resource_channels.c?view=diff&rev=432195&r1=432194&r2=432195
==============================================================================
--- branches/13/res/ari/resource_channels.c (original)
+++ branches/13/res/ari/resource_channels.c Tue Feb 24 15:58:35 2015
@@ -917,8 +917,6 @@
char *caller_id = NULL;
char *cid_num = NULL;
char *cid_name = NULL;
- RAII_VAR(struct ast_format_cap *, cap,
- ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT), ao2_cleanup);
char *stuff;
struct ast_channel *other = NULL;
struct ast_channel *chan = NULL;
@@ -930,12 +928,6 @@
struct ari_origination *origination;
pthread_t thread;
- if (!cap) {
- ast_ari_response_alloc_failed(response);
- return;
- }
- ast_format_cap_append(cap, ast_format_slin, 0);
-
if ((assignedids.uniqueid && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid))
|| (assignedids.uniqueid2 && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid2))) {
ast_ari_response_error(response, 400, "Bad Request",
@@ -1071,7 +1063,7 @@
}
}
- if (ast_dial_prerun(dial, other, cap)) {
+ if (ast_dial_prerun(dial, other, NULL)) {
ast_ari_response_alloc_failed(response);
ast_dial_destroy(dial);
ast_free(origination);
Modified: branches/13/res/res_pjsip_sdp_rtp.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_sdp_rtp.c?view=diff&rev=432195&r1=432194&r2=432195
==============================================================================
--- branches/13/res/res_pjsip_sdp_rtp.c (original)
+++ branches/13/res/res_pjsip_sdp_rtp.c Tue Feb 24 15:58:35 2015
@@ -240,8 +240,8 @@
/* get the joint capabilities between peer and endpoint */
ast_format_cap_get_compatible(caps, peer, joint);
if (!ast_format_cap_count(joint)) {
- struct ast_str *usbuf = ast_str_alloca(64);
- struct ast_str *thembuf = ast_str_alloca(64);
+ struct ast_str *usbuf = ast_str_alloca(256);
+ struct ast_str *thembuf = ast_str_alloca(256);
ast_rtp_codecs_payloads_destroy(&codecs);
ast_log(LOG_NOTICE, "No joint capabilities for '%s' media stream between our configuration(%s) and incoming SDP(%s)\n",
@@ -257,36 +257,17 @@
ast_format_cap_append_from_cap(session->req_caps, joint, AST_MEDIA_TYPE_UNKNOWN);
if (session->channel) {
- struct ast_format *fmt;
ast_channel_lock(session->channel);
- ast_format_cap_remove_by_type(caps, AST_MEDIA_TYPE_UNKNOWN);
- ast_format_cap_append_from_cap(caps, ast_channel_nativeformats(session->channel), AST_MEDIA_TYPE_UNKNOWN);
- ast_format_cap_remove_by_type(caps, media_type);
-
- /*
- * XXX Historically we picked the "best" joint format to use
- * and stuck with it. It would be nice to just append the
- * determined joint media capabilities to give translation
- * more formats to choose from when necessary. Unfortunately,
- * there are some areas of the system where this doesn't work
- * very well. (The softmix bridge in particular is reluctant
- * to pick higher fidelity formats and has a problem with
- * asymmetric sample rates.)
- */
- fmt = ast_format_cap_get_format(joint, 0);
- ast_format_cap_append(caps, fmt, 0);
/*
* Apply the new formats to the channel, potentially changing
* raw read/write formats and translation path while doing so.
*/
- ast_channel_nativeformats_set(session->channel, caps);
+ ast_channel_nativeformats_set(session->channel, joint);
ast_set_read_format(session->channel, ast_channel_readformat(session->channel));
ast_set_write_format(session->channel, ast_channel_writeformat(session->channel));
ast_channel_unlock(session->channel);
-
- ao2_ref(fmt, -1);
}
ast_rtp_codecs_payloads_destroy(&codecs);
More information about the svn-commits
mailing list