[asterisk-commits] rmudgett: branch 1.8 r411715 - in /branches/1.8: ./ channels/ configs/ includ...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Apr 4 13:32:57 CDT 2014


Author: rmudgett
Date: Fri Apr  4 13:32:46 2014
New Revision: 411715

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=411715
Log:
internal_timing: Remove the option and always make it enabled if a timing module is loaded.

The masquerade supertest frequently fails because either the local channel
chain doesn't completely optimize out or the DTMF handshake doesn't
completely get accross.  Local channel optimization requires frames
flowing to trigger when optimization can happen.  When optimization
happens the media frame that triggered the optimization is dropped.
Sending DTMF requires frames to flow in the other direction for timing
purposes while sending nothing.  If internal timing is not enabled when
MOH is playing, Asterisk switches to received timing when an audio frame
is received.  With optimization dropping media frames and MOH not sending
frames unless it receives frames, occasionaly there are no more frames
being passed and the test fails.

* The asterisk command line -I option and the asterisk.conf
internal_timing option are removed.  Asterisk now always uses internal
timing when needed if any timing module is loaded.  The issue
ASTERISK-14861 did this quite awhile ago in v1.4 but effectively is broken
if other internal timing modules besides DAHDI are used.  The
ast_read_generator_actions() now only does received timing if it has no
choice for frame generators like MOH, silence, and playback streaming.

* Cleaned up some code dealing with frame generators in
ast_deactivate_generator(), generator_write_format_change(),
ast_activate_generator(), and ast_channel_stop_silence_generator().

ASTERISK-22846 #close
Reported by: Matt Jordan

Review: https://reviewboard.asterisk.org/r/3414/

Modified:
    branches/1.8/UPGRADE.txt
    branches/1.8/channels/chan_sip.c
    branches/1.8/configs/asterisk.conf.sample
    branches/1.8/include/asterisk/options.h
    branches/1.8/main/asterisk.c
    branches/1.8/main/channel.c

Modified: branches/1.8/UPGRADE.txt
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/UPGRADE.txt?view=diff&rev=411715&r1=411714&r2=411715
==============================================================================
--- branches/1.8/UPGRADE.txt (original)
+++ branches/1.8/UPGRADE.txt Fri Apr  4 13:32:46 2014
@@ -18,6 +18,10 @@
 ===
 ===========================================================
 
+from 1.8.27.0 to 1.8.28.0:
+* The asterisk command line -I option and the asterisk.conf internal_timing
+  option are removed and always enabled if any timing module is loaded.
+
 from 1.8.26.0 to 1.8.27.0:
 * res_fax now returns the correct rates for V.27ter (4800 or 9600 bit/s).
   Because of this the default settings would not load, so the minrate (minimum

Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=411715&r1=411714&r2=411715
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Fri Apr  4 13:32:46 2014
@@ -12016,8 +12016,9 @@
 
 		ast_debug(3, "-- Done with adding codecs to SDP\n");
 
-		if (!p->owner || !ast_internal_timing_enabled(p->owner))
+		if (!p->owner || p->owner->timingfd == -1) {
 			ast_str_append(&a_audio, 0, "a=silenceSupp:off - - - -\r\n");
+		}
 
 		if (min_audio_packet_size)
 			ast_str_append(&a_audio, 0, "a=ptime:%d\r\n", min_audio_packet_size);

Modified: branches/1.8/configs/asterisk.conf.sample
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/configs/asterisk.conf.sample?view=diff&rev=411715&r1=411714&r2=411715
==============================================================================
--- branches/1.8/configs/asterisk.conf.sample (original)
+++ branches/1.8/configs/asterisk.conf.sample Fri Apr  4 13:32:46 2014
@@ -27,7 +27,6 @@
 ;dontwarn = yes			; Disable some warnings.
 ;dumpcore = yes			; Dump core on crash (same as -g at startup).
 ;languageprefix = yes		; Use the new sound prefix path syntax.
-;internal_timing = yes
 ;systemname = my_system_name	; Prefix uniqueid with a system name for
 				; Global uniqueness issues.
 ;autosystemname = yes		; Automatically set systemname to hostname,

Modified: branches/1.8/include/asterisk/options.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/options.h?view=diff&rev=411715&r1=411714&r2=411715
==============================================================================
--- branches/1.8/include/asterisk/options.h (original)
+++ branches/1.8/include/asterisk/options.h Fri Apr  4 13:32:46 2014
@@ -74,7 +74,7 @@
 	AST_OPT_FLAG_DONT_WARN = (1 << 18),
 	/*! End CDRs before the 'h' extension */
 	AST_OPT_FLAG_END_CDR_BEFORE_H_EXTEN = (1 << 19),
-	/*! Use DAHDI Timing for generators if available */
+	/*! Use DAHDI Timing for generators if available (No longer used) */
 	AST_OPT_FLAG_INTERNAL_TIMING = (1 << 20),
 	/*! Always fork, even if verbose or debug settings are non-zero */
 	AST_OPT_FLAG_ALWAYS_FORK = (1 << 21),
@@ -99,11 +99,7 @@
 };
 
 /*! These are the options that set by default when Asterisk starts */
-#if (defined(HAVE_DAHDI_VERSION) && HAVE_DAHDI_VERSION >= 230)
-#define AST_DEFAULT_OPTIONS AST_OPT_FLAG_TRANSCODE_VIA_SLIN | AST_OPT_FLAG_INTERNAL_TIMING
-#else
 #define AST_DEFAULT_OPTIONS AST_OPT_FLAG_TRANSCODE_VIA_SLIN
-#endif
 
 #define ast_opt_exec_includes		ast_test_flag(&ast_options, AST_OPT_FLAG_EXEC_INCLUDES)
 #define ast_opt_no_fork			ast_test_flag(&ast_options, AST_OPT_FLAG_NO_FORK)

Modified: branches/1.8/main/asterisk.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/asterisk.c?view=diff&rev=411715&r1=411714&r2=411715
==============================================================================
--- branches/1.8/main/asterisk.c (original)
+++ branches/1.8/main/asterisk.c Fri Apr  4 13:32:46 2014
@@ -484,7 +484,6 @@
 	ast_cli(a->fd, "  User name and group:         %s/%s\n", ast_config_AST_RUN_USER, ast_config_AST_RUN_GROUP);
 	ast_cli(a->fd, "  Executable includes:         %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_EXEC_INCLUDES) ? "Enabled" : "Disabled");
 	ast_cli(a->fd, "  Transcode via SLIN:          %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_TRANSCODE_VIA_SLIN) ? "Enabled" : "Disabled");
-	ast_cli(a->fd, "  Internal timing:             %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_INTERNAL_TIMING) ? "Enabled" : "Disabled");
 	ast_cli(a->fd, "  Transmit silence during rec: %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_TRANSMIT_SILENCE) ? "Enabled" : "Disabled");
 	ast_cli(a->fd, "  Generic PLC:                 %s\n", ast_test_flag(&ast_options, AST_OPT_FLAG_GENERIC_PLC) ? "Enabled" : "Disabled");
 
@@ -3003,7 +3002,6 @@
 	printf("   -g              Dump core in case of a crash\n");
 	printf("   -h              This help screen\n");
 	printf("   -i              Initialize crypto keys at startup\n");
-	printf("   -I              Enable internal timing if DAHDI timer is available\n");
 	printf("   -L <load>       Limit the maximum load average before rejecting new calls\n");
 	printf("   -M <value>      Limit the maximum number of calls to the specified value\n");
 	printf("   -m              Mute debugging and console output on the console\n");
@@ -3171,9 +3169,6 @@
 		/* Transmit SLINEAR silence while a channel is being recorded or DTMF is being generated on a channel */
 		} else if (!strcasecmp(v->name, "transmit_silence_during_record") || !strcasecmp(v->name, "transmit_silence")) {
 			ast_set2_flag(&ast_options, ast_true(v->value), AST_OPT_FLAG_TRANSMIT_SILENCE);
-		/* Enable internal timing */
-		} else if (!strcasecmp(v->name, "internal_timing")) {
-			ast_set2_flag(&ast_options, ast_true(v->value), AST_OPT_FLAG_INTERNAL_TIMING);
 		} else if (!strcasecmp(v->name, "maxcalls")) {
 			if ((sscanf(v->value, "%30d", &option_maxcalls) != 1) || (option_maxcalls < 0)) {
 				option_maxcalls = 0;
@@ -3482,9 +3477,6 @@
 		case 'h':
 			show_cli_help();
 			exit(0);
-		case 'I':
-			ast_set_flag(&ast_options, AST_OPT_FLAG_INTERNAL_TIMING);
-			break;
 		case 'i':
 			ast_set_flag(&ast_options, AST_OPT_FLAG_INIT_KEYS);
 			break;

Modified: branches/1.8/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/main/channel.c?view=diff&rev=411715&r1=411714&r2=411715
==============================================================================
--- branches/1.8/main/channel.c (original)
+++ branches/1.8/main/channel.c Fri Apr  4 13:32:46 2014
@@ -3088,18 +3088,26 @@
 	return __ast_answer(chan, 0, 1);
 }
 
-void ast_deactivate_generator(struct ast_channel *chan)
-{
-	ast_channel_lock(chan);
+static void deactivate_generator_nolock(struct ast_channel *chan)
+{
 	if (chan->generatordata) {
-		if (chan->generator && chan->generator->release)
-			chan->generator->release(chan, chan->generatordata);
+		struct ast_generator *generator = chan->generator;
+
+		if (generator && generator->release) {
+			generator->release(chan, chan->generatordata);
+		}
 		chan->generatordata = NULL;
 		chan->generator = NULL;
 		ast_channel_set_fd(chan, AST_GENERATOR_FD, -1);
 		ast_clear_flag(chan, AST_FLAG_WRITE_INT);
 		ast_settimeout(chan, 0, NULL, NULL);
 	}
+}
+
+void ast_deactivate_generator(struct ast_channel *chan)
+{
+	ast_channel_lock(chan);
+	deactivate_generator_nolock(chan);
 	ast_channel_unlock(chan);
 }
 
@@ -3143,8 +3151,11 @@
 
 	ast_channel_lock(chan);
 	if (chan->generatordata) {
-		if (chan->generator && chan->generator->release)
-			chan->generator->release(chan, chan->generatordata);
+		struct ast_generator *generator_old = chan->generator;
+
+		if (generator_old && generator_old->release) {
+			generator_old->release(chan, chan->generatordata);
+		}
 		chan->generatordata = NULL;
 	}
 	if (gen->alloc && !(chan->generatordata = gen->alloc(chan, params))) {
@@ -3710,48 +3721,56 @@
 
 static void ast_read_generator_actions(struct ast_channel *chan, struct ast_frame *f)
 {
-	if (chan->generator && chan->generator->generate && chan->generatordata &&  !ast_internal_timing_enabled(chan)) {
-		void *tmp = chan->generatordata;
-		int (*generate)(struct ast_channel *chan, void *tmp, int datalen, int samples) = chan->generator->generate;
-		int res;
-		int samples;
-
-		if (chan->timingfunc) {
-			ast_debug(1, "Generator got voice, switching to phase locked mode\n");
-			ast_settimeout(chan, 0, NULL, NULL);
-		}
-
-		chan->generatordata = NULL;     /* reset, to let writes go through */
-
-		if (f->subclass.codec != chan->writeformat) {
-			float factor;
-			factor = ((float) ast_format_rate(chan->writeformat)) / ((float) ast_format_rate(f->subclass.codec));
-			samples = (int) ( ((float) f->samples) * factor );
-		} else {
-			samples = f->samples;
-		}
-		
-		/* This unlock is here based on two assumptions that hold true at this point in the
-		 * code. 1) this function is only called from within __ast_read() and 2) all generators
-		 * call ast_write() in their generate callback.
-		 *
-		 * The reason this is added is so that when ast_write is called, the lock that occurs 
-		 * there will not recursively lock the channel. Doing this will cause intended deadlock 
-		 * avoidance not to work in deeper functions
-		 */
-		ast_channel_unlock(chan);
-		res = generate(chan, tmp, f->datalen, samples);
-		ast_channel_lock(chan);
-		chan->generatordata = tmp;
+	struct ast_generator *generator;
+	void *gendata;
+	int res;
+	int samples;
+
+	generator = chan->generator;
+	if (!generator
+		|| !generator->generate
+		|| f->frametype != AST_FRAME_VOICE
+		|| !chan->generatordata
+		|| chan->timingfunc) {
+		return;
+	}
+
+	/*
+	 * We must generate frames in phase locked mode since
+	 * we have no internal timer available.
+	 */
+
+	if (f->subclass.codec != chan->writeformat) {
+		float factor;
+
+		factor = ((float) ast_format_rate(chan->writeformat)) / ((float) ast_format_rate(f->subclass.codec));
+		samples = (int) (((float) f->samples) * factor);
+	} else {
+		samples = f->samples;
+	}
+
+	gendata = chan->generatordata;
+	chan->generatordata = NULL;     /* reset, to let writes go through */
+
+	/*
+	 * This unlock is here based on two assumptions that hold true at
+	 * this point in the code. 1) this function is only called from
+	 * within __ast_read() and 2) all generators call ast_write() in
+	 * their generate callback.
+	 *
+	 * The reason this is added is so that when ast_write is called,
+	 * the lock that occurs there will not recursively lock the
+	 * channel.  Doing this will allow deadlock avoidance to work in
+	 * deeper functions.
+	 */
+	ast_channel_unlock(chan);
+	res = generator->generate(chan, gendata, f->datalen, samples);
+	ast_channel_lock(chan);
+	if (generator == chan->generator) {
+		chan->generatordata = gendata;
 		if (res) {
 			ast_debug(1, "Auto-deactivating generator\n");
 			ast_deactivate_generator(chan);
-		}
-
-	} else if (f->frametype == AST_FRAME_CNG) {
-		if (chan->generator && !chan->timingfunc && (chan->timingfd > -1)) {
-			ast_debug(1, "Generator got CNG, switching to timed mode\n");
-			ast_settimeout(chan, 50, generator_force, chan);
 		}
 	}
 }
@@ -4358,7 +4377,7 @@
 
 int ast_internal_timing_enabled(struct ast_channel *chan)
 {
-	return (ast_opt_internal_timing && chan->timingfd > -1);
+	return chan->timingfd > -1;
 }
 
 struct ast_frame *ast_read(struct ast_channel *chan)
@@ -8300,30 +8319,24 @@
 	return state;
 }
 
-static int internal_deactivate_generator(struct ast_channel *chan, void* generator)
+static int deactivate_silence_generator(struct ast_channel *chan)
 {
 	ast_channel_lock(chan);
 
 	if (!chan->generatordata) {
-		ast_debug(1, "Trying to stop silence generator when there is no "
-		    "generator on '%s'\n", chan->name);
+		ast_debug(1, "Trying to stop silence generator when there is no generator on '%s'\n",
+			chan->name);
 		ast_channel_unlock(chan);
 		return 0;
 	}
-	if (chan->generator != generator) {
-		ast_debug(1, "Trying to stop silence generator when it is not the current "
-		    "generator on '%s'\n", chan->name);
+	if (chan->generator != &silence_generator) {
+		ast_debug(1, "Trying to stop silence generator when it is not the current generator on '%s'\n",
+			chan->name);
 		ast_channel_unlock(chan);
 		return 0;
 	}
-	if (chan->generator && chan->generator->release) {
-		chan->generator->release(chan, chan->generatordata);
-	}
-	chan->generatordata = NULL;
-	chan->generator = NULL;
-	ast_channel_set_fd(chan, AST_GENERATOR_FD, -1);
-	ast_clear_flag(chan, AST_FLAG_WRITE_INT);
-	ast_settimeout(chan, 0, NULL, NULL);
+	deactivate_generator_nolock(chan);
+
 	ast_channel_unlock(chan);
 
 	return 1;
@@ -8331,10 +8344,11 @@
 
 void ast_channel_stop_silence_generator(struct ast_channel *chan, struct ast_silence_generator *state)
 {
-	if (!state)
+	if (!state) {
 		return;
-
-	if (internal_deactivate_generator(chan, &silence_generator)) {
+	}
+
+	if (deactivate_silence_generator(chan)) {
 		ast_debug(1, "Stopped silence generator on '%s'\n", chan->name);
 
 		if (ast_set_write_format(chan, state->old_write_format) < 0)




More information about the asterisk-commits mailing list