<p>George Joseph has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/8796">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>Added stream metadata dump to "core show channel"<br><br>Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863<br>---<br>M include/asterisk/stream.h<br>M main/cli.c<br>M main/sdp.c<br>M main/sdp_state.c<br>M main/stream.c<br>M tests/test_stream.c<br>6 files changed, 293 insertions(+), 61 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/96/8796/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 c2d5a88..0a5550b 100644<br>--- a/include/asterisk/stream.h<br>+++ b/include/asterisk/stream.h<br>@@ -78,20 +78,6 @@<br> };<br> <br> /*!<br>- * \brief Stream data slots<br>- */<br>-enum ast_stream_data_slot {<br>- /*!<br>- * \brief Data slot for RTP instance<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>-};<br>-<br>-/*!<br> * \brief Create a new media stream representation<br> *<br> * \param name A name for the stream<br>@@ -239,32 +225,47 @@<br> enum ast_stream_state ast_stream_str2state(const char *str);<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_key An arbitrary metadata key<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(const struct ast_stream *stream,<br>+ const char *m_key);<br> <br> /*!<br>- * \brief Set the opaque stream data<br>+ * \brief Get all stream metadata keys<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> *<br>- * \return data<br>+ * \retval An ast_variable list of the metadata key/value pairs.<br>+ * \retval NULL if error or no variables are set.<br> *<br>- * \since 15<br>+ * When you're finished with the list, you must call<br>+ * ast_variables_destroy(list);<br>+ *<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>+struct ast_variable *ast_stream_get_metadata_list(const struct ast_stream *stream);<br>+<br>+/*!<br>+ * \brief Set a stream metadata value<br>+ *<br>+ * \param stream The media stream<br>+ * \param m_key An arbitrary metadata key<br>+ * \param value String metadata value or NULL to remove existing value<br>+ *<br>+ * \retval -1 failure<br>+ * \retval 0 success<br>+ *<br>+ * \since 15.5<br>+ */<br>+int ast_stream_set_metadata(struct ast_stream *stream, const char *m_key, const char *value);<br> <br> /*!<br> * \brief Get the position of the stream in the topology<br>@@ -278,6 +279,27 @@<br> int ast_stream_get_position(const struct ast_stream *stream);<br> <br> /*!<br>+ * \brief Get rtp_codecs associated with the stream<br>+ *<br>+ * \param stream The media stream<br>+ *<br>+ * \return The rtp_codecs<br>+ *<br>+ * \since 15.5<br>+ */<br>+struct ast_rtp_codecs *ast_stream_get_rtp_codecs(const struct ast_stream *stream);<br>+<br>+/*!<br>+ * \brief Set rtp_codecs associated with the stream<br>+ *<br>+ * \param stream The media stream<br>+ * \param rtp_codecs The rtp_codecs<br>+ *<br>+ * \since 15.5<br>+ */<br>+void ast_stream_set_rtp_codecs(struct ast_stream *stream, struct ast_rtp_codecs *rtp_codecs);<br>+<br>+/*!<br> * \brief Create a stream topology<br> *<br> * \retval non-NULL success<br>diff --git a/main/cli.c b/main/cli.c<br>index 5730be1..cf51d0d 100644<br>--- a/main/cli.c<br>+++ b/main/cli.c<br>@@ -1614,19 +1614,29 @@<br> ast_str_append(&output, 0, " -- Streams --\n");<br> for (stream_num = 0; stream_num < ast_stream_topology_get_count(ast_channel_get_stream_topology(chan)); stream_num++) {<br> struct ast_stream *stream = ast_stream_topology_get_stream(ast_channel_get_stream_topology(chan), stream_num);<br>+ struct ast_variable *metadata = ast_stream_get_metadata_list(stream);<br> <br> ast_str_append(&output, 0,<br> "Name: %s\n"<br> " Type: %s\n"<br> " State: %s\n"<br> " Group: %d\n"<br>- " Formats: %s\n",<br>+ " Formats: %s\n"<br>+ " Metadata:\n",<br> ast_stream_get_name(stream),<br> ast_codec_media_type2str(ast_stream_get_type(stream)),<br> ast_stream_state2str(ast_stream_get_state(stream)),<br> ast_stream_get_group(stream),<br> ast_format_cap_get_names(ast_stream_get_formats(stream), &codec_buf)<br> );<br>+<br>+ if (metadata) {<br>+ struct ast_variable *v;<br>+ for(v = metadata; v; v = v->next) {<br>+ ast_str_append(&output, 0, " %s: %s\n", v->name, v->value);<br>+ }<br>+ ast_variables_destroy(metadata);<br>+ }<br> }<br> <br> ast_channel_unlock(chan);<br>diff --git a/main/sdp.c b/main/sdp.c<br>index 7e283eb..e5c8803 100644<br>--- a/main/sdp.c<br>+++ b/main/sdp.c<br>@@ -848,8 +848,7 @@<br> ast_free(codecs);<br> return NULL;<br> }<br>- ast_stream_set_data(stream, AST_STREAM_DATA_RTP_CODECS, codecs,<br>- (ast_stream_data_free_fn) rtp_codecs_free);<br>+ ast_stream_set_rtp_codecs(stream, codecs);<br> <br> if (!m_line->port) {<br> /* Stream is declined. There may not be any attributes. */<br>diff --git a/main/sdp_state.c b/main/sdp_state.c<br>index 5f9ad5e..e022277 100644<br>--- a/main/sdp_state.c<br>+++ b/main/sdp_state.c<br>@@ -1732,7 +1732,7 @@<br> case AST_MEDIA_TYPE_AUDIO:<br> case AST_MEDIA_TYPE_VIDEO:<br> ao2_bump(joint_state_stream->rtp);<br>- codecs = ast_stream_get_data(remote_stream, AST_STREAM_DATA_RTP_CODECS);<br>+ codecs = ast_stream_get_rtp_codecs(remote_stream);<br> ast_assert(codecs != NULL);<br> if (sdp_state->role == SDP_ROLE_ANSWERER) {<br> /*<br>@@ -1789,7 +1789,7 @@<br> * Setup rx payload type mapping to prefer the mapping<br> * from the peer that the RFC says we SHOULD use.<br> */<br>- codecs = ast_stream_get_data(remote_stream, AST_STREAM_DATA_RTP_CODECS);<br>+ codecs = ast_stream_get_rtp_codecs(remote_stream);<br> ast_assert(codecs != NULL);<br> ast_rtp_codecs_payloads_xover(codecs, codecs, NULL);<br> ast_rtp_codecs_payloads_copy(codecs,<br>diff --git a/main/stream.c b/main/stream.c<br>index 61eef25..dd3e765 100644<br>--- a/main/stream.c<br>+++ b/main/stream.c<br>@@ -34,6 +34,15 @@<br> #include "asterisk/strings.h"<br> #include "asterisk/format.h"<br> #include "asterisk/format_cap.h"<br>+#include "asterisk/vector.h"<br>+#include "asterisk/config.h"<br>+#include "asterisk/rtp_engine.h"<br>+<br>+struct ast_stream_metadata_entry {<br>+ size_t length;<br>+ int value_start;<br>+ char name_value[0];<br>+};<br> <br> struct ast_stream {<br> /*!<br>@@ -57,19 +66,19 @@<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>+ struct ast_variable *metadata;<br> <br> /*!<br> * \brief The group that the stream is part of<br> */<br> int group;<br>+<br>+ /*!<br>+ * \brief The rtp_codecs used by the stream<br>+ */<br>+ struct ast_rtp_codecs *rtp_codecs;<br> <br> /*!<br> * \brief Name for the stream within the context of the channel it is on<br>@@ -105,7 +114,6 @@<br> {<br> struct ast_stream *new_stream;<br> size_t stream_size;<br>- int idx;<br> const char *stream_name;<br> <br> if (!stream) {<br>@@ -126,27 +134,23 @@<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>- }<br>+ new_stream->metadata = ast_stream_get_metadata_list(stream);<br>+<br>+ /* rtp_codecs aren't cloned */<br> <br> return new_stream;<br> }<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>+ ast_variables_destroy(stream->metadata);<br>+<br>+ if (stream->rtp_codecs) {<br>+ ast_rtp_codecs_payloads_destroy(stream->rtp_codecs);<br> }<br> <br> ao2_cleanup(stream->formats);<br>@@ -238,22 +242,81 @@<br> return AST_STREAM_STATE_REMOVED;<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(const struct ast_stream *stream, const char *m_key)<br> {<br>- ast_assert(stream != NULL);<br>+ struct ast_variable *v;<br> <br>- return stream->data[slot];<br>+ ast_assert_return(stream != NULL, NULL);<br>+ ast_assert_return(m_key != NULL, NULL);<br>+<br>+ for (v = stream->metadata; v; v = v->next) {<br>+ if (strcmp(v->name, m_key) == 0) {<br>+ return v->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>+struct ast_variable *ast_stream_get_metadata_list(const struct ast_stream *stream)<br> {<br>- ast_assert(stream != NULL);<br>+ struct ast_variable *v;<br>+ struct ast_variable *vout = NULL;<br> <br>- stream->data[slot] = data;<br>- stream->data_free_fn[slot] = data_free_fn;<br>+ ast_assert_return(stream != NULL, NULL);<br> <br>- return data;<br>+ for (v = stream->metadata; v; v = v->next) {<br>+ struct ast_variable *vt = ast_variable_new(v->name, v->value, "");<br>+<br>+ if (!vt) {<br>+ ast_variables_destroy(vout);<br>+ return NULL;<br>+ }<br>+<br>+ ast_variable_list_append(&vout, vt);<br>+ }<br>+<br>+ return vout;<br>+}<br>+<br>+int ast_stream_set_metadata(struct ast_stream *stream, const char *m_key, const char *value)<br>+{<br>+ struct ast_variable *v;<br>+ struct ast_variable *prev;<br>+<br>+ ast_assert_return(stream != NULL, -1);<br>+ ast_assert_return(m_key != NULL, -1);<br>+<br>+ prev = NULL;<br>+ v = stream->metadata;<br>+ while(v) {<br>+ struct ast_variable *next = v->next;<br>+ if (strcmp(v->name, m_key) == 0) {<br>+ if (prev) {<br>+ prev->next = next;<br>+ } else {<br>+ stream->metadata = next;<br>+ }<br>+ ast_free(v);<br>+ break;<br>+ } else {<br>+ prev = v;<br>+ }<br>+ v = next;<br>+ }<br>+<br>+ if (!value) {<br>+ return 0;<br>+ }<br>+<br>+ v = ast_variable_new(m_key, value, "");<br>+ if (!v) {<br>+ return -1;<br>+ }<br>+<br>+ ast_variable_list_append(&stream->metadata, v);<br>+<br>+ return 0;<br> }<br> <br> int ast_stream_get_position(const struct ast_stream *stream)<br>@@ -263,6 +326,24 @@<br> return stream->position;<br> }<br> <br>+struct ast_rtp_codecs *ast_stream_get_rtp_codecs(const struct ast_stream *stream)<br>+{<br>+ ast_assert(stream != NULL);<br>+<br>+ return stream->rtp_codecs;<br>+}<br>+<br>+void ast_stream_set_rtp_codecs(struct ast_stream *stream, struct ast_rtp_codecs *rtp_codecs)<br>+{<br>+ ast_assert(stream != NULL);<br>+<br>+ if (stream->rtp_codecs) {<br>+ ast_rtp_codecs_payloads_destroy(rtp_codecs);<br>+ }<br>+<br>+ stream->rtp_codecs = rtp_codecs;<br>+}<br>+<br> #define TOPOLOGY_INITIAL_STREAM_COUNT 2<br> struct ast_stream_topology *ast_stream_topology_alloc(void)<br> {<br>diff --git a/tests/test_stream.c b/tests/test_stream.c<br>index 8c88704..50f0ccb 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/8796">change 8796</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/8796"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </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: 8796 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>