<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/8781">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">streams: Add string metadata capability<br><br>Replaces the never used opaque data array.<br><br>Updated stream tests to include get/set metadata and<br>stream clone with metadata.<br><br>Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863<br>---<br>M include/asterisk/stream.h<br>M main/stream.c<br>M tests/test_stream.c<br>3 files changed, 230 insertions(+), 50 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/81/8781/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h<br>index bbde73d..3ba81d7 100644<br>--- a/include/asterisk/stream.h<br>+++ b/include/asterisk/stream.h<br>@@ -78,17 +78,13 @@<br> };<br> <br> /*!<br>- * \brief Stream data slots<br>+ * \brief Stream metadata types<br> */<br>-enum ast_stream_data_slot {<br>+enum ast_stream_metadata_type {<br> /*!<br>- * \brief Data slot for RTP instance<br>+ * \brief Track Label<br> */<br>- AST_STREAM_DATA_RTP_CODECS = 0,<br>- /*!<br>- * \brief Controls the size of the data pointer array<br>- */<br>- AST_STREAM_DATA_SLOT_MAX<br>+ AST_STREAM_METADATA_TRACK_LABEL = 0,<br> };<br> <br> /*!<br>@@ -228,32 +224,33 @@<br> const char *ast_stream_state2str(enum ast_stream_state state);<br> <br> /*!<br>- * \brief Get the opaque stream data<br>+ * \brief Get a stream metadata value<br> *<br> * \param stream The media stream<br>- * \param slot The data slot to retrieve<br>+ * \param m_type One of ast_stream_metadata_type<br> *<br>- * \retval non-NULL success<br>- * \retval NULL failure<br>+ * \retval non-NULL metadata value<br>+ * \retval NULL failure or not found<br> *<br>- * \since 15<br>+ * \since 15.5<br> */<br>-void *ast_stream_get_data(struct ast_stream *stream, enum ast_stream_data_slot slot);<br>+const char *ast_stream_get_metadata(struct ast_stream *stream,<br>+ enum ast_stream_metadata_type m_type);<br> <br> /*!<br>- * \brief Set the opaque stream data<br>+ * \brief Set a stream metadata value<br> *<br> * \param stream The media stream<br>- * \param slot The data slot to set<br>- * \param data Opaque data<br>- * \param data_free_fn Callback to free data when stream is freed. May be NULL for no action.<br>+ * \param m_type One of ast_stream_metadata_type<br>+ * \param value String metadata value or NULL to remove existing value<br> *<br>- * \return data<br>+ * \retval -1 failure<br>+ * \retval 0 success<br> *<br>- * \since 15<br>+ * \since 15.5<br> */<br>-void *ast_stream_set_data(struct ast_stream *stream, enum ast_stream_data_slot slot,<br>- void *data, ast_stream_data_free_fn data_free_fn);<br>+int ast_stream_set_metadata(struct ast_stream *stream, enum ast_stream_metadata_type m_type,<br>+ const char *value);<br> <br> /*!<br> * \brief Get the position of the stream in the topology<br>diff --git a/main/stream.c b/main/stream.c<br>index 8597485..be2f054 100644<br>--- a/main/stream.c<br>+++ b/main/stream.c<br>@@ -34,6 +34,12 @@<br> #include "asterisk/strings.h"<br> #include "asterisk/format.h"<br> #include "asterisk/format_cap.h"<br>+#include "asterisk/vector.h"<br>+<br>+struct ast_stream_metadata_entry {<br>+ enum ast_stream_metadata_type m_type;<br>+ char value[0];<br>+};<br> <br> struct ast_stream {<br> /*!<br>@@ -57,14 +63,9 @@<br> enum ast_stream_state state;<br> <br> /*!<br>- * \brief Opaque stream data<br>+ * \brief Stream metadata vector<br> */<br>- void *data[AST_STREAM_DATA_SLOT_MAX];<br>-<br>- /*!<br>- * \brief What to do with data when the stream is freed<br>- */<br>- ast_stream_data_free_fn data_free_fn[AST_STREAM_DATA_SLOT_MAX];<br>+ AST_VECTOR(, struct ast_stream_metadata_entry *) metadata;<br> <br> /*!<br> * \brief The group that the stream is part of<br>@@ -87,6 +88,7 @@<br> struct ast_stream *ast_stream_alloc(const char *name, enum ast_media_type type)<br> {<br> struct ast_stream *stream;<br>+ int rc;<br> <br> stream = ast_calloc(1, sizeof(*stream) + strlen(S_OR(name, "")) + 1);<br> if (!stream) {<br>@@ -98,6 +100,16 @@<br> stream->group = -1;<br> strcpy(stream->name, S_OR(name, "")); /* Safe */<br> <br>+ /*<br>+ * Since the elements are just 1 pointer, it's easier to initialize the vector always.<br>+ * It saves having to check to see if it's initialized when adding a new value.<br>+ */<br>+ rc = AST_VECTOR_INIT(&stream->metadata, 1);<br>+ if (rc != 0) {<br>+ ast_stream_free(stream);<br>+ return NULL;<br>+ }<br>+<br> return stream;<br> }<br> <br>@@ -105,8 +117,9 @@<br> {<br> struct ast_stream *new_stream;<br> size_t stream_size;<br>- int idx;<br> const char *stream_name;<br>+ int rc;<br>+ int i;<br> <br> if (!stream) {<br> return NULL;<br>@@ -126,10 +139,29 @@<br> ao2_ref(new_stream->formats, +1);<br> }<br> <br>- /* We cannot clone the opaque data because we don't know how. */<br>- for (idx = 0; idx < AST_STREAM_DATA_SLOT_MAX; ++idx) {<br>- new_stream->data[idx] = NULL;<br>- new_stream->data_free_fn[idx] = NULL;<br>+ /* Initialize the vector to at least 1 element or the number of elements in the source */<br>+ rc = AST_VECTOR_INIT(&new_stream->metadata, AST_VECTOR_SIZE(&stream->metadata) ?: 1);<br>+ if (rc != 0) {<br>+ ast_stream_free(new_stream);<br>+ return NULL;<br>+ }<br>+<br>+ for (i = 0; i < AST_VECTOR_SIZE(&stream->metadata); i++) {<br>+ struct ast_stream_metadata_entry *entry = AST_VECTOR_GET(&stream->metadata, i);<br>+ struct ast_stream_metadata_entry *new_entry =<br>+ ast_malloc(sizeof(*new_entry) + strlen(entry->value + 1));<br>+ if (!new_entry) {<br>+ ast_stream_free(new_stream);<br>+ return NULL;<br>+ }<br>+ new_entry->m_type = entry->m_type;<br>+ strcpy(new_entry->value, entry->value); /* Safe */<br>+ rc = AST_VECTOR_APPEND(&new_stream->metadata, new_entry);<br>+ if (rc != 0) {<br>+ ast_free(new_entry);<br>+ ast_stream_free(new_stream);<br>+ return NULL;<br>+ }<br> }<br> <br> return new_stream;<br>@@ -137,17 +169,12 @@<br> <br> void ast_stream_free(struct ast_stream *stream)<br> {<br>- int i;<br>-<br> if (!stream) {<br> return;<br> }<br> <br>- for (i = 0; i < AST_STREAM_DATA_SLOT_MAX; i++) {<br>- if (stream->data_free_fn[i]) {<br>- stream->data_free_fn[i](stream->data[i]);<br>- }<br>- }<br>+ AST_VECTOR_RESET(&stream->metadata, ast_free_ptr);<br>+ AST_VECTOR_FREE(&stream->metadata);<br> <br> ao2_cleanup(stream->formats);<br> ast_free(stream);<br>@@ -221,22 +248,58 @@<br> }<br> }<br> <br>-void *ast_stream_get_data(struct ast_stream *stream, enum ast_stream_data_slot slot)<br>+const char *ast_stream_get_metadata(struct ast_stream *stream,<br>+ enum ast_stream_metadata_type m_type)<br> {<br>- ast_assert(stream != NULL);<br>+ int i;<br> <br>- return stream->data[slot];<br>+ ast_assert_else(stream != NULL, return NULL);<br>+<br>+ for (i = 0; i < AST_VECTOR_SIZE(&stream->metadata); i++) {<br>+ struct ast_stream_metadata_entry *entry = AST_VECTOR_GET(&stream->metadata, i);<br>+ if (entry->m_type == m_type) {<br>+ return entry->value;<br>+ }<br>+ }<br>+<br>+ return NULL;<br> }<br> <br>-void *ast_stream_set_data(struct ast_stream *stream, enum ast_stream_data_slot slot,<br>- void *data, ast_stream_data_free_fn data_free_fn)<br>+int ast_stream_set_metadata(struct ast_stream *stream, enum ast_stream_metadata_type m_type,<br>+ const char *value)<br> {<br>- ast_assert(stream != NULL);<br>+ struct ast_stream_metadata_entry *new_entry;<br>+ int i;<br>+ int rc;<br> <br>- stream->data[slot] = data;<br>- stream->data_free_fn[slot] = data_free_fn;<br>+ ast_assert_else(stream != NULL, return -1);<br> <br>- return data;<br>+ for (i = 0; i < AST_VECTOR_SIZE(&stream->metadata); i++) {<br>+ struct ast_stream_metadata_entry *entry = AST_VECTOR_GET(&stream->metadata, i);<br>+ if (entry->m_type == m_type) {<br>+ AST_VECTOR_REMOVE_UNORDERED(&stream->metadata, i);<br>+ ast_free(entry);<br>+ break;<br>+ }<br>+ }<br>+<br>+ if (!value) {<br>+ return 0;<br>+ }<br>+<br>+ new_entry = ast_malloc(sizeof(*new_entry) + strlen(value) + 1);<br>+ if (!new_entry) {<br>+ return -1;<br>+ }<br>+ new_entry->m_type = m_type;<br>+ strcpy(new_entry->value, value); /* Safe */<br>+ rc = AST_VECTOR_APPEND(&stream->metadata, new_entry);<br>+ if (rc != 0) {<br>+ ast_free(new_entry);<br>+ return -1;<br>+ }<br>+<br>+ return 0;<br> }<br> <br> int ast_stream_get_position(const struct ast_stream *stream)<br>diff --git a/tests/test_stream.c b/tests/test_stream.c<br>index 8c88704..c0f50c8 100644<br>--- a/tests/test_stream.c<br>+++ b/tests/test_stream.c<br>@@ -38,6 +38,7 @@<br> #include "asterisk/format_cap.h"<br> #include "asterisk/format_cache.h"<br> #include "asterisk/channel.h"<br>+#include "asterisk/uuid.h"<br> <br> AST_TEST_DEFINE(stream_create)<br> {<br>@@ -224,6 +225,70 @@<br> return AST_TEST_PASS;<br> }<br> <br>+AST_TEST_DEFINE(stream_metadata)<br>+{<br>+ RAII_VAR(struct ast_stream *, stream, NULL, ast_stream_free);<br>+ char track_label[AST_UUID_STR_LEN + 1];<br>+ const char *stream_track_label;<br>+ int rc;<br>+<br>+ switch (cmd) {<br>+ case TEST_INIT:<br>+ info->name = "stream_metadata";<br>+ info->category = "/main/stream/";<br>+ info->summary = "stream metadata unit test";<br>+ info->description =<br>+ "Test that metadata operations on a stream works";<br>+ return AST_TEST_NOT_RUN;<br>+ case TEST_EXECUTE:<br>+ break;<br>+ }<br>+<br>+ stream = ast_stream_alloc("test", AST_MEDIA_TYPE_AUDIO);<br>+ if (!stream) {<br>+ ast_test_status_update(test, "Failed to create media stream given proper arguments\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ stream_track_label = ast_stream_get_metadata(stream, AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (stream_track_label) {<br>+ ast_test_status_update(test, "New stream HAD a track label\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ ast_uuid_generate_str(track_label, sizeof(track_label));<br>+ rc = ast_stream_set_metadata(stream, AST_STREAM_METADATA_TRACK_LABEL, track_label);<br>+ if (rc != 0) {<br>+ ast_test_status_update(test, "Failed to add track label\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ stream_track_label = ast_stream_get_metadata(stream, AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (!stream_track_label) {<br>+ ast_test_status_update(test, "Changed stream does not have a track label\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ if (strcmp(stream_track_label, track_label) != 0) {<br>+ ast_test_status_update(test, "Changed stream did not return same track label\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ rc = ast_stream_set_metadata(stream, AST_STREAM_METADATA_TRACK_LABEL, NULL);<br>+ if (rc != 0) {<br>+ ast_test_status_update(test, "Failed to remove track label\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ stream_track_label = ast_stream_get_metadata(stream, AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (stream_track_label) {<br>+ ast_test_status_update(test, "Changed stream still had a track label after we removed it\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ return AST_TEST_PASS;<br>+}<br>+<br> AST_TEST_DEFINE(stream_topology_create)<br> {<br> RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);<br>@@ -254,6 +319,11 @@<br> RAII_VAR(struct ast_stream_topology *, topology, NULL, ast_stream_topology_free);<br> RAII_VAR(struct ast_stream_topology *, cloned, NULL, ast_stream_topology_free);<br> struct ast_stream *audio_stream, *video_stream;<br>+ char audio_track_label[AST_UUID_STR_LEN + 1];<br>+ char video_track_label[AST_UUID_STR_LEN + 1];<br>+ const char *original_track_label;<br>+ const char *cloned_track_label;<br>+ int rc;<br> <br> switch (cmd) {<br> case TEST_INIT:<br>@@ -279,6 +349,13 @@<br> return AST_TEST_FAIL;<br> }<br> <br>+ ast_uuid_generate_str(audio_track_label, sizeof(audio_track_label));<br>+ rc = ast_stream_set_metadata(audio_stream, AST_STREAM_METADATA_TRACK_LABEL, audio_track_label);<br>+ if (rc != 0) {<br>+ ast_test_status_update(test, "Failed to add track label\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br> if (ast_stream_topology_append_stream(topology, audio_stream) == -1) {<br> ast_test_status_update(test, "Failed to append valid audio stream to stream topology\n");<br> ast_stream_free(audio_stream);<br>@@ -288,6 +365,13 @@<br> video_stream = ast_stream_alloc("video", AST_MEDIA_TYPE_VIDEO);<br> if (!video_stream) {<br> ast_test_status_update(test, "Failed to create a video stream for testing stream topology\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ ast_uuid_generate_str(video_track_label, sizeof(video_track_label));<br>+ rc = ast_stream_set_metadata(video_stream, AST_STREAM_METADATA_TRACK_LABEL, video_track_label);<br>+ if (rc != 0) {<br>+ ast_test_status_update(test, "Failed to add track label\n");<br> return AST_TEST_FAIL;<br> }<br> <br>@@ -313,8 +397,42 @@<br> return AST_TEST_FAIL;<br> }<br> <br>+ original_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(topology, 0),<br>+ AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (!original_track_label) {<br>+ ast_test_status_update(test, "Original topology stream 0 does not contain metadata\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+ cloned_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(cloned, 0),<br>+ AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (!cloned_track_label) {<br>+ ast_test_status_update(test, "Cloned topology stream 0 does not contain metadata\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+ if (strcmp(original_track_label, cloned_track_label) != 0) {<br>+ ast_test_status_update(test, "Cloned topology stream 0 track label was not the same as the original\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br> if (ast_stream_get_type(ast_stream_topology_get_stream(cloned, 1)) != ast_stream_get_type(ast_stream_topology_get_stream(topology, 1))) {<br> ast_test_status_update(test, "Cloned video stream does not contain same type as original\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+<br>+ original_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(topology, 1),<br>+ AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (!original_track_label) {<br>+ ast_test_status_update(test, "Original topology stream 1 does not contain metadata\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+ cloned_track_label = ast_stream_get_metadata(ast_stream_topology_get_stream(cloned, 1),<br>+ AST_STREAM_METADATA_TRACK_LABEL);<br>+ if (!cloned_track_label) {<br>+ ast_test_status_update(test, "Cloned topology stream 1 does not contain metadata\n");<br>+ return AST_TEST_FAIL;<br>+ }<br>+ if (strcmp(original_track_label, cloned_track_label) != 0) {<br>+ ast_test_status_update(test, "Cloned topology stream 1 track label was not the same as the original\n");<br> return AST_TEST_FAIL;<br> }<br> <br>@@ -2139,6 +2257,7 @@<br> AST_TEST_UNREGISTER(stream_set_type);<br> AST_TEST_UNREGISTER(stream_set_formats);<br> AST_TEST_UNREGISTER(stream_set_state);<br>+ AST_TEST_UNREGISTER(stream_metadata);<br> AST_TEST_UNREGISTER(stream_topology_create);<br> AST_TEST_UNREGISTER(stream_topology_clone);<br> AST_TEST_UNREGISTER(stream_topology_clone);<br>@@ -2169,6 +2288,7 @@<br> AST_TEST_REGISTER(stream_set_type);<br> AST_TEST_REGISTER(stream_set_formats);<br> AST_TEST_REGISTER(stream_set_state);<br>+ AST_TEST_REGISTER(stream_metadata);<br> AST_TEST_REGISTER(stream_topology_create);<br> AST_TEST_REGISTER(stream_topology_clone);<br> AST_TEST_REGISTER(stream_topology_append_stream);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8781">change 8781</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/8781"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863 </div>
<div style="display:none"> Gerrit-Change-Number: 8781 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>