[Asterisk-code-review] streams: Add string metadata capability (asterisk[master])

George Joseph asteriskteam at digium.com
Tue Apr 17 12:04:52 CDT 2018


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/8796


Change subject: streams: Add string metadata capability
......................................................................

streams: Add string metadata capability

Replaces the never used opaque data array.

Updated stream tests to include get/set metadata and
stream clone with metadata.

Added stream metadata dump to "core show channel"

Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863
---
M include/asterisk/stream.h
M main/cli.c
M main/sdp.c
M main/sdp_state.c
M main/stream.c
M tests/test_stream.c
6 files changed, 293 insertions(+), 61 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/96/8796/1

diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index c2d5a88..0a5550b 100644
--- a/include/asterisk/stream.h
+++ b/include/asterisk/stream.h
@@ -78,20 +78,6 @@
 };
 
 /*!
- * \brief Stream data slots
- */
-enum ast_stream_data_slot {
-	/*!
-	 * \brief Data slot for RTP instance
-	 */
-	AST_STREAM_DATA_RTP_CODECS = 0,
-	/*!
-	 * \brief Controls the size of the data pointer array
-	 */
-	AST_STREAM_DATA_SLOT_MAX
-};
-
-/*!
  * \brief Create a new media stream representation
  *
  * \param name A name for the stream
@@ -239,32 +225,47 @@
 enum ast_stream_state ast_stream_str2state(const char *str);
 
 /*!
- * \brief Get the opaque stream data
+ * \brief Get a stream metadata value
  *
  * \param stream The media stream
- * \param slot The data slot to retrieve
+ * \param m_key  An arbitrary metadata key
  *
- * \retval non-NULL success
- * \retval NULL failure
+ * \retval non-NULL metadata value
+ * \retval NULL failure or not found
  *
- * \since 15
+ * \since 15.5
  */
-void *ast_stream_get_data(struct ast_stream *stream, enum ast_stream_data_slot slot);
+const char *ast_stream_get_metadata(const struct ast_stream *stream,
+	const char *m_key);
 
 /*!
- * \brief Set the opaque stream data
+ * \brief Get all stream metadata keys
  *
  * \param stream The media stream
- * \param slot The data slot to set
- * \param data Opaque data
- * \param data_free_fn Callback to free data when stream is freed. May be NULL for no action.
  *
- * \return data
+ * \retval An ast_variable list of the metadata key/value pairs.
+ * \retval NULL if error or no variables are set.
  *
- * \since 15
+ * When you're finished with the list, you must call
+ *  ast_variables_destroy(list);
+ *
+ * \since 15.5
  */
-void *ast_stream_set_data(struct ast_stream *stream, enum ast_stream_data_slot slot,
-	void *data, ast_stream_data_free_fn data_free_fn);
+struct ast_variable *ast_stream_get_metadata_list(const struct ast_stream *stream);
+
+/*!
+ * \brief Set a stream metadata value
+ *
+ * \param stream The media stream
+ * \param m_key  An arbitrary metadata key
+ * \param value  String metadata value or NULL to remove existing value
+ *
+ * \retval -1 failure
+ * \retval 0  success
+ *
+ * \since 15.5
+ */
+int ast_stream_set_metadata(struct ast_stream *stream, const char *m_key, const char *value);
 
 /*!
  * \brief Get the position of the stream in the topology
@@ -278,6 +279,27 @@
 int ast_stream_get_position(const struct ast_stream *stream);
 
 /*!
+ * \brief Get rtp_codecs associated with the stream
+ *
+ * \param stream The media stream
+ *
+ * \return The rtp_codecs
+ *
+ * \since 15.5
+ */
+struct ast_rtp_codecs *ast_stream_get_rtp_codecs(const struct ast_stream *stream);
+
+/*!
+ * \brief Set rtp_codecs associated with the stream
+ *
+ * \param stream The media stream
+ * \param rtp_codecs The rtp_codecs
+ *
+ * \since 15.5
+ */
+void ast_stream_set_rtp_codecs(struct ast_stream *stream, struct ast_rtp_codecs *rtp_codecs);
+
+/*!
  * \brief Create a stream topology
  *
  * \retval non-NULL success
diff --git a/main/cli.c b/main/cli.c
index 5730be1..cf51d0d 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -1614,19 +1614,29 @@
 	ast_str_append(&output, 0, " -- Streams --\n");
 	for (stream_num = 0; stream_num < ast_stream_topology_get_count(ast_channel_get_stream_topology(chan)); stream_num++) {
 		struct ast_stream *stream = ast_stream_topology_get_stream(ast_channel_get_stream_topology(chan), stream_num);
+		struct ast_variable *metadata = ast_stream_get_metadata_list(stream);
 
 		ast_str_append(&output, 0,
 			"Name: %s\n"
 			"    Type: %s\n"
 			"    State: %s\n"
 			"    Group: %d\n"
-			"    Formats: %s\n",
+			"    Formats: %s\n"
+			"    Metadata:\n",
 			ast_stream_get_name(stream),
 			ast_codec_media_type2str(ast_stream_get_type(stream)),
 			ast_stream_state2str(ast_stream_get_state(stream)),
 			ast_stream_get_group(stream),
 			ast_format_cap_get_names(ast_stream_get_formats(stream), &codec_buf)
 			);
+
+		if (metadata) {
+			struct ast_variable *v;
+			for(v = metadata; v; v = v->next) {
+				ast_str_append(&output, 0, "        %s: %s\n", v->name, v->value);
+			}
+			ast_variables_destroy(metadata);
+		}
 	}
 
 	ast_channel_unlock(chan);
diff --git a/main/sdp.c b/main/sdp.c
index 7e283eb..e5c8803 100644
--- a/main/sdp.c
+++ b/main/sdp.c
@@ -848,8 +848,7 @@
 			ast_free(codecs);
 			return NULL;
 		}
-		ast_stream_set_data(stream, AST_STREAM_DATA_RTP_CODECS, codecs,
-			(ast_stream_data_free_fn) rtp_codecs_free);
+		ast_stream_set_rtp_codecs(stream, codecs);
 
 		if (!m_line->port) {
 			/* Stream is declined.  There may not be any attributes. */
diff --git a/main/sdp_state.c b/main/sdp_state.c
index 5f9ad5e..e022277 100644
--- a/main/sdp_state.c
+++ b/main/sdp_state.c
@@ -1732,7 +1732,7 @@
 	case AST_MEDIA_TYPE_AUDIO:
 	case AST_MEDIA_TYPE_VIDEO:
 		ao2_bump(joint_state_stream->rtp);
-		codecs = ast_stream_get_data(remote_stream, AST_STREAM_DATA_RTP_CODECS);
+		codecs = ast_stream_get_rtp_codecs(remote_stream);
 		ast_assert(codecs != NULL);
 		if (sdp_state->role == SDP_ROLE_ANSWERER) {
 			/*
@@ -1789,7 +1789,7 @@
 		 * Setup rx payload type mapping to prefer the mapping
 		 * from the peer that the RFC says we SHOULD use.
 		 */
-		codecs = ast_stream_get_data(remote_stream, AST_STREAM_DATA_RTP_CODECS);
+		codecs = ast_stream_get_rtp_codecs(remote_stream);
 		ast_assert(codecs != NULL);
 		ast_rtp_codecs_payloads_xover(codecs, codecs, NULL);
 		ast_rtp_codecs_payloads_copy(codecs,
diff --git a/main/stream.c b/main/stream.c
index 61eef25..dd3e765 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -34,6 +34,15 @@
 #include "asterisk/strings.h"
 #include "asterisk/format.h"
 #include "asterisk/format_cap.h"
+#include "asterisk/vector.h"
+#include "asterisk/config.h"
+#include "asterisk/rtp_engine.h"
+
+struct ast_stream_metadata_entry {
+	size_t length;
+	int value_start;
+	char name_value[0];
+};
 
 struct ast_stream {
 	/*!
@@ -57,19 +66,19 @@
 	enum ast_stream_state state;
 
 	/*!
-	 * \brief Opaque stream data
+	 * \brief Stream metadata vector
 	 */
-	void *data[AST_STREAM_DATA_SLOT_MAX];
-
-	/*!
-	 * \brief What to do with data when the stream is freed
-	 */
-	ast_stream_data_free_fn data_free_fn[AST_STREAM_DATA_SLOT_MAX];
+	struct ast_variable *metadata;
 
 	/*!
 	 * \brief The group that the stream is part of
 	 */
 	int group;
+
+	/*!
+	 * \brief The rtp_codecs used by the stream
+	 */
+	struct ast_rtp_codecs *rtp_codecs;
 
 	/*!
 	 * \brief Name for the stream within the context of the channel it is on
@@ -105,7 +114,6 @@
 {
 	struct ast_stream *new_stream;
 	size_t stream_size;
-	int idx;
 	const char *stream_name;
 
 	if (!stream) {
@@ -126,27 +134,23 @@
 		ao2_ref(new_stream->formats, +1);
 	}
 
-	/* We cannot clone the opaque data because we don't know how. */
-	for (idx = 0; idx < AST_STREAM_DATA_SLOT_MAX; ++idx) {
-		new_stream->data[idx] = NULL;
-		new_stream->data_free_fn[idx] = NULL;
-	}
+	new_stream->metadata = ast_stream_get_metadata_list(stream);
+
+	/* rtp_codecs aren't cloned */
 
 	return new_stream;
 }
 
 void ast_stream_free(struct ast_stream *stream)
 {
-	int i;
-
 	if (!stream) {
 		return;
 	}
 
-	for (i = 0; i < AST_STREAM_DATA_SLOT_MAX; i++) {
-		if (stream->data_free_fn[i]) {
-			stream->data_free_fn[i](stream->data[i]);
-		}
+	ast_variables_destroy(stream->metadata);
+
+	if (stream->rtp_codecs) {
+		ast_rtp_codecs_payloads_destroy(stream->rtp_codecs);
 	}
 
 	ao2_cleanup(stream->formats);
@@ -238,22 +242,81 @@
 	return AST_STREAM_STATE_REMOVED;
 }
 
-void *ast_stream_get_data(struct ast_stream *stream, enum ast_stream_data_slot slot)
+const char *ast_stream_get_metadata(const struct ast_stream *stream, const char *m_key)
 {
-	ast_assert(stream != NULL);
+	struct ast_variable *v;
 
-	return stream->data[slot];
+	ast_assert_return(stream != NULL, NULL);
+	ast_assert_return(m_key != NULL, NULL);
+
+	for (v = stream->metadata; v; v = v->next) {
+		if (strcmp(v->name, m_key) == 0) {
+			return v->value;
+		}
+	}
+
+	return NULL;
 }
 
-void *ast_stream_set_data(struct ast_stream *stream, enum ast_stream_data_slot slot,
-	void *data, ast_stream_data_free_fn data_free_fn)
+struct ast_variable *ast_stream_get_metadata_list(const struct ast_stream *stream)
 {
-	ast_assert(stream != NULL);
+	struct ast_variable *v;
+	struct ast_variable *vout = NULL;
 
-	stream->data[slot] = data;
-	stream->data_free_fn[slot] = data_free_fn;
+	ast_assert_return(stream != NULL, NULL);
 
-	return data;
+	for (v = stream->metadata; v; v = v->next) {
+		struct ast_variable *vt = ast_variable_new(v->name, v->value, "");
+
+		if (!vt) {
+			ast_variables_destroy(vout);
+			return NULL;
+		}
+
+		ast_variable_list_append(&vout, vt);
+	}
+
+	return vout;
+}
+
+int ast_stream_set_metadata(struct ast_stream *stream, const char *m_key, const char *value)
+{
+	struct ast_variable *v;
+	struct ast_variable *prev;
+
+	ast_assert_return(stream != NULL, -1);
+	ast_assert_return(m_key != NULL, -1);
+
+	prev = NULL;
+	v = stream->metadata;
+	while(v) {
+		struct ast_variable *next = v->next;
+		if (strcmp(v->name, m_key) == 0) {
+			if (prev) {
+				prev->next = next;
+			} else {
+				stream->metadata = next;
+			}
+			ast_free(v);
+			break;
+		} else {
+			prev = v;
+		}
+		v = next;
+	}
+
+	if (!value) {
+		return 0;
+	}
+
+	v = ast_variable_new(m_key, value, "");
+	if (!v) {
+		return -1;
+	}
+
+	ast_variable_list_append(&stream->metadata, v);
+
+	return 0;
 }
 
 int ast_stream_get_position(const struct ast_stream *stream)
@@ -263,6 +326,24 @@
 	return stream->position;
 }
 
+struct ast_rtp_codecs *ast_stream_get_rtp_codecs(const struct ast_stream *stream)
+{
+	ast_assert(stream != NULL);
+
+	return stream->rtp_codecs;
+}
+
+void ast_stream_set_rtp_codecs(struct ast_stream *stream, struct ast_rtp_codecs *rtp_codecs)
+{
+	ast_assert(stream != NULL);
+
+	if (stream->rtp_codecs) {
+		ast_rtp_codecs_payloads_destroy(rtp_codecs);
+	}
+
+	stream->rtp_codecs = rtp_codecs;
+}
+
 #define TOPOLOGY_INITIAL_STREAM_COUNT 2
 struct ast_stream_topology *ast_stream_topology_alloc(void)
 {
diff --git a/tests/test_stream.c b/tests/test_stream.c
index 8c88704..50f0ccb 100644
--- a/tests/test_stream.c
+++ b/tests/test_stream.c
@@ -38,6 +38,7 @@
 #include "asterisk/format_cap.h"
 #include "asterisk/format_cache.h"
 #include "asterisk/channel.h"
+#include "asterisk/uuid.h"
 
 AST_TEST_DEFINE(stream_create)
 {
@@ -224,6 +225,70 @@
 	return AST_TEST_PASS;
 }
 
+AST_TEST_DEFINE(stream_metadata)
+{
+	RAII_VAR(struct ast_stream *, stream, NULL, ast_stream_free);
+	char track_label[AST_UUID_STR_LEN + 1];
+	const char *stream_track_label;
+	int rc;
+
+	switch (cmd) {
+	case TEST_INIT:
+		info->name = "stream_metadata";
+		info->category = "/main/stream/";
+		info->summary = "stream metadata unit test";
+		info->description =
+			"Test that metadata operations on a stream works";
+		return AST_TEST_NOT_RUN;
+	case TEST_EXECUTE:
+		break;
+	}
+
+	stream = ast_stream_alloc("test", AST_MEDIA_TYPE_AUDIO);
+	if (!stream) {
+		ast_test_status_update(test, "Failed to create media stream given proper arguments\n");
+		return AST_TEST_FAIL;
+	}
+
+	stream_track_label = ast_stream_get_metadata(stream, "AST_STREAM_METADATA_TRACK_LABEL");
+	if (stream_track_label) {
+		ast_test_status_update(test, "New stream HAD a track label\n");
+		return AST_TEST_FAIL;
+	}
+
+	ast_uuid_generate_str(track_label, sizeof(track_label));
+	rc = ast_stream_set_metadata(stream, "AST_STREAM_METADATA_TRACK_LABEL", track_label);
+	if (rc != 0) {
+		ast_test_status_update(test, "Failed to add track label\n");
+		return AST_TEST_FAIL;
+	}
+
+	stream_track_label = ast_stream_get_metadata(stream, "AST_STREAM_METADATA_TRACK_LABEL");
+	if (!stream_track_label) {
+		ast_test_status_update(test, "Changed stream does not have a track label\n");
+		return AST_TEST_FAIL;
+	}
+
+	if (strcmp(stream_track_label, track_label) != 0) {
+		ast_test_status_update(test, "Changed stream did not return same track label\n");
+		return AST_TEST_FAIL;
+	}
+
+	rc = ast_stream_set_metadata(stream, "AST_STREAM_METADATA_TRACK_LABEL", NULL);
+	if (rc != 0) {
+		ast_test_status_update(test, "Failed to remove track label\n");
+		return AST_TEST_FAIL;
+	}
+
+	stream_track_label = ast_stream_get_metadata(stream, "AST_STREAM_METADATA_TRACK_LABEL");
+	if (stream_track_label) {
+		ast_test_status_update(test, "Changed stream still had a track label after we removed it\n");
+		return AST_TEST_FAIL;
+	}
+
+	return AST_TEST_PASS;
+}
+
 AST_TEST_DEFINE(stream_topology_create)
 {
 	RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);
@@ -254,6 +319,11 @@
 	RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);
 	RAII_VAR(struct ast_stream_topology *, cloned, NULL, ast_stream_topology_free);
 	struct ast_stream *audio_stream, *video_stream;
+	char audio_track_label[AST_UUID_STR_LEN + 1];
+	char video_track_label[AST_UUID_STR_LEN + 1];
+	const char *original_track_label;
+	const char *cloned_track_label;
+	int rc;
 
 	switch (cmd) {
 	case TEST_INIT:
@@ -279,6 +349,13 @@
 		return AST_TEST_FAIL;
 	}
 
+	ast_uuid_generate_str(audio_track_label, sizeof(audio_track_label));
+	rc = ast_stream_set_metadata(audio_stream, "AST_STREAM_METADATA_TRACK_LABEL", audio_track_label);
+	if (rc != 0) {
+		ast_test_status_update(test, "Failed to add track label\n");
+		return AST_TEST_FAIL;
+	}
+
 	if (ast_stream_topology_append_stream(topology, audio_stream) == -1) {
 		ast_test_status_update(test, "Failed to append valid audio stream to stream topology\n");
 		ast_stream_free(audio_stream);
@@ -288,6 +365,13 @@
 	video_stream = ast_stream_alloc("video", AST_MEDIA_TYPE_VIDEO);
 	if (!video_stream) {
 		ast_test_status_update(test, "Failed to create a video stream for testing stream topology\n");
+		return AST_TEST_FAIL;
+	}
+
+	ast_uuid_generate_str(video_track_label, sizeof(video_track_label));
+	rc = ast_stream_set_metadata(video_stream, "AST_STREAM_METADATA_TRACK_LABEL", video_track_label);
+	if (rc != 0) {
+		ast_test_status_update(test, "Failed to add track label\n");
 		return AST_TEST_FAIL;
 	}
 
@@ -313,8 +397,42 @@
 		return AST_TEST_FAIL;
 	}
 
+	original_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(topology, 0),
+		"AST_STREAM_METADATA_TRACK_LABEL");
+	if (!original_track_label) {
+		ast_test_status_update(test, "Original topology stream 0 does not contain metadata\n");
+		return AST_TEST_FAIL;
+	}
+	cloned_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(cloned, 0),
+		"AST_STREAM_METADATA_TRACK_LABEL");
+	if (!cloned_track_label) {
+		ast_test_status_update(test, "Cloned topology stream 0 does not contain metadata\n");
+		return AST_TEST_FAIL;
+	}
+	if (strcmp(original_track_label, cloned_track_label) != 0) {
+		ast_test_status_update(test, "Cloned topology stream 0 track label was not the same as the original\n");
+		return AST_TEST_FAIL;
+	}
+
 	if (ast_stream_get_type(ast_stream_topology_get_stream(cloned, 1)) != ast_stream_get_type(ast_stream_topology_get_stream(topology, 1))) {
 		ast_test_status_update(test, "Cloned video stream does not contain same type as original\n");
+		return AST_TEST_FAIL;
+	}
+
+	original_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(topology, 1),
+		"AST_STREAM_METADATA_TRACK_LABEL");
+	if (!original_track_label) {
+		ast_test_status_update(test, "Original topology stream 1 does not contain metadata\n");
+		return AST_TEST_FAIL;
+	}
+	cloned_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(cloned, 1),
+		"AST_STREAM_METADATA_TRACK_LABEL");
+	if (!cloned_track_label) {
+		ast_test_status_update(test, "Cloned topology stream 1 does not contain metadata\n");
+		return AST_TEST_FAIL;
+	}
+	if (strcmp(original_track_label, cloned_track_label) != 0) {
+		ast_test_status_update(test, "Cloned topology stream 1 track label was not the same as the original\n");
 		return AST_TEST_FAIL;
 	}
 
@@ -2139,6 +2257,7 @@
 	AST_TEST_UNREGISTER(stream_set_type);
 	AST_TEST_UNREGISTER(stream_set_formats);
 	AST_TEST_UNREGISTER(stream_set_state);
+	AST_TEST_UNREGISTER(stream_metadata);
 	AST_TEST_UNREGISTER(stream_topology_create);
 	AST_TEST_UNREGISTER(stream_topology_clone);
 	AST_TEST_UNREGISTER(stream_topology_clone);
@@ -2169,6 +2288,7 @@
 	AST_TEST_REGISTER(stream_set_type);
 	AST_TEST_REGISTER(stream_set_formats);
 	AST_TEST_REGISTER(stream_set_state);
+	AST_TEST_REGISTER(stream_metadata);
 	AST_TEST_REGISTER(stream_topology_create);
 	AST_TEST_REGISTER(stream_topology_clone);
 	AST_TEST_REGISTER(stream_topology_append_stream);

-- 
To view, visit https://gerrit.asterisk.org/8796
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863
Gerrit-Change-Number: 8796
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180417/53806d10/attachment-0001.html>


More information about the asterisk-code-review mailing list