[svn-commits] rmudgett: branch group/media_formats-reviewed-trunk r418709 - in /team/group/...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Jul 15 14:32:03 CDT 2014


Author: rmudgett
Date: Tue Jul 15 14:31:58 2014
New Revision: 418709

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=418709
Log:
media_format: Make ast_format_cache_get_slin_by_rate() not return a ref bumped format.

There are a few places where the ref originally returned by
ast_format_cache_get_slin_by_rate() is not really needed.  The
ast_format_cache_get_slin_by_rate() is really a convenience function to
pick the best global ast_format_slinxxx value which is accessed directly
throughout the code.

* Simplified many uses of ast_format_cache_get_slin_by_rate() to not have
to unref the format.

* Added some BUGBUG comments about the translation code trashing the
normal static frame setup when a special frame is needed in codec_dahdi.c
and codec_speex.c.

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

Modified:
    team/group/media_formats-reviewed-trunk/apps/app_jack.c
    team/group/media_formats-reviewed-trunk/apps/app_mixmonitor.c
    team/group/media_formats-reviewed-trunk/bridges/bridge_softmix.c
    team/group/media_formats-reviewed-trunk/codecs/codec_dahdi.c
    team/group/media_formats-reviewed-trunk/codecs/codec_resample.c
    team/group/media_formats-reviewed-trunk/codecs/codec_speex.c
    team/group/media_formats-reviewed-trunk/include/asterisk/format_cache.h
    team/group/media_formats-reviewed-trunk/main/audiohook.c
    team/group/media_formats-reviewed-trunk/main/channel.c
    team/group/media_formats-reviewed-trunk/main/format_cache.c
    team/group/media_formats-reviewed-trunk/main/slinfactory.c
    team/group/media_formats-reviewed-trunk/res/res_stasis_snoop.c

Modified: team/group/media_formats-reviewed-trunk/apps/app_jack.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/apps/app_jack.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/apps/app_jack.c (original)
+++ team/group/media_formats-reviewed-trunk/apps/app_jack.c Tue Jul 15 14:31:58 2014
@@ -382,7 +382,6 @@
 	if (jack_data->has_audiohook)
 		ast_audiohook_destroy(&jack_data->audiohook);
 
-	ao2_cleanup(jack_data->audiohook_format);
 	ast_string_field_free_memory(jack_data);
 
 	ast_free(jack_data);
@@ -860,7 +859,7 @@
 
 	if (ast_format_cmp(frame->subclass.format, jack_data->audiohook_format) == AST_FORMAT_CMP_NOT_EQUAL) {
 		ast_log(LOG_WARNING, "Expected frame in %s for the audiohook, but got format %s\n",
-		    ast_format_get_name(jack_data->audiohook_format),
+			ast_format_get_name(jack_data->audiohook_format),
 			ast_format_get_name(frame->subclass.format));
 		ast_channel_unlock(chan);
 		return 0;

Modified: team/group/media_formats-reviewed-trunk/apps/app_mixmonitor.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/apps/app_mixmonitor.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/apps/app_mixmonitor.c (original)
+++ team/group/media_formats-reviewed-trunk/apps/app_mixmonitor.c Tue Jul 15 14:31:58 2014
@@ -746,8 +746,6 @@
 	}
 	ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
 
-	ao2_ref(format_slin, -1);
-
 	/* kill the audiohook */
 	destroy_monitor_audiohook(mixmonitor);
 

Modified: team/group/media_formats-reviewed-trunk/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/bridges/bridge_softmix.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/bridges/bridge_softmix.c (original)
+++ team/group/media_formats-reviewed-trunk/bridges/bridge_softmix.c Tue Jul 15 14:31:58 2014
@@ -208,8 +208,6 @@
 {
 	struct softmix_translate_helper_entry *entry;
 
-	ao2_cleanup(trans_helper->slin_src);
-
 	while ((entry = AST_LIST_REMOVE_HEAD(&trans_helper->entries, entry))) {
 		softmix_translate_helper_free_entry(entry);
 	}
@@ -219,7 +217,6 @@
 {
 	struct softmix_translate_helper_entry *entry;
 
-	ao2_cleanup(trans_helper->slin_src);
 	trans_helper->slin_src = ast_format_cache_get_slin_by_rate(sample_rate);
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&trans_helper->entries, entry, entry) {
 		if (entry->trans_pvt) {
@@ -328,16 +325,27 @@
 		ast_slinfactory_destroy(&sc->factory);
 		ast_dsp_free(sc->dsp);
 	}
-	/* Setup read/write frame parameters */
+
+	/* Setup write frame parameters */
 	sc->write_frame.frametype = AST_FRAME_VOICE;
 	ao2_cleanup(sc->write_frame.subclass.format);
-	sc->write_frame.subclass.format = ast_format_cache_get_slin_by_rate(rate);
+	/*
+	 * NOTE: The format is bumped here because translation could
+	 * be needed and the format changed to the translated format
+	 * for the channel.  The translated format may not be a
+	 * static cached format.
+	 */
+	sc->write_frame.subclass.format = ao2_bump(ast_format_cache_get_slin_by_rate(rate));
 	sc->write_frame.data.ptr = sc->final_buf;
 	sc->write_frame.datalen = SOFTMIX_DATALEN(rate, interval);
 	sc->write_frame.samples = SOFTMIX_SAMPLES(rate, interval);
 
+	/* Setup read frame parameters */
 	sc->read_frame.frametype = AST_FRAME_VOICE;
-	ao2_cleanup(sc->read_frame.subclass.format);
+	/*
+	 * NOTE: The format is not bumbed here because it will always
+	 * be a signed linear format.
+	 */
 	sc->read_frame.subclass.format = ast_format_cache_get_slin_by_rate(channel_read_rate);
 	sc->read_frame.data.ptr = sc->our_buf;
 	sc->read_frame.datalen = SOFTMIX_DATALEN(channel_read_rate, interval);
@@ -455,7 +463,6 @@
 
 	/* Drop any formats on the frames */
 	ao2_cleanup(sc->write_frame.subclass.format);
-	ao2_cleanup(sc->read_frame.subclass.format);
 
 	/* Drop the DSP */
 	ast_dsp_free(sc->dsp);
@@ -867,14 +874,12 @@
 			ast_log(LOG_WARNING,
 				"Bridge %s: Conference mixing error, requested mixing length greater than mixing buffer.\n",
 				bridge->uniqueid);
-			ao2_ref(cur_slin, -1);
 			goto softmix_cleanup;
 		}
 
 		/* Grow the mixing array buffer as participants are added. */
 		if (mixing_array.max_num_entries < bridge->num_channels
 			&& softmix_mixing_array_grow(&mixing_array, bridge->num_channels + 5)) {
-			ao2_ref(cur_slin, -1);
 			goto softmix_cleanup;
 		}
 
@@ -955,8 +960,6 @@
 			/* A frame is now ready for the channel. */
 			ast_bridge_channel_queue_frame(bridge_channel, &sc->write_frame);
 		}
-
-		ao2_ref(cur_slin, -1);
 
 		update_all_rates = 0;
 		if (!stat_iteration_counter) {

Modified: team/group/media_formats-reviewed-trunk/codecs/codec_dahdi.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/codecs/codec_dahdi.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/codecs/codec_dahdi.c (original)
+++ team/group/media_formats-reviewed-trunk/codecs/codec_dahdi.c Tue Jul 15 14:31:58 2014
@@ -79,6 +79,11 @@
 	int decoders;
 } channels;
 
+/*
+ * Good grief, this array is large and sparse! array[1 << 10], something like 80k
+ *
+ * BUGBUG Type mismatch: The users of the array only give a 0-31 index value not a (1 << (0-31)) index value.
+ */
 static struct ast_codec codecs[] = {
 	[DAHDI_FORMAT_G723_1] = {
 		.name = "g723",
@@ -305,6 +310,7 @@
 
 	if (2 == dahdip->fake) {
 		dahdip->fake = 1;
+/* BUGBUG need to setup a new static frame to prevent destroying the translators normal static frame. */
 		pvt->f.frametype = AST_FRAME_VOICE;
 		ao2_cleanup(pvt->f.subclass.format);
 		pvt->f.subclass.format = NULL;
@@ -382,6 +388,7 @@
 
 	if (2 == dahdip->fake) {
 		dahdip->fake = 1;
+/* BUGBUG need to setup a new static frame to prevent destroying the translators normal static frame. */
 		pvt->f.frametype = AST_FRAME_VOICE;
 		ao2_cleanup(pvt->f.subclass.format);
 		pvt->f.subclass.format = NULL;
@@ -484,6 +491,7 @@
 	}
 
 	/* This will never be reached */
+	ast_assert(0);
 	return NULL;
 }
 
@@ -504,12 +512,8 @@
 	dahdip->fmts.srcfmt = ast_format_compatibility_codec2bitfield(src_codec);
 	dahdip->fmts.dstfmt = ast_format_compatibility_codec2bitfield(dst_codec);
 
-	pvt->f.frametype = AST_FRAME_VOICE;
-	pvt->f.subclass.format = dahdi_format_to_cached(dahdip->fmts.dstfmt);
-	pvt->f.mallocd = 0;
-	pvt->f.offset = AST_FRIENDLY_OFFSET;
-	pvt->f.src = pvt->t->name;
-	pvt->f.data.ptr = pvt->outbuf.c;
+	ast_assert(pvt->f.subclass.format == NULL);
+	pvt->f.subclass.format = ao2_bump(dahdi_format_to_cached(dahdip->fmts.dstfmt));
 
 	ast_debug(1, "Opening transcoder channel from %s to %s.\n", src_codec->name, dst_codec->name);
 

Modified: team/group/media_formats-reviewed-trunk/codecs/codec_resample.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/codecs/codec_resample.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/codecs/codec_resample.c (original)
+++ team/group/media_formats-reviewed-trunk/codecs/codec_resample.c Tue Jul 15 14:31:58 2014
@@ -98,7 +98,8 @@
 		return -1;
 	}
 
-	pvt->f.subclass.format = ast_format_cache_get_slin_by_rate(pvt->t->dst_codec.sample_rate);
+	ast_assert(pvt->f.subclass.format == NULL);
+	pvt->f.subclass.format = ao2_bump(ast_format_cache_get_slin_by_rate(pvt->t->dst_codec.sample_rate));
 
 	return 0;
 }

Modified: team/group/media_formats-reviewed-trunk/codecs/codec_speex.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/codecs/codec_speex.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/codecs/codec_speex.c (original)
+++ team/group/media_formats-reviewed-trunk/codecs/codec_speex.c Tue Jul 15 14:31:58 2014
@@ -308,10 +308,14 @@
 		} else {
 			tmp->silent_state = 1;
 			speex_bits_reset(&tmp->bits);
+
+/* BUGBUG need to setup a new static frame to prevent destroying the translators normal static frame. */
+			ao2_cleanup(pvt->f.subclass.format);
 			memset(&pvt->f, 0, sizeof(pvt->f));
 			pvt->f.frametype = AST_FRAME_CNG;
 			pvt->f.samples = samples;
 			/* XXX what now ? format etc... */
+/* BUGBUG should return ast_frisolate(setup local static frame) here */
 		}
 	}
 

Modified: team/group/media_formats-reviewed-trunk/include/asterisk/format_cache.h
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/include/asterisk/format_cache.h?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/include/asterisk/format_cache.h (original)
+++ team/group/media_formats-reviewed-trunk/include/asterisk/format_cache.h Tue Jul 15 14:31:58 2014
@@ -278,10 +278,13 @@
  *
  * \param rate The sample rate
  *
+ * \details
+ * This is a convenience function that returns one of the global
+ * ast_format_slinxxx formats.
+ *
  * \return pointer to the signed linear format
  *
- * \note The returned format has its reference count incremented. It must be
- * dropped using ao2_ref or ao2_cleanup.
+ * \note The returned format has NOT had its reference count incremented.
  */
 struct ast_format *ast_format_cache_get_slin_by_rate(unsigned int rate);
 

Modified: team/group/media_formats-reviewed-trunk/main/audiohook.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/audiohook.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/audiohook.c (original)
+++ team/group/media_formats-reviewed-trunk/main/audiohook.c Tue Jul 15 14:31:58 2014
@@ -92,8 +92,6 @@
 	default:
 		break;
 	}
-
-	ao2_ref(slin, -1);
 
 	return 0;
 }
@@ -232,17 +230,14 @@
 		.datalen = sizeof(buf),
 		.samples = samples,
 	};
-	struct ast_frame *out;
 
 	/* Ensure the factory is able to give us the samples we want */
 	if (samples > ast_slinfactory_available(factory)) {
-		ao2_ref(frame.subclass.format, -1);
 		return NULL;
 	}
 
 	/* Read data in from factory */
 	if (!ast_slinfactory_read(factory, buf, samples)) {
-		ao2_ref(frame.subclass.format, -1);
 		return NULL;
 	}
 
@@ -251,10 +246,7 @@
 		ast_frame_adjust_volume(&frame, vol);
 	}
 
-	out = ast_frdup(&frame);
-	ao2_ref(frame.subclass.format, -1);
-
-	return out;
+	return ast_frdup(&frame);
 }
 
 static struct ast_frame *audiohook_read_frame_both(struct ast_audiohook *audiohook, size_t samples, struct ast_frame **read_reference, struct ast_frame **write_reference)
@@ -267,7 +259,6 @@
 		.datalen = sizeof(buf1),
 		.samples = samples,
 	};
-	struct ast_frame *out;
 
 	/* Make sure both factories have the required samples */
 	usable_read = (ast_slinfactory_available(&audiohook->read_factory) >= samples ? 1 : 0);
@@ -362,10 +353,7 @@
 	frame.subclass.format = ast_format_cache_get_slin_by_rate(audiohook->hook_internal_samp_rate);
 
 	/* Yahoo, a combined copy of the audio! */
-	out = ast_frdup(&frame);
-	ao2_ref(frame.subclass.format, -1);
-
-	return out;
+	return ast_frdup(&frame);
 }
 
 static struct ast_frame *audiohook_read_frame_helper(struct ast_audiohook *audiohook, size_t samples, enum ast_audiohook_direction direction, struct ast_format *format, struct ast_frame **read_reference, struct ast_frame **write_reference)
@@ -404,7 +392,6 @@
 			/* Setup new translation path for this format... if we fail we can't very well return signed linear so free the frame and return nothing */
 			if (!(audiohook->trans_pvt = ast_translator_build_path(format, slin))) {
 				ast_frfree(read_frame);
-				ao2_ref(slin, -1);
 				return NULL;
 			}
 			ao2_replace(audiohook->format, format);
@@ -414,8 +401,6 @@
 	} else {
 		final_frame = read_frame;
 	}
-
-	ao2_ref(slin, -1);
 
 	return final_frame;
 }
@@ -751,7 +736,6 @@
 
 	slin = ast_format_cache_get_slin_by_rate(audiohook_list->list_internal_samp_rate);
 	if (ast_format_cmp(frame->subclass.format, slin) == AST_FORMAT_CMP_EQUAL) {
-		ao2_ref(slin, -1);
 		return new_frame;
 	}
 
@@ -760,13 +744,10 @@
 			ast_translator_free_path(in_translate->trans_pvt);
 		}
 		if (!(in_translate->trans_pvt = ast_translator_build_path(slin, frame->subclass.format))) {
-			ao2_ref(slin, -1);
 			return NULL;
 		}
 		ao2_replace(in_translate->format, frame->subclass.format);
 	}
-
-	ao2_ref(slin, -1);
 
 	if (!(new_frame = ast_translate(in_translate->trans_pvt, frame, 0))) {
 		return NULL;

Modified: team/group/media_formats-reviewed-trunk/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/channel.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/channel.c (original)
+++ team/group/media_formats-reviewed-trunk/main/channel.c Tue Jul 15 14:31:58 2014
@@ -6135,7 +6135,7 @@
 
 			/* pick the best signed linear format based upon what preserves the sample rate the best. */
 			ao2_ref(best_src_fmt, -1);
-			best_src_fmt = ast_format_cache_get_slin_by_rate(best_sample_rate);
+			best_src_fmt = ao2_bump(ast_format_cache_get_slin_by_rate(best_sample_rate));
 		}
 	}
 

Modified: team/group/media_formats-reviewed-trunk/main/format_cache.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/format_cache.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/format_cache.c (original)
+++ team/group/media_formats-reviewed-trunk/main/format_cache.c Tue Jul 15 14:31:58 2014
@@ -483,23 +483,23 @@
 struct ast_format *ast_format_cache_get_slin_by_rate(unsigned int rate)
 {
 	if (rate >= 192000) {
-		return ao2_bump(ast_format_slin192);
+		return ast_format_slin192;
 	} else if (rate >= 96000) {
-		return ao2_bump(ast_format_slin96);
+		return ast_format_slin96;
 	} else if (rate >= 48000) {
-		return ao2_bump(ast_format_slin48);
+		return ast_format_slin48;
 	} else if (rate >= 44100) {
-		return ao2_bump(ast_format_slin44);
+		return ast_format_slin44;
 	} else if (rate >= 32000) {
-		return ao2_bump(ast_format_slin32);
+		return ast_format_slin32;
 	} else if (rate >= 24000) {
-		return ao2_bump(ast_format_slin24);
+		return ast_format_slin24;
 	} else if (rate >= 16000) {
-		return ao2_bump(ast_format_slin16);
+		return ast_format_slin16;
 	} else if (rate >= 12000) {
-		return ao2_bump(ast_format_slin12);
-	}
-	return ao2_bump(ast_format_slin);
+		return ast_format_slin12;
+	}
+	return ast_format_slin;
 }
 
 int ast_format_cache_is_slinear(struct ast_format *format)

Modified: team/group/media_formats-reviewed-trunk/main/slinfactory.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/slinfactory.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/slinfactory.c (original)
+++ team/group/media_formats-reviewed-trunk/main/slinfactory.c Tue Jul 15 14:31:58 2014
@@ -71,7 +71,9 @@
 	}
 
 	ao2_cleanup(sf->output_format);
+	sf->output_format = NULL;
 	ao2_cleanup(sf->format);
+	sf->format = NULL;
 }
 
 int ast_slinfactory_feed(struct ast_slinfactory *sf, struct ast_frame *f)

Modified: team/group/media_formats-reviewed-trunk/res/res_stasis_snoop.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/res/res_stasis_snoop.c?view=diff&rev=418709&r1=418708&r2=418709
==============================================================================
--- team/group/media_formats-reviewed-trunk/res/res_stasis_snoop.c (original)
+++ team/group/media_formats-reviewed-trunk/res/res_stasis_snoop.c Tue Jul 15 14:31:58 2014
@@ -95,8 +95,6 @@
 
 	ast_free(snoop->app);
 
-	ao2_cleanup(snoop->spy_format);
-
 	ast_channel_cleanup(snoop->chan);
 }
 




More information about the svn-commits mailing list