chan_iax2: Avoid unnecessarily backlogging non-voice frames.

Currently, when receiving an unauthenticated call, we keep track
of the negotiated format in the chosenformat, which allows us
to later create the channel using the right format. However,
this was not done for authenticated calls. This meant that in
certain circumstances, if we had not yet received a voice frame
from the peer, only certain other types of frames (e.g. text),
there were no variables containing the appropriate frame.
This led to problems in the jitterbuffer callback where we
unnecessarily bailed out of retrieving a frame from the jitterbuffer.
This was logic intentionally added in commit 73103bdcd5
in response to an earlier regression, and while this prevents
crashes, it also backlogs legitimate frames unnecessarily.

The abort logic was initially added because at this point in the
code, we did not have the negotiated format available to us.
However, it should always be available to us as a last resort
in chosenformat, so we now pull it from there if needed. This
allows us to process frames the jitterbuffer even if voicefmt
and peerfmt aren't set and still avoid the crash. The failsafe
logic is retained, but now it shouldn't be triggered anymore.

Resolves: #1054
(cherry picked from commit ea3ac94bbf)
releases/21
Naveen Albert 4 months ago committed by Asterisk Development Team
parent ad49fffd54
commit 2253fa052d

@ -798,7 +798,8 @@ struct chan_iax2_pvt {
unsigned short peercallno;
/*! Negotiated format, this is only used to remember what format was
chosen for an unauthenticated call so that the channel can get
created later using the right format */
created later using the right format. We also use it for
authenticated calls to check the format from __get_from_jb. */
iax2_format chosenformat;
/*! Peer selected format */
iax2_format peerformat;
@ -4217,10 +4218,24 @@ static void __get_from_jb(const void *p)
* In this case, fall back to using the format negotiated during call setup,
* so we don't stall the jitterbuffer completely. */
voicefmt = ast_format_compatibility_bitfield2format(pvt->peerformat);
if (!voicefmt) {
/* As a last resort, we can use pvt->chosenformat.
* This is set when we receive a call (either authenticated or unauthenticated),
* so even if we haven't received any voice frames yet, we can still use the
* right format.
*
* If we have to do this, in most cases, we aren't even processing voice frames
* anyways, it's likely a non-voice frame. In that case, the format doesn't
* really matter so much, because we could just pass 20 to jb_get instead
* of calling ast_format_get_default_ms. However, until jb_get returns,
* we don't actually know what kind of frame it is for sure, so use
* the right format just to be safe. */
voicefmt = ast_format_compatibility_bitfield2format(pvt->chosenformat);
}
}
if (!voicefmt) {
/* Really shouldn't happen, but if it does, should be looked into */
ast_log(LOG_WARNING, "No voice format and no peer format available on %s, backlogging frame\n", ast_channel_name(pvt->owner));
/* This should never happen, since we should always be able to have an acceptable format to use. */
ast_log(LOG_ERROR, "No voice, peer, or chosen format available on %s, backlogging frame\n", ast_channel_name(pvt->owner));
goto cleanup; /* Don't crash if there's no voice format */
}
ret = jb_get(pvt->jb, &frame, ms, ast_format_get_default_ms(voicefmt));
@ -11569,6 +11584,11 @@ static int socket_process_helper(struct iax2_thread *thread)
VERBOSE_PREFIX_4,
using_prefs);
/* Unlike unauthenticated calls, we don't need to store
* the chosen format for channel creation.
* However, this is helpful for __get_from_jb. */
iaxs[fr->callno]->chosenformat = format;
ast_set_flag(&iaxs[fr->callno]->state, IAX_STATE_STARTED);
c = ast_iax2_new(fr->callno, AST_STATE_RING, format,
&iaxs[fr->callno]->rprefs, NULL, NULL, 1);

Loading…
Cancel
Save