[Asterisk-code-review] streams: Add string metadata capability (asterisk[15])
Joshua Colp
asteriskteam at digium.com
Wed Apr 25 13:45:58 CDT 2018
Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/8781 )
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/stream.c
M tests/test_stream.c
4 files changed, 240 insertions(+), 58 deletions(-)
Approvals:
Benjamin Keith Ford: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved; Approved for Submit
diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index bbde73d..99998c7 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
@@ -228,32 +214,47 @@
const char *ast_stream_state2str(enum ast_stream_state state);
/*!
- * \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
diff --git a/main/cli.c b/main/cli.c
index 556c09b..b992726 100644
--- a/main/cli.c
+++ b/main/cli.c
@@ -1646,19 +1646,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/stream.c b/main/stream.c
index 8597485..5d4970a 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -34,6 +34,14 @@
#include "asterisk/strings.h"
#include "asterisk/format.h"
#include "asterisk/format_cap.h"
+#include "asterisk/vector.h"
+#include "asterisk/config.h"
+
+struct ast_stream_metadata_entry {
+ size_t length;
+ int value_start;
+ char name_value[0];
+};
struct ast_stream {
/*!
@@ -57,14 +65,9 @@
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
@@ -105,7 +108,6 @@
{
struct ast_stream *new_stream;
size_t stream_size;
- int idx;
const char *stream_name;
if (!stream) {
@@ -126,28 +128,18 @@
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);
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);
ao2_cleanup(stream->formats);
ast_free(stream);
@@ -221,22 +213,81 @@
}
}
-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)
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/8781
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863
Gerrit-Change-Number: 8781
Gerrit-PatchSet: 5
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180425/3edf1832/attachment-0001.html>
More information about the asterisk-code-review
mailing list