[Asterisk-code-review] streams: Add string metadata capability (asterisk[15])
George Joseph
asteriskteam at digium.com
Fri Apr 13 15:43:24 CDT 2018
George Joseph has uploaded this change for review. ( 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.
Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863
---
M include/asterisk/stream.h
M main/stream.c
M tests/test_stream.c
3 files changed, 230 insertions(+), 50 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/81/8781/1
diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index bbde73d..3ba81d7 100644
--- a/include/asterisk/stream.h
+++ b/include/asterisk/stream.h
@@ -78,17 +78,13 @@
};
/*!
- * \brief Stream data slots
+ * \brief Stream metadata types
*/
-enum ast_stream_data_slot {
+enum ast_stream_metadata_type {
/*!
- * \brief Data slot for RTP instance
+ * \brief Track Label
*/
- AST_STREAM_DATA_RTP_CODECS = 0,
- /*!
- * \brief Controls the size of the data pointer array
- */
- AST_STREAM_DATA_SLOT_MAX
+ AST_STREAM_METADATA_TRACK_LABEL = 0,
};
/*!
@@ -228,32 +224,33 @@
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_type One of ast_stream_metadata_type
*
- * \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(struct ast_stream *stream,
+ enum ast_stream_metadata_type m_type);
/*!
- * \brief Set the opaque stream data
+ * \brief Set a stream metadata value
*
* \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.
+ * \param m_type One of ast_stream_metadata_type
+ * \param value String metadata value or NULL to remove existing value
*
- * \return data
+ * \retval -1 failure
+ * \retval 0 success
*
- * \since 15
+ * \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);
+int ast_stream_set_metadata(struct ast_stream *stream, enum ast_stream_metadata_type m_type,
+ const char *value);
/*!
* \brief Get the position of the stream in the topology
diff --git a/main/stream.c b/main/stream.c
index 8597485..be2f054 100644
--- a/main/stream.c
+++ b/main/stream.c
@@ -34,6 +34,12 @@
#include "asterisk/strings.h"
#include "asterisk/format.h"
#include "asterisk/format_cap.h"
+#include "asterisk/vector.h"
+
+struct ast_stream_metadata_entry {
+ enum ast_stream_metadata_type m_type;
+ char value[0];
+};
struct ast_stream {
/*!
@@ -57,14 +63,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];
+ AST_VECTOR(, struct ast_stream_metadata_entry *) metadata;
/*!
* \brief The group that the stream is part of
@@ -87,6 +88,7 @@
struct ast_stream *ast_stream_alloc(const char *name, enum ast_media_type type)
{
struct ast_stream *stream;
+ int rc;
stream = ast_calloc(1, sizeof(*stream) + strlen(S_OR(name, "")) + 1);
if (!stream) {
@@ -98,6 +100,16 @@
stream->group = -1;
strcpy(stream->name, S_OR(name, "")); /* Safe */
+ /*
+ * Since the elements are just 1 pointer, it's easier to initialize the vector always.
+ * It saves having to check to see if it's initialized when adding a new value.
+ */
+ rc = AST_VECTOR_INIT(&stream->metadata, 1);
+ if (rc != 0) {
+ ast_stream_free(stream);
+ return NULL;
+ }
+
return stream;
}
@@ -105,8 +117,9 @@
{
struct ast_stream *new_stream;
size_t stream_size;
- int idx;
const char *stream_name;
+ int rc;
+ int i;
if (!stream) {
return NULL;
@@ -126,10 +139,29 @@
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;
+ /* Initialize the vector to at least 1 element or the number of elements in the source */
+ rc = AST_VECTOR_INIT(&new_stream->metadata, AST_VECTOR_SIZE(&stream->metadata) ?: 1);
+ if (rc != 0) {
+ ast_stream_free(new_stream);
+ return NULL;
+ }
+
+ for (i = 0; i < AST_VECTOR_SIZE(&stream->metadata); i++) {
+ struct ast_stream_metadata_entry *entry = AST_VECTOR_GET(&stream->metadata, i);
+ struct ast_stream_metadata_entry *new_entry =
+ ast_malloc(sizeof(*new_entry) + strlen(entry->value + 1));
+ if (!new_entry) {
+ ast_stream_free(new_stream);
+ return NULL;
+ }
+ new_entry->m_type = entry->m_type;
+ strcpy(new_entry->value, entry->value); /* Safe */
+ rc = AST_VECTOR_APPEND(&new_stream->metadata, new_entry);
+ if (rc != 0) {
+ ast_free(new_entry);
+ ast_stream_free(new_stream);
+ return NULL;
+ }
}
return new_stream;
@@ -137,17 +169,12 @@
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_VECTOR_RESET(&stream->metadata, ast_free_ptr);
+ AST_VECTOR_FREE(&stream->metadata);
ao2_cleanup(stream->formats);
ast_free(stream);
@@ -221,22 +248,58 @@
}
}
-void *ast_stream_get_data(struct ast_stream *stream, enum ast_stream_data_slot slot)
+const char *ast_stream_get_metadata(struct ast_stream *stream,
+ enum ast_stream_metadata_type m_type)
{
- ast_assert(stream != NULL);
+ int i;
- return stream->data[slot];
+ ast_assert_else(stream != NULL, return NULL);
+
+ for (i = 0; i < AST_VECTOR_SIZE(&stream->metadata); i++) {
+ struct ast_stream_metadata_entry *entry = AST_VECTOR_GET(&stream->metadata, i);
+ if (entry->m_type == m_type) {
+ return entry->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)
+int ast_stream_set_metadata(struct ast_stream *stream, enum ast_stream_metadata_type m_type,
+ const char *value)
{
- ast_assert(stream != NULL);
+ struct ast_stream_metadata_entry *new_entry;
+ int i;
+ int rc;
- stream->data[slot] = data;
- stream->data_free_fn[slot] = data_free_fn;
+ ast_assert_else(stream != NULL, return -1);
- return data;
+ for (i = 0; i < AST_VECTOR_SIZE(&stream->metadata); i++) {
+ struct ast_stream_metadata_entry *entry = AST_VECTOR_GET(&stream->metadata, i);
+ if (entry->m_type == m_type) {
+ AST_VECTOR_REMOVE_UNORDERED(&stream->metadata, i);
+ ast_free(entry);
+ break;
+ }
+ }
+
+ if (!value) {
+ return 0;
+ }
+
+ new_entry = ast_malloc(sizeof(*new_entry) + strlen(value) + 1);
+ if (!new_entry) {
+ return -1;
+ }
+ new_entry->m_type = m_type;
+ strcpy(new_entry->value, value); /* Safe */
+ rc = AST_VECTOR_APPEND(&stream->metadata, new_entry);
+ if (rc != 0) {
+ ast_free(new_entry);
+ return -1;
+ }
+
+ 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..c0f50c8 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: newchange
Gerrit-Change-Id: Id7473aa4b374d7ab53046c20e321037ba9a56863
Gerrit-Change-Number: 8781
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/20180413/d89e6c9a/attachment-0001.html>
More information about the asterisk-code-review
mailing list