[Iaxclient-cvs] SF.net SVN: iaxclient: [1423] trunk/lib

[<jpg...@us...>]

Revision: 1423
          http://iaxclient.svn.sourceforge.net/iaxclient/?rev=1423&view=rev
Author:   jpgrayson
Date:     2008-04-24 07:18:28 -0700 (Thu, 24 Apr 2008)

Log Message:
-----------
Fix problem where media (audio, video, CNG) frames might be sent prior to the call being ACCEPTed.

When iaxclient initiates a call, it sends a NEW frame. The server end of the call must send back an ACCEPT frame prior to the call being in the "Linked" state. It is only after the call is in this Linked state that media frames may be transmitted by iaxclient. Note that media frames _are_ allowed to be transmitted prior to the call being ANSWERed (or completed in iaxclient parlance). See this thread for details:

  http://osdir.com/ml/telephony.pbx.asterisk.iaxclient.devel/2005-08/msg00021.html

This modifies iaxclient's decision logic for when it may send audio. Basically either the call has to have been answered (IAXC_CALL_STATE_COMPLETE) or the call has to be outgoing and the audio format set (which happens when the ACCEPT frame comes back from the server).
For video, we take the same approach and only allow video to go out after ACCEPT for outgoing calls and after ANSWER (complete) for incoming calls.

Without this fix, it is possible for iaxclient to send a NEW and (for example) CNG frames in short succession. The server may then receive the CNG frame first, before the NEW frame. The server sends the correct INVAL response to this call-less CNG frame which results in iaxclient destroying the call and sending a hangup event to the application. This is pretty bad, but then to make things wierder, the server will, of course, receive the NEW frame shortly after the CNG frame and respond to that with an ACCEPT frame.

C --> NEW -\/--> S
C --> CNG -/\--> S
C <-- INVAL  <-- S * server received CNG first
C <-- ACCEPT <-- S * server responds to NEW second!

Modified Paths:
--------------
    trunk/lib/iaxclient_lib.c
    trunk/lib/video.c

Modified: trunk/lib/iaxclient_lib.c
===================================================================
--- trunk/lib/iaxclient_lib.c	2008-04-24 13:24:06 UTC (rev 1422)
+++ trunk/lib/iaxclient_lib.c	2008-04-24 14:18:28 UTC (rev 1423)
@@ -832,11 +832,13 @@
 	/* TODO: maybe we shouldn't allocate 8kB on the stack here. */
 	short buf [4096];
 
+	struct iaxc_call * call =
+		selected_call >= 0 ? &calls[selected_call] : 0;
+
 	int want_send_audio =
-		selected_call >= 0 &&
-		((calls[selected_call].state & IAXC_CALL_STATE_OUTGOING) ||
-		 (calls[selected_call].state & IAXC_CALL_STATE_COMPLETE))
-		&& !(audio_prefs & IAXC_AUDIO_PREF_SEND_DISABLE);
+		call && !(audio_prefs & IAXC_AUDIO_PREF_SEND_DISABLE) &&
+		((call->state & IAXC_CALL_STATE_COMPLETE) ||
+		 (call->format && (call->state & IAXC_CALL_STATE_OUTGOING)));
 
 	int want_local_audio =
 		(audio_prefs & IAXC_AUDIO_PREF_RECV_LOCAL_RAW) ||
@@ -852,9 +854,8 @@
 			audio_driver.start(&audio_driver);
 
 			/* use codec minimum if higher */
-			cmin = want_send_audio && calls[selected_call].encoder ?
-				calls[selected_call].encoder->minimum_frame_size :
-				1;
+			cmin = want_send_audio && call->encoder ?
+				call->encoder->minimum_frame_size : 1;
 
 			to_read = cmin > minimum_outgoing_framesize ?
 				cmin : minimum_outgoing_framesize;
@@ -887,10 +888,9 @@
 						to_read * 2, (unsigned char *)buf);
 
 			if ( want_send_audio )
-				audio_send_encoded_audio(&calls[selected_call],
-						selected_call, buf,
-						calls[selected_call].format &
-							IAXC_AUDIO_FORMAT_MASK,
+				audio_send_encoded_audio(call, selected_call,
+						buf, call->format &
+						IAXC_AUDIO_FORMAT_MASK,
 						to_read);
 		}
 	}
@@ -1370,6 +1370,15 @@
 
 	codec_destroy( callNo );
 
+	/* When the ACCEPT comes back from the other-end, these formats
+	 * are set. Whether the format is set or not determines whether
+	 * we are in the Linked state (see the iax2 rfc).
+	 * These will have already been cleared by iaxc_clear_call(),
+	 * but we reset them anyway just to be pedantic.
+	 */
+	calls[callNo].format = 0;
+	calls[callNo].vformat = 0;
+
 	if ( ext )
 	{
 		strncpy(calls[callNo].remote_name, num, IAXC_EVENT_BUFSIZ);

Modified: trunk/lib/video.c
===================================================================
--- trunk/lib/video.c	2008-04-24 13:24:06 UTC (rev 1422)
+++ trunk/lib/video.c	2008-04-24 14:18:28 UTC (rev 1423)
@@ -668,18 +668,26 @@
 
 	call = &calls[selected_call];
 
-	if ( !call || !(call->state & (IAXC_CALL_STATE_COMPLETE |
-					IAXC_CALL_STATE_OUTGOING)) )
+	if ( call->vformat &&
+			( call->state & IAXC_CALL_STATE_COMPLETE ||
+			  call->state & IAXC_CALL_STATE_OUTGOING ) )
 	{
-		goto callback_done;
+		/* For incoming calls, we must ANSWER (complete) the
+		 * call before sending video. For outgoing calls, the
+		 * call must only be ACCEPTed (vformat set) before we
+		 * start sending video. 
+		 */
 	}
-
-	if ( call->vformat == 0 )
+	else if ( !call->vformat && call->state & IAXC_CALL_STATE_COMPLETE )
 	{
 		fprintf(stderr, "video format not set for call %d\n",
 				selected_call);
 		goto callback_failed;
 	}
+	else
+	{
+		goto callback_done;
+	}
 
 	if ( !need_encode )
 	{


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.