res_fax: Fix latent bug exposed by ASTERISK-24841 changes.

Three fax related tests started failing as a result of changes made for
ASTERISK-24841:
tests/fax/pjsip/gateway_t38_g711
tests/fax/sip/gateway_mix1
tests/fax/sip/gateway_mix3

Historically, ast_channel_make_compatible() did nothing if the channels
were already "compatible" even if they had a sub-optimal translation path
already setup.  With the changes from ASTERISK-24841 this is no longer
true in order to allow the best translation paths to always be picked.  In
res_fax.c:fax_gateway_framehook() code manually setup the channels to go
through slin and then called ast_channel_make_compatible().  With the
previous version of ast_channel_make_compatible() this was always a
no-operation.

* Remove call to ast_channel_make_compatible() in fax_gateway_framehook()
that now undoes what was just setup when the framehook is attached.

* Fixed locking around saving the channel formats in
fax_gateway_framehook() to ensure that the formats that are saved are
consistent.

* Fix copy pasta errors in fax_gateway_framehook() that confuses read and
write when dealing with saved channel formats.

ASTERISK-24841
Reported by: Matt Jordan

Change-Id: I6fda0877104a370af586a5e8cf9e161a484da78d

diff --git a/res/res_fax.c b/res/res_fax.c
index c57f446ff267924207e1755d3b77dcbcb9916063..39cb3b369073322e7db0ee7cef4f0677d0451069 100644 (file)
--- a/res/res_fax.c
+++ b/res/res_fax.c
@@ -3291,13 +3291,13 @@ static struct ast_frame *fax_gateway_framehook(struct ast_channel *chan, struct
 
                if (gateway->bridged) {
                        ast_set_read_format(chan, gateway->chan_read_format);
-                       ast_set_read_format(chan, gateway->chan_write_format);
+                       ast_set_write_format(chan, gateway->chan_write_format);
 
                        ast_channel_unlock(chan);
                        peer = ast_channel_bridge_peer(chan);
                        if (peer) {
                                ast_set_read_format(peer, gateway->peer_read_format);
-                               ast_set_read_format(peer, gateway->peer_write_format);
+                               ast_set_write_format(peer, gateway->peer_write_format);
                                ast_channel_make_compatible(chan, peer);
                        }
                        ast_channel_lock(chan);
@@ -3340,23 +3340,25 @@ static struct ast_frame *fax_gateway_framehook(struct ast_channel *chan, struct
                        gateway->timeout_start = ast_tvnow();
                }
 
+               ast_channel_unlock(chan);
+               ast_channel_lock_both(chan, peer);
+
                /* we are bridged, change r/w formats to SLIN for v21 preamble
                 * detection and T.30 */
                ao2_replace(gateway->chan_read_format, ast_channel_readformat(chan));
-               ao2_replace(gateway->chan_write_format, ast_channel_readformat(chan));
+               ao2_replace(gateway->chan_write_format, ast_channel_writeformat(chan));
 
                ao2_replace(gateway->peer_read_format, ast_channel_readformat(peer));
-               ao2_replace(gateway->peer_write_format, ast_channel_readformat(peer));
+               ao2_replace(gateway->peer_write_format, ast_channel_writeformat(peer));
 
                ast_set_read_format(chan, ast_format_slin);
                ast_set_write_format(chan, ast_format_slin);
 
-               ast_channel_unlock(chan);
                ast_set_read_format(peer, ast_format_slin);
                ast_set_write_format(peer, ast_format_slin);
 
-               ast_channel_make_compatible(chan, peer);
-               ast_channel_lock(chan);
+               ast_channel_unlock(peer);
+
                gateway->bridged = 1;
        }