Author: rmudgett Date: Fri Apr 10 18:37:20 2015 New Revision: 434672 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=434672 Log: chan_pjsip/res_pjsip/bridge_softmix/core: Improve translation path choices.
With this patch, chan_pjsip/res_pjsip now sets the native formats to the codecs negotiated by a call. * The changes in chan_pjsip.c and res_pjsip_sdp_rtp.c set the native formats to include all the negotiated audio codecs instead of only the initial preferred audio codec and later the currently received audio codec. * The audio frame handling in channel.c:ast_read() is more streamlined and will automatically adjust to changes in received frame formats. The new policy is to remove translation and pass the new frame format to the receiver except if the translation was to a signed linear format. A more long winded version is commented in ast_read() along with some caveats. * The audio frame handling in channel.c:ast_write() is more streamlined and will automatically adjust any needed translation to changes in the frame formats sent. Frame formats sent can change for many reasons such as a recording is being played back or the bridged peer changed the format it sends. Since it is a normal expectation that sent formats can change, the codec mismatch warning message is demoted to a debug message. * Removed the short circuit check in channel.c:ast_channel_make_compatible_helper(). Two party bridges need to make channels compatible with each other. However, transfers and moving channels among bridges can result in otherwise compatible channels having sub-optimal translation paths if the make compatible check is short circuited. A result of forcing the reevaluation of channel compatibility is that the asterisk.conf:transcode_via_slin and codecs.conf:genericplc options take effect consistently now. It is unfortunate that these two options are enabled by default and negate some of the benefits to the changes in channel.c:ast_read() by forcing translation through signed linear on a two party bridge. * Improved the softmix bridge technology to better control the translation of frames to the bridge. All of the incoming translation is now normally handled by ast_read() instead of splitting any translation steps between ast_read() and the slin factory. If any frame comes in with an unexpected format then the translation path in ast_read() is updated for the next frame and the slin factory handles the current frame translation. This is the final patch in a series of patches aimed at improving translation path choices. The other patches are on the following reviews: https://reviewboard.asterisk.org/r/4600/ https://reviewboard.asterisk.org/r/4605/ ASTERISK-24841 #close Reported by: Matt Jordan Review: https://reviewboard.asterisk.org/r/4609/ ........ Merged revisions 434671 from http://svn.asterisk.org/svn/asterisk/branches/13 Modified: trunk/ (props changed) trunk/bridges/bridge_softmix.c trunk/channels/chan_pjsip.c trunk/include/asterisk/channel.h trunk/main/channel.c trunk/res/res_pjsip_sdp_rtp.c Propchange: trunk/ ------------------------------------------------------------------------------ Binary property 'branch-13-merged' - no diff available. Modified: trunk/bridges/bridge_softmix.c URL: http://svnview.digium.com/svn/asterisk/trunk/bridges/bridge_softmix.c?view=diff&rev=434672&r1=434671&r2=434672 ============================================================================== --- trunk/bridges/bridge_softmix.c (original) +++ trunk/bridges/bridge_softmix.c Fri Apr 10 18:37:20 2015 @@ -57,6 +57,9 @@ #define MAX_DATALEN 8096 +/*! The minimum sample rate of the bridge. */ +#define SOFTMIX_MIN_SAMPLE_RATE 8000 /* 8 kHz sample rate */ + /*! \brief Interval at which mixing will take place. Valid options are 10, 20, and 40. */ #define DEFAULT_SOFTMIX_INTERVAL 20 @@ -96,8 +99,8 @@ struct ast_slinfactory factory; /*! Frame that contains mixed audio to be written out to the channel */ struct ast_frame write_frame; - /*! Frame that contains mixed audio read from the channel */ - struct ast_frame read_frame; + /*! Current expected read slinear format. */ + struct ast_format *read_slin_format; /*! DSP for detecting silence */ struct ast_dsp *dsp; /*! @@ -353,7 +356,9 @@ static void set_softmix_bridge_data(int rate, int interval, struct ast_bridge_channel *bridge_channel, int reset) { struct softmix_channel *sc = bridge_channel->tech_pvt; - unsigned int channel_read_rate = ast_format_get_sample_rate(ast_channel_rawreadformat(bridge_channel->chan)); + struct ast_format *slin_format; + + slin_format = ast_format_cache_get_slin_by_rate(rate); ast_mutex_lock(&sc->lock); if (reset) { @@ -369,31 +374,29 @@ * for the channel. The translated format may not be a * static cached format. */ - ao2_replace(sc->write_frame.subclass.format, ast_format_cache_get_slin_by_rate(rate)); + ao2_replace(sc->write_frame.subclass.format, slin_format); sc->write_frame.data.ptr = sc->final_buf; sc->write_frame.datalen = SOFTMIX_DATALEN(rate, interval); sc->write_frame.samples = SOFTMIX_SAMPLES(rate, interval); - /* Setup read frame parameters */ - sc->read_frame.frametype = AST_FRAME_VOICE; /* - * NOTE: The read_frame format does not hold a reference because it + * NOTE: The read_slin_format does not hold a reference because it * will always be a signed linear format. */ - sc->read_frame.subclass.format = ast_format_cache_get_slin_by_rate(channel_read_rate); - sc->read_frame.data.ptr = sc->our_buf; - sc->read_frame.datalen = SOFTMIX_DATALEN(channel_read_rate, interval); - sc->read_frame.samples = SOFTMIX_SAMPLES(channel_read_rate, interval); + sc->read_slin_format = slin_format; /* Setup smoother */ - ast_slinfactory_init_with_format(&sc->factory, sc->write_frame.subclass.format); + ast_slinfactory_init_with_format(&sc->factory, slin_format); /* set new read and write formats on channel. */ - ast_set_read_format(bridge_channel->chan, sc->read_frame.subclass.format); - ast_set_write_format(bridge_channel->chan, sc->write_frame.subclass.format); + ast_channel_lock(bridge_channel->chan); + ast_set_read_format_path(bridge_channel->chan, + ast_channel_rawreadformat(bridge_channel->chan), slin_format); + ast_channel_unlock(bridge_channel->chan); + ast_set_write_format(bridge_channel->chan, slin_format); /* set up new DSP. This is on the read side only right before the read frame enters the smoother. */ - sc->dsp = ast_dsp_new_with_rate(channel_read_rate); + sc->dsp = ast_dsp_new_with_rate(rate); /* we want to aggressively detect silence to avoid feedback */ if (bridge_channel->tech_args.talking_threshold) { ast_dsp_set_threshold(sc->dsp, bridge_channel->tech_args.talking_threshold); @@ -591,6 +594,28 @@ /* Write the frame into the conference */ ast_mutex_lock(&sc->lock); + + if (ast_format_cmp(frame->subclass.format, sc->read_slin_format) != AST_FORMAT_CMP_EQUAL) { + /* + * The incoming frame is not the expected format. Update + * the channel's translation path to get us slinear from + * the new format for the next frame. + * + * There is the possibility that this frame is an old slinear + * rate frame that was in flight when the softmix bridge + * changed rates. If so it will self correct on subsequent + * frames. + */ + ast_channel_lock(bridge_channel->chan); + ast_debug(1, "Channel %s wrote unexpected format into bridge. Got %s, expected %s.\n", + ast_channel_name(bridge_channel->chan), + ast_format_get_name(frame->subclass.format), + ast_format_get_name(sc->read_slin_format)); + ast_set_read_format_path(bridge_channel->chan, frame->subclass.format, + sc->read_slin_format); + ast_channel_unlock(bridge_channel->chan); + } + ast_dsp_silence_with_energy(sc->dsp, frame, &totalsilence, &cur_energy); if (bridge->softmix.video_mode.mode == AST_BRIDGE_VIDEO_MODE_TALKER_SRC) { @@ -729,15 +754,19 @@ struct ast_bridge_channel *bridge_channel) { int channel_native_rate; - int i; + /* Gather stats about channel sample rates. */ - channel_native_rate = MAX(ast_format_get_sample_rate(ast_channel_rawwriteformat(bridge_channel->chan)), + ast_channel_lock(bridge_channel->chan); + channel_native_rate = MAX(SOFTMIX_MIN_SAMPLE_RATE, ast_format_get_sample_rate(ast_channel_rawreadformat(bridge_channel->chan))); - - if (channel_native_rate > stats->highest_supported_rate) { + ast_channel_unlock(bridge_channel->chan); + + if (stats->highest_supported_rate < channel_native_rate) { stats->highest_supported_rate = channel_native_rate; } - if (channel_native_rate > softmix_data->internal_rate) { + if (softmix_data->internal_rate < channel_native_rate) { + int i; + for (i = 0; i < ARRAY_LEN(stats->sample_rates); i++) { if (stats->sample_rates[i] == channel_native_rate) { stats->num_channels[i]++; @@ -749,7 +778,7 @@ } } stats->num_above_internal_rate++; - } else if (channel_native_rate == softmix_data->internal_rate) { + } else if (softmix_data->internal_rate == channel_native_rate) { stats->num_at_internal_rate++; } } @@ -1119,8 +1148,8 @@ softmix_bridge_data_destroy(softmix_data); return -1; } - /* start at 8khz, let it grow from there */ - softmix_data->internal_rate = 8000; + /* start at minimum rate, let it grow from there */ + softmix_data->internal_rate = SOFTMIX_MIN_SAMPLE_RATE; softmix_data->internal_mixing_interval = DEFAULT_SOFTMIX_INTERVAL; bridge->tech_pvt = softmix_data; Modified: trunk/channels/chan_pjsip.c URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_pjsip.c?view=diff&rev=434672&r1=434671&r2=434672 ============================================================================== --- trunk/channels/chan_pjsip.c (original) +++ trunk/channels/chan_pjsip.c Fri Apr 10 18:37:20 2015 @@ -627,22 +627,6 @@ if (f->frametype != AST_FRAME_VOICE) { return f; - } - - if (ast_format_cap_iscompatible_format(ast_channel_nativeformats(ast), f->subclass.format) == AST_FORMAT_CMP_NOT_EQUAL) { - struct ast_format_cap *caps; - - ast_debug(1, "Oooh, format changed to %s\n", ast_format_get_name(f->subclass.format)); - - caps = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT); - if (caps) { - ast_format_cap_append(caps, f->subclass.format, 0); - ast_channel_nativeformats_set(ast, caps); - ao2_ref(caps, -1); - } - - ast_set_read_format(ast, ast_channel_readformat(ast)); - ast_set_write_format(ast, ast_channel_writeformat(ast)); } if (channel->session->dsp) { Modified: trunk/include/asterisk/channel.h URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/channel.h?view=diff&rev=434672&r1=434671&r2=434672 ============================================================================== --- trunk/include/asterisk/channel.h (original) +++ trunk/include/asterisk/channel.h Fri Apr 10 18:37:20 2015 @@ -1951,6 +1951,21 @@ /*! \brief Send empty audio to prime a channel driver */ int ast_prod(struct ast_channel *chan); + +/*! + * \brief Set specific read path on channel. + * \since 13.4.0 + * + * \param chan Channel to setup read path. + * \param raw_format Format to expect from the channel driver. + * \param core_format What the core wants to read. + * + * \pre chan is locked + * + * \retval 0 on success. + * \retval -1 on error. + */ +int ast_set_read_format_path(struct ast_channel *chan, struct ast_format *raw_format, struct ast_format *core_format); /*! * \brief Sets read format on channel chan from capabilities Modified: trunk/main/channel.c URL: http://svnview.digium.com/svn/asterisk/trunk/main/channel.c?view=diff&rev=434672&r1=434671&r2=434672 ============================================================================== --- trunk/main/channel.c (original) +++ trunk/main/channel.c Fri Apr 10 18:37:20 2015 @@ -4114,78 +4114,133 @@ ast_frfree(f); f = &ast_null_frame; } - } else if (f->frametype == AST_FRAME_VOICE && ast_format_cap_iscompatible_format(ast_channel_nativeformats(chan), f->subclass.format) == AST_FORMAT_CMP_NOT_EQUAL) { - /* This frame is not one of the current native formats -- drop it on the floor */ - struct ast_str *codec_buf = ast_str_alloca(64); - ast_log(LOG_NOTICE, "Dropping incompatible voice frame on %s of format %s since our native format has changed to %s\n", - ast_channel_name(chan), ast_format_get_name(f->subclass.format), ast_format_cap_get_names(ast_channel_nativeformats(chan), &codec_buf)); - ast_frfree(f); - f = &ast_null_frame; - } else if (f->frametype == AST_FRAME_VOICE) { - /* Send frame to audiohooks if present */ - if (ast_channel_audiohooks(chan)) { - struct ast_frame *old_frame = f; - f = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_READ, f); - if (old_frame != f) - ast_frfree(old_frame); + break; + } + if (f->frametype != AST_FRAME_VOICE) { + break; + } + if (ast_format_cmp(f->subclass.format, ast_channel_rawreadformat(chan)) != AST_FORMAT_CMP_EQUAL + && ast_format_cmp(f->subclass.format, ast_channel_readformat(chan)) != AST_FORMAT_CMP_EQUAL) { + struct ast_format *core_format; + + /* + * Note: This frame may not be one of the current native + * formats. We may have gotten it out of the read queue from + * a previous multi-frame translation, from a framehook + * injected frame, or the device we're talking to isn't + * respecting negotiated formats. Regardless we will accept + * all frames. + * + * Update the read translation path to handle the new format + * that just came in. If the core wants slinear we need to + * setup a new translation path because the core is usually + * doing something with the audio itself and may not handle + * any other format. e.g., Softmix bridge, holding bridge + * announcer channel, recording, AMD... Otherwise, we'll + * setup to pass the frame as is to the core. In this case + * the core doesn't care. The channel is likely in + * autoservice, safesleep, or the channel is in a bridge. + * Let the bridge technology deal with format compatibility + * between the channels in the bridge. + * + * Beware of the transcode_via_slin and genericplc options as + * they force any transcoding to go through slin on a bridge. + * Unfortunately transcode_via_slin is enabled by default and + * genericplc is enabled in the codecs.conf.sample file. + * + * XXX Only updating translation to slinear frames has some + * corner cases if slinear is one of the native formats and + * there are different sample rates involved. We might wind + * up with conflicting translation paths between channels + * where the read translation path on this channel reduces + * the sample rate followed by a write translation path on + * the peer channel that increases the sample rate. + */ + core_format = ast_channel_readformat(chan); + if (!ast_format_cache_is_slinear(core_format)) { + core_format = f->subclass.format; } - if (ast_channel_monitor(chan) && ast_channel_monitor(chan)->read_stream ) { - /* XXX what does this do ? */ + if (ast_set_read_format_path(chan, f->subclass.format, core_format)) { + /* Drop frame. We couldn't make it compatible with the core. */ + ast_frfree(f); + f = &ast_null_frame; + break; + } + } + /* Send frame to audiohooks if present */ + if (ast_channel_audiohooks(chan)) { + struct ast_frame *old_frame = f; + + f = ast_audiohook_write_list(chan, ast_channel_audiohooks(chan), AST_AUDIOHOOK_DIRECTION_READ, f); + if (old_frame != f) { + ast_frfree(old_frame); + } + } + if (ast_channel_monitor(chan) && ast_channel_monitor(chan)->read_stream) { + /* XXX what does this do ? */ #ifndef MONITOR_CONSTANT_DELAY - int jump = ast_channel_outsmpl(chan) - ast_channel_insmpl(chan) - 4 * f->samples; - if (jump >= 0) { - jump = calc_monitor_jump((ast_channel_outsmpl(chan) - ast_channel_insmpl(chan)), - ast_format_get_sample_rate(f->subclass.format), - ast_format_get_sample_rate(ast_channel_monitor(chan)->read_stream->fmt->format)); - if (ast_seekstream(ast_channel_monitor(chan)->read_stream, jump, SEEK_FORCECUR) == -1) { - ast_log(LOG_WARNING, "Failed to perform seek in monitoring read stream, synchronization between the files may be broken\n"); - } - ast_channel_insmpl_set(chan, ast_channel_insmpl(chan) + (ast_channel_outsmpl(chan) - ast_channel_insmpl(chan)) + f->samples); - } else { - ast_channel_insmpl_set(chan, ast_channel_insmpl(chan) + f->samples); + int jump = ast_channel_outsmpl(chan) - ast_channel_insmpl(chan) - 4 * f->samples; + if (jump >= 0) { + jump = calc_monitor_jump((ast_channel_outsmpl(chan) - ast_channel_insmpl(chan)), + ast_format_get_sample_rate(f->subclass.format), + ast_format_get_sample_rate(ast_channel_monitor(chan)->read_stream->fmt->format)); + if (ast_seekstream(ast_channel_monitor(chan)->read_stream, jump, SEEK_FORCECUR) == -1) { + ast_log(LOG_WARNING, "Failed to perform seek in monitoring read stream, synchronization between the files may be broken\n"); } + ast_channel_insmpl_set(chan, ast_channel_insmpl(chan) + (ast_channel_outsmpl(chan) - ast_channel_insmpl(chan)) + f->samples); + } else { + ast_channel_insmpl_set(chan, ast_channel_insmpl(chan) + f->samples); + } #else - int jump = calc_monitor_jump((ast_channel_outsmpl(chan) - ast_channel_insmpl(chan)), - ast_format_get_sample_rate(f->subclass.codec), - ast_format_get_sample_rate(ast_channel_monitor(chan)->read_stream->fmt->format)); - if (jump - MONITOR_DELAY >= 0) { - if (ast_seekstream(ast_channel_monitor(chan)->read_stream, jump - f->samples, SEEK_FORCECUR) == -1) - ast_log(LOG_WARNING, "Failed to perform seek in monitoring read stream, synchronization between the files may be broken\n"); - ast_channel_insmpl(chan) += ast_channel_outsmpl(chan) - ast_channel_insmpl(chan); - } else - ast_channel_insmpl(chan) += f->samples; + int jump = calc_monitor_jump((ast_channel_outsmpl(chan) - ast_channel_insmpl(chan)), + ast_format_get_sample_rate(f->subclass.codec), + ast_format_get_sample_rate(ast_channel_monitor(chan)->read_stream->fmt->format)); + if (jump - MONITOR_DELAY >= 0) { + if (ast_seekstream(ast_channel_monitor(chan)->read_stream, jump - f->samples, SEEK_FORCECUR) == -1) { + ast_log(LOG_WARNING, "Failed to perform seek in monitoring read stream, synchronization between the files may be broken\n"); + } + ast_channel_insmpl(chan) += ast_channel_outsmpl(chan) - ast_channel_insmpl(chan); + } else { + ast_channel_insmpl(chan) += f->samples; + } #endif - if (ast_channel_monitor(chan)->state == AST_MONITOR_RUNNING) { - if (ast_writestream(ast_channel_monitor(chan)->read_stream, f) < 0) - ast_log(LOG_WARNING, "Failed to write data to channel monitor read stream\n"); - } + if (ast_channel_monitor(chan)->state == AST_MONITOR_RUNNING) { + if (ast_writestream(ast_channel_monitor(chan)->read_stream, f) < 0) + ast_log(LOG_WARNING, "Failed to write data to channel monitor read stream\n"); } - - if (ast_channel_readtrans(chan) && (f = ast_translate(ast_channel_readtrans(chan), f, 1)) == NULL) { + } + + if (ast_channel_readtrans(chan) + && ast_format_cmp(f->subclass.format, ast_channel_rawreadformat(chan)) == AST_FORMAT_CMP_EQUAL) { + f = ast_translate(ast_channel_readtrans(chan), f, 1); + if (!f) { f = &ast_null_frame; } - - /* it is possible for the translation process on chan->readtrans to have - produced multiple frames from the single input frame we passed it; if - this happens, queue the additional frames *before* the frames we may - have queued earlier. if the readq was empty, put them at the head of - the queue, and if it was not, put them just after the frame that was - at the end of the queue. - */ - if (AST_LIST_NEXT(f, frame_list)) { - if (!readq_tail) { - ast_queue_frame_head(chan, AST_LIST_NEXT(f, frame_list)); - } else { - __ast_queue_frame(chan, AST_LIST_NEXT(f, frame_list), 0, readq_tail); - } - ast_frfree(AST_LIST_NEXT(f, frame_list)); - AST_LIST_NEXT(f, frame_list) = NULL; + } + + /* + * It is possible for the translation process on the channel to have + * produced multiple frames from the single input frame we passed it; if + * this happens, queue the additional frames *before* the frames we may + * have queued earlier. if the readq was empty, put them at the head of + * the queue, and if it was not, put them just after the frame that was + * at the end of the queue. + */ + if (AST_LIST_NEXT(f, frame_list)) { + if (!readq_tail) { + ast_queue_frame_head(chan, AST_LIST_NEXT(f, frame_list)); + } else { + __ast_queue_frame(chan, AST_LIST_NEXT(f, frame_list), 0, readq_tail); } - - /* Run generator sitting on the line if timing device not available - * and synchronous generation of outgoing frames is necessary */ - ast_read_generator_actions(chan, f); - } + ast_frfree(AST_LIST_NEXT(f, frame_list)); + AST_LIST_NEXT(f, frame_list) = NULL; + } + + /* + * Run generator sitting on the line if timing device not available + * and synchronous generation of outgoing frames is necessary + */ + ast_read_generator_actions(chan, f); break; default: /* Just pass it on! */ @@ -5034,29 +5089,35 @@ } /* If the frame is in the raw write format, then it's easy... just use the frame - otherwise we will have to translate */ - if (ast_format_cmp(fr->subclass.format, ast_channel_rawwriteformat(chan)) != AST_FORMAT_CMP_NOT_EQUAL) { + if (ast_format_cmp(fr->subclass.format, ast_channel_rawwriteformat(chan)) == AST_FORMAT_CMP_EQUAL) { f = fr; } else { - if ((ast_format_cap_iscompatible_format(ast_channel_nativeformats(chan), fr->subclass.format) == AST_FORMAT_CMP_NOT_EQUAL) && - (ast_format_cmp(ast_channel_writeformat(chan), fr->subclass.format) != AST_FORMAT_CMP_EQUAL)) { - struct ast_str *codec_buf = ast_str_alloca(64); + if (ast_format_cmp(ast_channel_writeformat(chan), fr->subclass.format) != AST_FORMAT_CMP_EQUAL) { + struct ast_str *codec_buf = ast_str_alloca(256); /* - * XXX Something is not right. We are not compatible with this - * frame. Bad things can happen. Problems range from no audio, - * one-way audio, to unexplained line hangups. As a last resort - * try to adjust the format. Ideally, we do not want to do this - * because it indicates a deeper problem. For now, we log these - * events to reduce user impact and help identify the problem - * areas. + * We are not setup to write this frame. Things may have changed + * on the peer side of the world and we try to adjust the format to + * make it compatible again. However, bad things can happen if we + * cannot setup a new translation path. Problems range from no + * audio, one-way audio, to garbled audio. The best we can do is + * request the call to hangup since we could not make it compatible. + * + * Being continuously spammed by this message likely indicates a + * problem with the peer because it cannot make up its mind about + * which format to use. */ - ast_log(LOG_WARNING, "Codec mismatch on channel %s setting write format to %s from %s native formats %s\n", - ast_channel_name(chan), ast_format_get_name(fr->subclass.format), ast_format_get_name(ast_channel_writeformat(chan)), + ast_debug(1, "Channel %s changing write format from %s to %s, native formats %s\n", + ast_channel_name(chan), + ast_format_get_name(ast_channel_writeformat(chan)), + ast_format_get_name(fr->subclass.format), ast_format_cap_get_names(ast_channel_nativeformats(chan), &codec_buf)); - ast_set_write_format(chan, fr->subclass.format); - } - - f = (ast_channel_writetrans(chan)) ? ast_translate(ast_channel_writetrans(chan), fr, 0) : fr; + if (ast_set_write_format(chan, fr->subclass.format)) { + /* Could not handle the new write format. Induce a hangup. */ + break; + } + } + f = ast_channel_writetrans(chan) ? ast_translate(ast_channel_writetrans(chan), fr, 0) : fr; } if (!f) { @@ -5216,6 +5277,42 @@ return res; } +int ast_set_read_format_path(struct ast_channel *chan, struct ast_format *raw_format, struct ast_format *core_format) +{ + struct ast_trans_pvt *trans_old; + struct ast_trans_pvt *trans_new; + + if (ast_format_cmp(ast_channel_rawreadformat(chan), raw_format) == AST_FORMAT_CMP_EQUAL + && ast_format_cmp(ast_channel_readformat(chan), core_format) == AST_FORMAT_CMP_EQUAL) { + /* Nothing to setup */ + return 0; + } + + ast_debug(1, "Channel %s setting read format path: %s -> %s\n", + ast_channel_name(chan), + ast_format_get_name(raw_format), + ast_format_get_name(core_format)); + + /* Setup new translation path. */ + if (ast_format_cmp(raw_format, core_format) != AST_FORMAT_CMP_EQUAL) { + trans_new = ast_translator_build_path(core_format, raw_format); + if (!trans_new) { + return -1; + } + } else { + /* No translation needed. */ + trans_new = NULL; + } + trans_old = ast_channel_readtrans(chan); + if (trans_old) { + ast_translator_free_path(trans_old); + } + ast_channel_readtrans_set(chan, trans_new); + ast_channel_set_rawreadformat(chan, raw_format); + ast_channel_set_readformat(chan, core_format); + return 0; +} + struct set_format_access { const char *direction; struct ast_trans_pvt *(*get_trans)(const struct ast_channel *chan); @@ -6209,15 +6306,17 @@ RAII_VAR(struct ast_format *, best_dst_fmt, NULL, ao2_cleanup); int no_path; + /* + * We cannot short circuit this code because it is possible to ask + * to make compatible two channels that are "compatible" because + * they already have translation paths setup but together make for + * a sub-optimal path. e.g., The From channel has g722 -> ulaw + * and the To channel has ulaw -> g722. They are "compatible" but + * together the translations are unnecessary and the audio loses + * fidelity in the process. + */ + ast_channel_lock_both(from, to); - - if ((ast_format_cmp(ast_channel_readformat(from), ast_channel_writeformat(to)) != AST_FORMAT_CMP_NOT_EQUAL) && - (ast_format_cmp(ast_channel_readformat(to), ast_channel_writeformat(from)) != AST_FORMAT_CMP_NOT_EQUAL)) { - /* Already compatible! Moving on ... */ - ast_channel_unlock(to); - ast_channel_unlock(from); - return 0; - } src_cap = ast_channel_nativeformats(from); /* shallow copy, do not destroy */ dst_cap = ast_channel_nativeformats(to); /* shallow copy, do not destroy */ Modified: trunk/res/res_pjsip_sdp_rtp.c URL: http://svnview.digium.com/svn/asterisk/trunk/res/res_pjsip_sdp_rtp.c?view=diff&rev=434672&r1=434671&r2=434672 ============================================================================== --- trunk/res/res_pjsip_sdp_rtp.c (original) +++ trunk/res/res_pjsip_sdp_rtp.c Fri Apr 10 18:37:20 2015 @@ -252,8 +252,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", @@ -269,33 +269,22 @@ 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_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); + ast_format_cap_append_from_cap(caps, joint, media_type); /* * 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_set_read_format(session->channel, ast_channel_readformat(session->channel)); - ast_set_write_format(session->channel, ast_channel_writeformat(session->channel)); + if (media_type == AST_MEDIA_TYPE_AUDIO) { + ast_set_read_format(session->channel, ast_channel_readformat(session->channel)); + ast_set_write_format(session->channel, ast_channel_writeformat(session->channel)); + } if ((session->endpoint->dtmf == AST_SIP_DTMF_AUTO) && (ast_rtp_instance_dtmf_mode_get(session_media->rtp) == AST_RTP_DTMF_MODE_RFC2833) && (session->dsp)) { @@ -309,8 +298,6 @@ } } ast_channel_unlock(session->channel); - - ao2_ref(fmt, -1); } ast_rtp_codecs_payloads_destroy(&codecs); -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
