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

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index c628e60f0ab40f6530214a5b20fffa5ce8372057..04fd57e02b71dcffa0898b4c61116449d13c5a1d 100644 (file)
--- 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);