From 2253fa052d558866b03e04fe2471feb0663cd043 Mon Sep 17 00:00:00 2001 From: Naveen Albert Date: Thu, 9 Jan 2025 20:49:14 -0500 Subject: [PATCH] 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 73103bdcd5b342ce5dfa32039333ffadad551151 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 ea3ac94bbfa0dea90b9516713140349a8a95c8c3) --- channels/chan_iax2.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c index c628e60f0a..04fd57e02b 100644 --- a/channels/chan_iax2.c +++ b/channels/chan_iax2.c @@ -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);