From ae7689f09333d88867a5eb6feeec8437c17dc1d2 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Fri, 28 Apr 2017 12:30:34 -0500 Subject: [PATCH 1/4] SDP: Update ast_get_topology_from_sdp() to keep RTP map. * Add failure exits to ast_get_topology_from_sdp(). Change-Id: I4cc85c1ede8d712766ed20f544dbcef04c8c1049 --- include/asterisk/sdp.h | 3 +- include/asterisk/stream.h | 70 +++++++++++++++++++-------------------- main/sdp.c | 64 ++++++++++++++++++++++++++--------- main/sdp_state.c | 3 +- tests/test_sdp.c | 6 +++- 5 files changed, 93 insertions(+), 53 deletions(-) diff --git a/include/asterisk/sdp.h b/include/asterisk/sdp.h index 06470c4b05..8aa9e3b7fa 100644 --- a/include/asterisk/sdp.h +++ b/include/asterisk/sdp.h @@ -638,11 +638,12 @@ void ast_sdp_rtpmap_free(struct ast_sdp_rtpmap *rtpmap); * each m-line corresponding to a stream in the created topology. * * \param sdp The SDP to convert + * \param g726_non_standard Non-zero if G.726 is non-standard * * \retval NULL An error occurred when converting * \retval non-NULL The generated stream topology * * \since 15.0.0 */ -struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp); +struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp, int g726_non_standard); #endif /* _SDP_PRIV_H */ diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h index 821ecec030..526aee17d2 100644 --- a/include/asterisk/stream.h +++ b/include/asterisk/stream.h @@ -55,39 +55,39 @@ typedef void (*ast_stream_data_free_fn)(void *); * \brief States that a stream may be in */ enum ast_stream_state { - /*! - * \brief Set when the stream has been removed - */ - AST_STREAM_STATE_REMOVED = 0, - /*! - * \brief Set when the stream is sending and receiving media - */ - AST_STREAM_STATE_SENDRECV, - /*! - * \brief Set when the stream is sending media only - */ - AST_STREAM_STATE_SENDONLY, - /*! - * \brief Set when the stream is receiving media only - */ - AST_STREAM_STATE_RECVONLY, - /*! - * \brief Set when the stream is not sending OR receiving media - */ - AST_STREAM_STATE_INACTIVE, + /*! + * \brief Set when the stream has been removed + */ + AST_STREAM_STATE_REMOVED = 0, + /*! + * \brief Set when the stream is sending and receiving media + */ + AST_STREAM_STATE_SENDRECV, + /*! + * \brief Set when the stream is sending media only + */ + AST_STREAM_STATE_SENDONLY, + /*! + * \brief Set when the stream is receiving media only + */ + AST_STREAM_STATE_RECVONLY, + /*! + * \brief Set when the stream is not sending OR receiving media + */ + AST_STREAM_STATE_INACTIVE, }; /*! * \brief Stream data slots */ enum ast_stream_data_slot { - /*! - * \brief Data slot for RTP instance - */ - AST_STREAM_DATA_RTP_INSTANCE = 0, - /*! - * \brief Controls the size of the data pointer array - */ + /*! + * \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 }; @@ -386,15 +386,15 @@ struct ast_stream_topology *ast_stream_topology_create_from_format_cap( * * \param topology The topology of streams * - * \retval non-NULL success - * \retval NULL failure - * - * \note The stream topology is NOT altered by this function. - * - * \since 15 - */ + * \retval non-NULL success + * \retval NULL failure + * + * \note The stream topology is NOT altered by this function. + * + * \since 15 + */ struct ast_format_cap *ast_format_cap_from_stream_topology( - struct ast_stream_topology *topology); + struct ast_stream_topology *topology); /*! * \brief Gets the first stream of a specific type from the topology diff --git a/main/sdp.c b/main/sdp.c index 62acdd3f75..019c6699f1 100644 --- a/main/sdp.c +++ b/main/sdp.c @@ -693,6 +693,17 @@ static void process_fmtp(const struct ast_sdp_m_line *m_line, int payload, ao2_ref(format, -1); } +/* + * Needed so we don't have an external function referenced as data. + * The dynamic linker doesn't handle that very well. + */ +static void rtp_codecs_free(struct ast_rtp_codecs *codecs) +{ + if (codecs) { + ast_rtp_codecs_payloads_destroy(codecs); + } +} + /*! * \brief Convert an SDP stream into an Asterisk stream * @@ -700,16 +711,19 @@ static void process_fmtp(const struct ast_sdp_m_line *m_line, int payload, * This takes formats, as well as clock-rate and fmtp attributes into account. * * \param m_line The SDP media section to convert + * \param g726_non_standard Non-zero if G.726 is non-standard + * * \retval NULL An error occurred * \retval non-NULL The converted stream */ -static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line) +static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line, int g726_non_standard) { int i; int non_ast_fmts; - struct ast_rtp_codecs codecs; + struct ast_rtp_codecs *codecs; struct ast_format_cap *caps; struct ast_stream *stream; + enum ast_rtp_options options; caps = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT); if (!caps) { @@ -724,8 +738,15 @@ static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line) switch (ast_stream_get_type(stream)) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - ast_rtp_codecs_payloads_initialize(&codecs); + codecs = ast_calloc(1, sizeof(*codecs)); + if (!codecs || ast_rtp_codecs_payloads_initialize(codecs)) { + rtp_codecs_free(codecs); + ast_stream_free(stream); + ao2_ref(caps, -1); + return NULL; + } + options = g726_non_standard ? AST_RTP_OPT_G726_NONSTANDARD : 0; for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) { struct ast_sdp_payload *payload_s; struct ast_sdp_rtpmap *rtpmap; @@ -733,22 +754,25 @@ static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line) payload_s = ast_sdp_m_get_payload(m_line, i); sscanf(payload_s->fmt, "%30d", &payload); - ast_rtp_codecs_payloads_set_m_type(&codecs, NULL, payload); rtpmap = sdp_payload_get_rtpmap(m_line, payload); if (!rtpmap) { + /* No rtpmap attribute. Try static payload type format assignment */ + ast_rtp_codecs_payloads_set_m_type(codecs, NULL, payload); continue; } - ast_rtp_codecs_payloads_set_rtpmap_type_rate(&codecs, NULL, - payload, m_line->type, rtpmap->encoding_name, 0, - rtpmap->clock_rate); - ast_sdp_rtpmap_free(rtpmap); - process_fmtp(m_line, payload, &codecs); + if (!ast_rtp_codecs_payloads_set_rtpmap_type_rate(codecs, NULL, payload, + m_line->type, rtpmap->encoding_name, options, rtpmap->clock_rate)) { + /* Successfully mapped the payload type to format */ + process_fmtp(m_line, payload, codecs); + } + ast_sdp_rtpmap_free(rtpmap); } - ast_rtp_codecs_payload_formats(&codecs, caps, &non_ast_fmts); - ast_rtp_codecs_payloads_destroy(&codecs); + ast_rtp_codecs_payload_formats(codecs, caps, &non_ast_fmts); + ast_stream_set_data(stream, AST_STREAM_DATA_RTP_CODECS, codecs, + (ast_stream_data_free_fn) rtp_codecs_free); break; case AST_MEDIA_TYPE_IMAGE: for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) { @@ -773,7 +797,7 @@ static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line) return stream; } -struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp) +struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp, int g726_non_standard) { struct ast_stream_topology *topology; int i; @@ -786,11 +810,21 @@ struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp) for (i = 0; i < ast_sdp_get_m_count(sdp); ++i) { struct ast_stream *stream; - stream = get_stream_from_m(ast_sdp_get_m(sdp, i)); + stream = get_stream_from_m(ast_sdp_get_m(sdp, i), g726_non_standard); if (!stream) { - continue; + /* + * The topology cannot match the SDP because + * we failed to create a corresponding stream. + */ + ast_stream_topology_free(topology); + return NULL; + } + if (ast_stream_topology_append_stream(topology, stream) < 0) { + /* Failed to add stream to topology */ + ast_stream_free(stream); + ast_stream_topology_free(topology); + return NULL; } - ast_stream_topology_append_stream(topology, stream); } return topology; diff --git a/main/sdp_state.c b/main/sdp_state.c index a9979eb7d9..6ce06ee523 100644 --- a/main/sdp_state.c +++ b/main/sdp_state.c @@ -1007,7 +1007,8 @@ static int merge_sdps(struct ast_sdp_state *sdp_state, const struct ast_sdp *rem struct ast_stream_topology *remote_capabilities; int i; - remote_capabilities = ast_get_topology_from_sdp(remote_sdp); + remote_capabilities = ast_get_topology_from_sdp(remote_sdp, + sdp_state->options->g726_non_standard); if (!remote_capabilities) { return -1; } diff --git a/tests/test_sdp.c b/tests/test_sdp.c index a5d3710d89..408888fc00 100644 --- a/tests/test_sdp.c +++ b/tests/test_sdp.c @@ -623,7 +623,11 @@ AST_TEST_DEFINE(sdp_to_topology) goto end; } - topology = ast_get_topology_from_sdp(sdp); + topology = ast_get_topology_from_sdp(sdp, 0); + if (!topology) { + res = AST_TEST_FAIL; + goto end; + } if (ast_stream_topology_get_count(topology) != 3) { ast_test_status_update(test, "Unexpected topology count '%d'. Expecting 2\n", From be5809fac8b423536a6af05817db5705c2977f53 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Fri, 28 Apr 2017 19:48:29 -0500 Subject: [PATCH 2/4] SDP: Rework merge_capabilities(). * Tried to give better variable names. * Made our SDP answer use the offer's RTP payload types as the SDP RFC says we SHOULD. * Updating the local topology now takes the stream format caps. We are likely preparing to send an offer. Change-Id: I34d3be8e3036402a8575ffcae3eebc5ce348d7c0 --- main/sdp_state.c | 176 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 120 insertions(+), 56 deletions(-) diff --git a/main/sdp_state.c b/main/sdp_state.c index 6ce06ee523..598a6102cd 100644 --- a/main/sdp_state.c +++ b/main/sdp_state.c @@ -522,6 +522,7 @@ const struct ast_sdp_options *ast_sdp_state_get_options( * * \param local Our local stream * \param remote A remote stream + * * \retval NULL An error occurred * \retval non-NULL The joint stream created */ @@ -542,10 +543,6 @@ static struct ast_stream *merge_streams(const struct ast_stream *local, return NULL; } - if (!local) { - return joint_stream; - } - joint_cap = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT); if (!joint_cap) { ast_stream_free(joint_stream); @@ -576,63 +573,100 @@ static struct ast_stream *merge_streams(const struct ast_stream *local, * \param local The local topology * \param media_type The type of stream we are looking for * \param[in,out] media_indices Keeps track of where to start searching in the topology - * \retval NULL No corresponding stream found - * \retval non-NULL The corresponding stream + * + * \retval -1 No corresponding stream found + * \retval index The corresponding stream index */ static int get_corresponding_index(const struct ast_stream_topology *local, enum ast_media_type media_type, int *media_indices) { int i; - int winner = -1; for (i = media_indices[media_type]; i < ast_stream_topology_get_count(local); ++i) { struct ast_stream *candidate; candidate = ast_stream_topology_get_stream(local, i); if (ast_stream_get_type(candidate) == media_type) { - winner = i; - break; + media_indices[media_type] = i + 1; + return i; } } - media_indices[media_type] = i + 1; - return winner; + /* No stream of the type left in the topology */ + media_indices[media_type] = i; + return -1; } +/*! + * XXX TODO The merge_capabilities() function needs to be split into + * merging for new local topologies and new remote topologies. Also + * the possibility of changing the stream types needs consideration. + * Audio to video may or may not need us to keep the same RTP instance + * because the stream position is still RTP. A new RTP instance would + * cause us to change ports. Audio to image is definitely going to + * happen for T.38. + * + * A new remote topology as an initial offer needs to dictate the + * number of streams and the order. As a sdp_state option we may + * allow creation of new active streams not defined by the current + * local topology. A subsequent remote offer can change the stream + * types and add streams. The sdp_state option could regulate + * creation of new active streams here as well. An answer cannot + * change stream types or the number of streams but can decline + * streams. Any attempt to do so should report an error and possibly + * disconnect the call. + * + * A local topology update needs to be merged differently. It cannot + * reduce the number of streams already defined without violating the + * SDP RFC. The local merge could take the new topology stream + * verbatim and add declined streams to fill out any shortfall with + * the exiting topology. This strategy is needed if we want to change + * an audio stream to an image stream for T.38 fax and vice versa. + * The local merge could take the new topology and map the streams to + * the existing local topology. The new topology stream format caps + * would be copied into the merged topology so we could change what + * codecs are negotiated. + */ /*! * \brief Merge existing stream capabilities and a new topology into joint capabilities. * - * This is a bit complicated. The idea is that we already have some capabilities set, and - * we've now been confronted with a new stream topology. We want to take what's been - * presented to us and merge those new capabilities with our own. + * \param sdp_state The state needing capabilities merged + * \param new_topology The new topology to base merged capabilities on + * \param is_local If new_topology is a local update. * - * For each of the new streams, we try to find a corresponding stream in our current - * capabilities. If we find one, then we get the compatible formats of the two streams - * and create a new stream with those formats set. We then will re-use the underlying - * media instance (such as an RTP instance) on this merged stream. + * \details + * This is a bit complicated. The idea is that we already have some + * capabilities set, and we've now been confronted with a new stream + * topology. We want to take what's been presented to us and merge + * those new capabilities with our own. * - * The create_new parameter determines whether we should attempt to create new media - * instances. - * If we do not find a corresponding stream, then we create a new one. If the - * create_new parameter is true, this created stream is made a clone of the new stream, - * and a media instance is created. If the create_new parameter is not true, then the - * created stream has no formats set and no media instance is created for it. + * For each of the new streams, we try to find a corresponding stream + * in our proposed capabilities. If we find one, then we get the + * compatible formats of the two streams and create a new stream with + * those formats set. We then will re-use the underlying media + * instance (such as an RTP instance) on this merged stream. + * + * The is_local parameter determines whether we should attempt to + * create new media instances. If we do not find a corresponding + * stream, then we create a new one. If the is_local parameter is + * true, this created stream is made a clone of the new stream, and a + * media instance is created. If the is_local parameter is not true, + * then the created stream has no formats set and no media instance is + * created for it. * - * \param current Current capabilities of the SDP state (may be NULL) - * \param new_topology The new topology to base merged capabilities on - * \param options The options set on the SDP state * \retval NULL An error occurred * \retval non-NULL The merged capabilities */ -static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_capabilities *current, - const struct ast_stream_topology *new_topology, const struct ast_sdp_options *options, int create_missing) +static struct sdp_state_capabilities *merge_capabilities(const struct ast_sdp_state *sdp_state, + const struct ast_stream_topology *new_topology, int is_local) { + const struct sdp_state_capabilities *local = sdp_state->proposed_capabilities; struct sdp_state_capabilities *joint_capabilities; - struct ast_stream_topology *topology; int media_indices[AST_MEDIA_TYPE_END] = {0}; int i; + static const char dummy_name[] = "dummy"; - ast_assert(current != NULL); + ast_assert(local != NULL); joint_capabilities = ast_calloc(1, sizeof(*joint_capabilities)); if (!joint_capabilities) { @@ -644,20 +678,18 @@ static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_ goto fail; } - if (AST_VECTOR_INIT(&joint_capabilities->streams, AST_VECTOR_SIZE(¤t->streams))) { + if (AST_VECTOR_INIT(&joint_capabilities->streams, AST_VECTOR_SIZE(&local->streams))) { goto fail; } - ast_sockaddr_copy(&joint_capabilities->connection_address, ¤t->connection_address); - topology = current->topology; + ast_sockaddr_copy(&joint_capabilities->connection_address, &local->connection_address); for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) { enum ast_media_type new_stream_type; struct ast_stream *new_stream; - struct ast_stream *current_stream; + struct ast_stream *local_stream; struct ast_stream *joint_stream; - struct sdp_state_stream *current_state_stream; struct sdp_state_stream *joint_state_stream; - int current_index; + int local_index; joint_state_stream = ast_calloc(1, sizeof(*joint_state_stream)); if (!joint_state_stream) { @@ -667,26 +699,57 @@ static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_ new_stream = ast_stream_topology_get_stream(new_topology, i); new_stream_type = ast_stream_get_type(new_stream); - current_index = get_corresponding_index(topology, new_stream_type, media_indices); - if (current_index >= 0) { - current_stream = ast_stream_topology_get_stream(topology, current_index); - joint_stream = merge_streams(current_stream, new_stream); + local_index = get_corresponding_index(local->topology, new_stream_type, media_indices); + if (0 <= local_index) { + local_stream = ast_stream_topology_get_stream(local->topology, local_index); + if (!strcmp(ast_stream_get_name(local_stream), dummy_name)) { + /* The local stream is a non-exixtent dummy stream. */ + local_stream = NULL; + } + } else { + local_stream = NULL; + } + if (local_stream) { + struct sdp_state_stream *local_state_stream; + struct ast_rtp_codecs *codecs; + + if (is_local) { + /* Replace the local stream with the new local stream. */ + joint_stream = ast_stream_clone(new_stream); + } else { + joint_stream = merge_streams(local_stream, new_stream); + } if (!joint_stream) { sdp_state_stream_free(joint_state_stream); goto fail; } - current_state_stream = AST_VECTOR_GET(¤t->streams, current_index); - joint_state_stream->type = current_state_stream->type; + local_state_stream = AST_VECTOR_GET(&local->streams, local_index); + joint_state_stream->type = local_state_stream->type; switch (joint_state_stream->type) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - joint_state_stream->instance = ao2_bump(current_state_stream->instance); + joint_state_stream->instance = ao2_bump(local_state_stream->instance); + if (is_local) { + break; + } + codecs = ast_stream_get_data(new_stream, AST_STREAM_DATA_RTP_CODECS); + ast_assert(codecs != NULL); + if (sdp_state->role == SDP_ROLE_ANSWERER) { + /* + * Setup rx payload type mapping to prefer the mapping + * from the peer that the RFC says we SHOULD use. + */ + ast_rtp_codecs_payloads_xover(codecs, codecs, NULL); + } + ast_rtp_codecs_payloads_copy(codecs, + ast_rtp_instance_get_codecs(joint_state_stream->instance), + joint_state_stream->instance); break; case AST_MEDIA_TYPE_IMAGE: - joint_state_stream->udptl = ao2_bump(current_state_stream->udptl); - joint_state_stream->t38_local_params = current_state_stream->t38_local_params; + joint_state_stream->udptl = ao2_bump(local_state_stream->udptl); + joint_state_stream->t38_local_params = local_state_stream->t38_local_params; break; case AST_MEDIA_TYPE_UNKNOWN: case AST_MEDIA_TYPE_TEXT: @@ -694,13 +757,14 @@ static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_ break; } - if (!ast_sockaddr_isnull(¤t_state_stream->connection_address)) { - ast_sockaddr_copy(&joint_state_stream->connection_address, ¤t_state_stream->connection_address); + if (!ast_sockaddr_isnull(&local_state_stream->connection_address)) { + ast_sockaddr_copy(&joint_state_stream->connection_address, + &local_state_stream->connection_address); } else { ast_sockaddr_setnull(&joint_state_stream->connection_address); } - joint_state_stream->locally_held = current_state_stream->locally_held; - } else if (create_missing) { + joint_state_stream->locally_held = local_state_stream->locally_held; + } else if (is_local) { /* We don't have a stream state that corresponds to the stream in the new topology, so * create a stream state as appropriate. */ @@ -713,7 +777,8 @@ static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_ switch (new_stream_type) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - joint_state_stream->instance = create_rtp(options, new_stream_type); + joint_state_stream->instance = create_rtp(sdp_state->options, + new_stream_type); if (!joint_state_stream->instance) { ast_stream_free(joint_stream); sdp_state_stream_free(joint_state_stream); @@ -721,7 +786,7 @@ static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_ } break; case AST_MEDIA_TYPE_IMAGE: - joint_state_stream->udptl = create_udptl(options); + joint_state_stream->udptl = create_udptl(sdp_state->options); if (!joint_state_stream->udptl) { ast_stream_free(joint_stream); sdp_state_stream_free(joint_state_stream); @@ -740,7 +805,7 @@ static struct sdp_state_capabilities *merge_capabilities(const struct sdp_state_ * dummy stream to go in its place so that the resulting SDP created will contain * the stream but will have no port or codecs set */ - joint_stream = ast_stream_alloc("dummy", new_stream_type); + joint_stream = ast_stream_alloc(dummy_name, new_stream_type); if (!joint_stream) { sdp_state_stream_free(joint_state_stream); goto fail; @@ -1013,8 +1078,7 @@ static int merge_sdps(struct ast_sdp_state *sdp_state, const struct ast_sdp *rem return -1; } - joint_capabilities = merge_capabilities(sdp_state->proposed_capabilities, - remote_capabilities, sdp_state->options, 0); + joint_capabilities = merge_capabilities(sdp_state, remote_capabilities, 0); ast_stream_topology_free(remote_capabilities); if (!joint_capabilities) { return -1; @@ -1127,7 +1191,7 @@ int ast_sdp_state_update_local_topology(struct ast_sdp_state *sdp_state, struct ast_assert(sdp_state != NULL); ast_assert(streams != NULL); - capabilities = merge_capabilities(sdp_state->proposed_capabilities, streams, sdp_state->options, 1); + capabilities = merge_capabilities(sdp_state, streams, 1); if (!capabilities) { return -1; } From 367042bd3e58233bdb23542c1f19ec4907f07ff2 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Fri, 5 May 2017 14:49:30 -0500 Subject: [PATCH 3/4] SDP: Explicitly stop a RTP instance before destoying it. * Made sdp_add_m_from_rtp_stream() and sdp_add_m_from_udptl_stream() handle generating disabled/declined streams. * Added /main/sdp/sdp_merge_asymmetric unit test. It currently does not check the offerer side negotiated SDP because that isn't the purpose of this patch and there is much to be done to handle declined/dummy streams. * Added T.38 image streams to the /main/sdp/sdp_merge_symmetric and /main/sdp/sdp_merge_crisscross unit tests. Change-Id: Ib4dcb3ca4f9a9133b376f4e3302f9a1f963f2b31 --- include/asterisk/stream.h | 2 +- main/sdp_state.c | 439 +++++++++++++++++++++++--------------- tests/test_sdp.c | 167 ++++++++++++++- 3 files changed, 429 insertions(+), 179 deletions(-) diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h index 526aee17d2..b453ab9c32 100644 --- a/include/asterisk/stream.h +++ b/include/asterisk/stream.h @@ -56,7 +56,7 @@ typedef void (*ast_stream_data_free_fn)(void *); */ enum ast_stream_state { /*! - * \brief Set when the stream has been removed + * \brief Set when the stream has been removed/declined */ AST_STREAM_STATE_REMOVED = 0, /*! diff --git a/main/sdp_state.c b/main/sdp_state.c index 598a6102cd..9b116ca549 100644 --- a/main/sdp_state.c +++ b/main/sdp_state.c @@ -64,6 +64,11 @@ enum ast_sdp_role { typedef int (*state_fn)(struct ast_sdp_state *state); +struct sdp_state_rtp { + /*! The underlying RTP instance */ + struct ast_rtp_instance *instance; +}; + struct sdp_state_udptl { /*! The underlying UDPTL instance */ struct ast_udptl *instance; @@ -74,7 +79,7 @@ struct sdp_state_stream { enum ast_media_type type; union { /*! The underlying RTP instance */ - struct ast_rtp_instance *instance; + struct sdp_state_rtp *rtp; /*! The underlying UDPTL instance */ struct sdp_state_udptl *udptl; }; @@ -86,6 +91,16 @@ struct sdp_state_stream { struct ast_control_t38_parameters t38_local_params; }; +static void sdp_state_rtp_destroy(void *obj) +{ + struct sdp_state_rtp *rtp = obj; + + if (rtp->instance) { + ast_rtp_instance_stop(rtp->instance); + ast_rtp_instance_destroy(rtp->instance); + } +} + static void sdp_state_udptl_destroy(void *obj) { struct sdp_state_udptl *udptl = obj; @@ -100,9 +115,7 @@ static void sdp_state_stream_free(struct sdp_state_stream *state_stream) switch (state_stream->type) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - if (state_stream->instance) { - ast_rtp_instance_destroy(state_stream->instance); - } + ao2_cleanup(state_stream->rtp); break; case AST_MEDIA_TYPE_IMAGE: ao2_cleanup(state_stream->udptl); @@ -145,10 +158,10 @@ static void sdp_state_capabilities_free(struct sdp_state_capabilities *capabilit static struct ast_sched_context *sched; /*! \brief Internal function which creates an RTP instance */ -static struct ast_rtp_instance *create_rtp(const struct ast_sdp_options *options, +static struct sdp_state_rtp *create_rtp(const struct ast_sdp_options *options, enum ast_media_type media_type) { - struct ast_rtp_instance *rtp; + struct sdp_state_rtp *rtp; struct ast_rtp_engine_ice *ice; static struct ast_sockaddr address_rtp; struct ast_sockaddr *media_address = &address_rtp; @@ -167,38 +180,57 @@ static struct ast_rtp_instance *create_rtp(const struct ast_sdp_options *options } } - rtp = ast_rtp_instance_new(options->rtp_engine, sched, media_address, NULL); + rtp = ao2_alloc_options(sizeof(*rtp), sdp_state_rtp_destroy, AO2_ALLOC_OPT_LOCK_NOLOCK); if (!rtp) { + return NULL; + } + + rtp->instance = ast_rtp_instance_new(options->rtp_engine, sched, media_address, NULL); + if (!rtp->instance) { ast_log(LOG_ERROR, "Unable to create RTP instance using RTP engine '%s'\n", options->rtp_engine); + ao2_ref(rtp, -1); return NULL; } - ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_RTCP, AST_RTP_INSTANCE_RTCP_STANDARD); - ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_NAT, options->rtp_symmetric); + ast_rtp_instance_set_prop(rtp->instance, AST_RTP_PROPERTY_RTCP, + AST_RTP_INSTANCE_RTCP_STANDARD); + ast_rtp_instance_set_prop(rtp->instance, AST_RTP_PROPERTY_NAT, + options->rtp_symmetric); - if (options->ice == AST_SDP_ICE_DISABLED && (ice = ast_rtp_instance_get_ice(rtp))) { - ice->stop(rtp); + if (options->ice == AST_SDP_ICE_DISABLED + && (ice = ast_rtp_instance_get_ice(rtp->instance))) { + ice->stop(rtp->instance); } if (options->dtmf == AST_SDP_DTMF_RFC_4733 || options->dtmf == AST_SDP_DTMF_AUTO) { - ast_rtp_instance_dtmf_mode_set(rtp, AST_RTP_DTMF_MODE_RFC2833); - ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_DTMF, 1); + ast_rtp_instance_dtmf_mode_set(rtp->instance, AST_RTP_DTMF_MODE_RFC2833); + ast_rtp_instance_set_prop(rtp->instance, AST_RTP_PROPERTY_DTMF, 1); } else if (options->dtmf == AST_SDP_DTMF_INBAND) { - ast_rtp_instance_dtmf_mode_set(rtp, AST_RTP_DTMF_MODE_INBAND); + ast_rtp_instance_dtmf_mode_set(rtp->instance, AST_RTP_DTMF_MODE_INBAND); } - if (media_type == AST_MEDIA_TYPE_AUDIO && - (options->tos_audio || options->cos_audio)) { - ast_rtp_instance_set_qos(rtp, options->tos_audio, - options->cos_audio, "SIP RTP Audio"); - } else if (media_type == AST_MEDIA_TYPE_VIDEO && - (options->tos_video || options->cos_video)) { - ast_rtp_instance_set_qos(rtp, options->tos_video, - options->cos_video, "SIP RTP Video"); + switch (media_type) { + case AST_MEDIA_TYPE_AUDIO: + if (options->tos_audio || options->cos_audio) { + ast_rtp_instance_set_qos(rtp->instance, options->tos_audio, + options->cos_audio, "SIP RTP Audio"); + } + break; + case AST_MEDIA_TYPE_VIDEO: + if (options->tos_video || options->cos_video) { + ast_rtp_instance_set_qos(rtp->instance, options->tos_video, + options->cos_video, "SIP RTP Video"); + } + break; + case AST_MEDIA_TYPE_IMAGE: + case AST_MEDIA_TYPE_TEXT: + case AST_MEDIA_TYPE_UNKNOWN: + case AST_MEDIA_TYPE_END: + break; } - ast_rtp_instance_set_last_rx(rtp, time(NULL)); + ast_rtp_instance_set_last_rx(rtp->instance, time(NULL)); return rtp; } @@ -278,8 +310,8 @@ static struct sdp_state_capabilities *sdp_initialize_state_capabilities(const st switch (state_stream->type) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - state_stream->instance = create_rtp(options, state_stream->type); - if (!state_stream->instance) { + state_stream->rtp = create_rtp(options, state_stream->type); + if (!state_stream->rtp) { sdp_state_stream_free(state_stream); sdp_state_capabilities_free(capabilities); return NULL; @@ -413,11 +445,11 @@ struct ast_rtp_instance *ast_sdp_state_get_rtp_instance( sdp_state->proposed_capabilities->topology, stream_index)) == AST_MEDIA_TYPE_VIDEO); stream_state = sdp_state_get_stream(sdp_state, stream_index); - if (!stream_state) { + if (!stream_state || !stream_state->rtp) { return NULL; } - return stream_state->instance; + return stream_state->rtp->instance; } struct ast_udptl *ast_sdp_state_get_udptl_instance( @@ -467,7 +499,7 @@ int ast_sdp_state_get_stream_connection_address(const struct ast_sdp_state *sdp_ stream_index))) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - ast_rtp_instance_get_local_address(stream_state->instance, address); + ast_rtp_instance_get_local_address(stream_state->rtp->instance, address); break; case AST_MEDIA_TYPE_IMAGE: ast_udptl_get_us(stream_state->udptl->instance, address); @@ -730,7 +762,7 @@ static struct sdp_state_capabilities *merge_capabilities(const struct ast_sdp_st switch (joint_state_stream->type) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - joint_state_stream->instance = ao2_bump(local_state_stream->instance); + joint_state_stream->rtp = ao2_bump(local_state_stream->rtp); if (is_local) { break; } @@ -744,8 +776,8 @@ static struct sdp_state_capabilities *merge_capabilities(const struct ast_sdp_st ast_rtp_codecs_payloads_xover(codecs, codecs, NULL); } ast_rtp_codecs_payloads_copy(codecs, - ast_rtp_instance_get_codecs(joint_state_stream->instance), - joint_state_stream->instance); + ast_rtp_instance_get_codecs(joint_state_stream->rtp->instance), + joint_state_stream->rtp->instance); break; case AST_MEDIA_TYPE_IMAGE: joint_state_stream->udptl = ao2_bump(local_state_stream->udptl); @@ -777,9 +809,9 @@ static struct sdp_state_capabilities *merge_capabilities(const struct ast_sdp_st switch (new_stream_type) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - joint_state_stream->instance = create_rtp(sdp_state->options, + joint_state_stream->rtp = create_rtp(sdp_state->options, new_stream_type); - if (!joint_state_stream->instance) { + if (!joint_state_stream->rtp) { ast_stream_free(joint_stream); sdp_state_stream_free(joint_state_stream); goto fail; @@ -954,24 +986,33 @@ static void update_ice(const struct ast_sdp_state *state, struct ast_rtp_instanc * sides. * * \param state The SDP state in which SDPs have been negotiated - * \param rtp The RTP instance that is being updated + * \param rtp The RTP wrapper that is being updated * \param options Our locally-supported SDP options * \param remote_sdp The SDP we most recently received * \param remote_m_line The remote SDP stream that corresponds to the RTP instance we are modifying */ -static void update_rtp_after_merge(const struct ast_sdp_state *state, struct ast_rtp_instance *rtp, +static void update_rtp_after_merge(const struct ast_sdp_state *state, + struct sdp_state_rtp *rtp, const struct ast_sdp_options *options, const struct ast_sdp *remote_sdp, const struct ast_sdp_m_line *remote_m_line) { - if (ast_sdp_options_get_rtcp_mux(options) && ast_sdp_m_find_attribute(remote_m_line, "rtcp-mux", -1)) { - ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_RTCP, AST_RTP_INSTANCE_RTCP_MUX); + if (!rtp) { + /* This is a dummy stream */ + return; + } + + if (ast_sdp_options_get_rtcp_mux(options) + && ast_sdp_m_find_attribute(remote_m_line, "rtcp-mux", -1)) { + ast_rtp_instance_set_prop(rtp->instance, AST_RTP_PROPERTY_RTCP, + AST_RTP_INSTANCE_RTCP_MUX); } else { - ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_RTCP, AST_RTP_INSTANCE_RTCP_STANDARD); + ast_rtp_instance_set_prop(rtp->instance, AST_RTP_PROPERTY_RTCP, + AST_RTP_INSTANCE_RTCP_STANDARD); } if (ast_sdp_options_get_ice(options) == AST_SDP_ICE_ENABLED_STANDARD) { - update_ice(state, rtp, options, remote_sdp, remote_m_line); + update_ice(state, rtp->instance, options, remote_sdp, remote_m_line); } } @@ -1000,6 +1041,11 @@ static void update_udptl_after_merge(const struct ast_sdp_state *state, struct s unsigned int fax_max_datagram; struct ast_sockaddr *addrs; + if (!udptl) { + /* This is a dummy stream */ + return; + } + a_line = ast_sdp_m_find_attribute(remote_m_line, "t38faxmaxdatagram", -1); if (!a_line) { a_line = ast_sdp_m_find_attribute(remote_m_line, "t38maxdatagram", -1); @@ -1103,7 +1149,7 @@ static int merge_sdps(struct ast_sdp_state *sdp_state, const struct ast_sdp *rem switch (ast_stream_get_type(ast_stream_topology_get_stream(joint_capabilities->topology, i))) { case AST_MEDIA_TYPE_AUDIO: case AST_MEDIA_TYPE_VIDEO: - update_rtp_after_merge(sdp_state, state_stream->instance, sdp_state->options, + update_rtp_after_merge(sdp_state, state_stream->rtp, sdp_state->options, remote_sdp, ast_sdp_get_m(remote_sdp, i)); break; case AST_MEDIA_TYPE_IMAGE: @@ -1312,123 +1358,163 @@ static int sdp_add_m_from_rtp_stream(struct ast_sdp *sdp, const struct ast_sdp_s struct ast_format_cap *caps; int i; int rtp_code; + int rtp_port; int min_packet_size = 0; int max_packet_size = 0; enum ast_media_type media_type; char tmp[64]; - struct ast_sockaddr address_rtp; + struct sdp_state_stream *stream_state; struct ast_rtp_instance *rtp; struct ast_sdp_a_line *a_line; stream = ast_stream_topology_get_stream(capabilities->topology, stream_index); - rtp = AST_VECTOR_GET(&capabilities->streams, stream_index)->instance; ast_assert(sdp && options && stream); + caps = ast_stream_get_formats(stream); + + stream_state = AST_VECTOR_GET(&capabilities->streams, stream_index); + if (stream_state->rtp && caps && ast_format_cap_count(caps)) { + rtp = stream_state->rtp->instance; + } else { + /* This is a disabled stream */ + rtp = NULL; + } + if (rtp) { + struct ast_sockaddr address_rtp; + if (ast_sdp_state_get_stream_connection_address(sdp_state, 0, &address_rtp)) { return -1; } + rtp_port = ast_sockaddr_port(&address_rtp); } else { - ast_sockaddr_setnull(&address_rtp); + rtp_port = 0; } m_line = ast_sdp_m_alloc( ast_codec_media_type2str(ast_stream_get_type(stream)), - ast_sockaddr_port(&address_rtp), 1, + rtp_port, 1, options->encryption != AST_SDP_ENCRYPTION_DISABLED ? "RTP/SAVP" : "RTP/AVP", NULL); if (!m_line) { return -1; } - caps = ast_stream_get_formats(stream); + if (rtp_port) { + /* Stream is not declined/disabled */ + for (i = 0; i < ast_format_cap_count(caps); i++) { + struct ast_format *format = ast_format_cap_get_format(caps, i); - for (i = 0; i < ast_format_cap_count(caps); i++) { - struct ast_format *format = ast_format_cap_get_format(caps, i); + rtp_code = ast_rtp_codecs_payload_code(ast_rtp_instance_get_codecs(rtp), 1, + format, 0); + if (rtp_code == -1) { + ast_log(LOG_WARNING,"Unable to get rtp codec payload code for %s\n", + ast_format_get_name(format)); + ao2_ref(format, -1); + continue; + } - if ((rtp_code = ast_rtp_codecs_payload_code(ast_rtp_instance_get_codecs(rtp), 1, format, 0)) == -1) { - ast_log(LOG_WARNING,"Unable to get rtp codec payload code for %s\n", ast_format_get_name(format)); - ao2_ref(format, -1); - continue; - } + if (ast_sdp_m_add_format(m_line, options, rtp_code, 1, format, 0)) { + ast_sdp_m_free(m_line); + ao2_ref(format, -1); + return -1; + } + + if (ast_format_get_maximum_ms(format) + && ((ast_format_get_maximum_ms(format) < max_packet_size) + || !max_packet_size)) { + max_packet_size = ast_format_get_maximum_ms(format); + } - if (ast_sdp_m_add_format(m_line, options, rtp_code, 1, format, 0)) { - ast_sdp_m_free(m_line); ao2_ref(format, -1); - return -1; } - if (ast_format_get_maximum_ms(format) && - ((ast_format_get_maximum_ms(format) < max_packet_size) || !max_packet_size)) { - max_packet_size = ast_format_get_maximum_ms(format); + media_type = ast_stream_get_type(stream); + if (media_type != AST_MEDIA_TYPE_VIDEO + && (options->dtmf == AST_SDP_DTMF_RFC_4733 || options->dtmf == AST_SDP_DTMF_AUTO)) { + i = AST_RTP_DTMF; + rtp_code = ast_rtp_codecs_payload_code( + ast_rtp_instance_get_codecs(rtp), 0, NULL, i); + if (-1 < rtp_code) { + if (ast_sdp_m_add_format(m_line, options, rtp_code, 0, NULL, i)) { + ast_sdp_m_free(m_line); + return -1; + } + + snprintf(tmp, sizeof(tmp), "%d 0-16", rtp_code); + a_line = ast_sdp_a_alloc("fmtp", tmp); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } + } } - ao2_ref(format, -1); - } + /* If ptime is set add it as an attribute */ + min_packet_size = ast_rtp_codecs_get_framing(ast_rtp_instance_get_codecs(rtp)); + if (!min_packet_size) { + min_packet_size = ast_format_cap_get_framing(caps); + } + if (min_packet_size) { + snprintf(tmp, sizeof(tmp), "%d", min_packet_size); - media_type = ast_stream_get_type(stream); - if (rtp && media_type != AST_MEDIA_TYPE_VIDEO - && (options->dtmf == AST_SDP_DTMF_RFC_4733 || options->dtmf == AST_SDP_DTMF_AUTO)) { - i = AST_RTP_DTMF; - rtp_code = ast_rtp_codecs_payload_code( - ast_rtp_instance_get_codecs(rtp), 0, NULL, i); - if (-1 < rtp_code) { - if (ast_sdp_m_add_format(m_line, options, rtp_code, 0, NULL, i)) { + a_line = ast_sdp_a_alloc("ptime", tmp); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); ast_sdp_m_free(m_line); return -1; } + } - snprintf(tmp, sizeof(tmp), "%d 0-16", rtp_code); - a_line = ast_sdp_a_alloc("fmtp", tmp); + if (max_packet_size) { + snprintf(tmp, sizeof(tmp), "%d", max_packet_size); + a_line = ast_sdp_a_alloc("maxptime", tmp); if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { ast_sdp_a_free(a_line); ast_sdp_m_free(m_line); return -1; } } - } - if (ast_sdp_m_get_a_count(m_line) == 0) { - ast_sdp_m_free(m_line); - return 0; - } - - /* If ptime is set add it as an attribute */ - min_packet_size = ast_rtp_codecs_get_framing(ast_rtp_instance_get_codecs(rtp)); - if (!min_packet_size) { - min_packet_size = ast_format_cap_get_framing(caps); - } - if (min_packet_size) { - snprintf(tmp, sizeof(tmp), "%d", min_packet_size); - - a_line = ast_sdp_a_alloc("ptime", tmp); + a_line = ast_sdp_a_alloc(ast_sdp_state_get_locally_held(sdp_state, stream_index) + ? "sendonly" : "sendrecv", ""); if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { ast_sdp_a_free(a_line); ast_sdp_m_free(m_line); return -1; } - } - if (max_packet_size) { - snprintf(tmp, sizeof(tmp), "%d", max_packet_size); - a_line = ast_sdp_a_alloc("maxptime", tmp); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); + add_ssrc_attributes(m_line, options, rtp); + } else { + /* Declined/disabled stream */ + struct ast_sdp_payload *payload; + const char *fmt; + + /* + * Add a static payload type placeholder to the declined/disabled stream. + * + * XXX We should use the default payload type in the received offer but + * we don't have that available. + */ + switch (ast_stream_get_type(stream)) { + default: + case AST_MEDIA_TYPE_AUDIO: + fmt = "0"; /* ulaw */ + break; + case AST_MEDIA_TYPE_VIDEO: + fmt = "31"; /* H.261 */ + break; + } + payload = ast_sdp_payload_alloc(fmt); + if (!payload || ast_sdp_m_add_payload(m_line, payload)) { + ast_sdp_payload_free(payload); ast_sdp_m_free(m_line); return -1; } } - a_line = ast_sdp_a_alloc(ast_sdp_state_get_locally_held(sdp_state, stream_index) ? "sendonly" : "sendrecv", ""); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; - } - - add_ssrc_attributes(m_line, options, rtp); - if (ast_sdp_add_m(sdp, m_line)) { ast_sdp_m_free(m_line); return -1; @@ -1465,27 +1551,37 @@ static int sdp_add_m_from_udptl_stream(struct ast_sdp *sdp, const struct ast_sdp struct ast_sdp_m_line *m_line; struct ast_sdp_payload *payload; char tmp[64]; - struct ast_sockaddr address_udptl; struct sdp_state_udptl *udptl; struct ast_sdp_a_line *a_line; struct sdp_state_stream *stream_state; + int udptl_port; stream = ast_stream_topology_get_stream(capabilities->topology, stream_index); - udptl = AST_VECTOR_GET(&capabilities->streams, stream_index)->udptl; ast_assert(sdp && options && stream); + stream_state = AST_VECTOR_GET(&capabilities->streams, stream_index); + if (stream_state->udptl) { + udptl = stream_state->udptl; + } else { + /* This is a disabled stream */ + udptl = NULL; + } + if (udptl) { + struct ast_sockaddr address_udptl; + if (ast_sdp_state_get_stream_connection_address(sdp_state, 0, &address_udptl)) { return -1; } + udptl_port = ast_sockaddr_port(&address_udptl); } else { - ast_sockaddr_setnull(&address_udptl); + udptl_port = 0; } m_line = ast_sdp_m_alloc( ast_codec_media_type2str(ast_stream_get_type(stream)), - ast_sockaddr_port(&address_udptl), 1, "udptl", NULL); + udptl_port, 1, "udptl", NULL); if (!m_line) { return -1; } @@ -1497,97 +1593,100 @@ static int sdp_add_m_from_udptl_stream(struct ast_sdp *sdp, const struct ast_sdp return -1; } - stream_state = sdp_state_get_stream(sdp_state, stream_index); - - snprintf(tmp, sizeof(tmp), "%u", stream_state->t38_local_params.version); - a_line = ast_sdp_a_alloc("T38FaxVersion", tmp); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; - } - - snprintf(tmp, sizeof(tmp), "%u", t38_get_rate(stream_state->t38_local_params.rate)); - a_line = ast_sdp_a_alloc("T38FaxMaxBitRate", tmp); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; - } + if (udptl_port) { + /* Stream is not declined/disabled */ + stream_state = sdp_state_get_stream(sdp_state, stream_index); - if (stream_state->t38_local_params.fill_bit_removal) { - a_line = ast_sdp_a_alloc("T38FaxFillBitRemoval", ""); + snprintf(tmp, sizeof(tmp), "%u", stream_state->t38_local_params.version); + a_line = ast_sdp_a_alloc("T38FaxVersion", tmp); if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { ast_sdp_a_free(a_line); ast_sdp_m_free(m_line); return -1; } - } - if (stream_state->t38_local_params.transcoding_mmr) { - a_line = ast_sdp_a_alloc("T38FaxTranscodingMMR", ""); + snprintf(tmp, sizeof(tmp), "%u", t38_get_rate(stream_state->t38_local_params.rate)); + a_line = ast_sdp_a_alloc("T38FaxMaxBitRate", tmp); if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { ast_sdp_a_free(a_line); ast_sdp_m_free(m_line); return -1; } - } - if (stream_state->t38_local_params.transcoding_jbig) { - a_line = ast_sdp_a_alloc("T38FaxTranscodingJBIG", ""); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; + if (stream_state->t38_local_params.fill_bit_removal) { + a_line = ast_sdp_a_alloc("T38FaxFillBitRemoval", ""); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } } - } - switch (stream_state->t38_local_params.rate_management) { - case AST_T38_RATE_MANAGEMENT_TRANSFERRED_TCF: - a_line = ast_sdp_a_alloc("T38FaxRateManagement", "transferredTCF"); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; + if (stream_state->t38_local_params.transcoding_mmr) { + a_line = ast_sdp_a_alloc("T38FaxTranscodingMMR", ""); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } } - break; - case AST_T38_RATE_MANAGEMENT_LOCAL_TCF: - a_line = ast_sdp_a_alloc("T38FaxRateManagement", "localTCF"); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; + + if (stream_state->t38_local_params.transcoding_jbig) { + a_line = ast_sdp_a_alloc("T38FaxTranscodingJBIG", ""); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } } - break; - } - snprintf(tmp, sizeof(tmp), "%u", ast_udptl_get_local_max_datagram(udptl->instance)); - a_line = ast_sdp_a_alloc("T38FaxMaxDatagram", tmp); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; - } + switch (stream_state->t38_local_params.rate_management) { + case AST_T38_RATE_MANAGEMENT_TRANSFERRED_TCF: + a_line = ast_sdp_a_alloc("T38FaxRateManagement", "transferredTCF"); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } + break; + case AST_T38_RATE_MANAGEMENT_LOCAL_TCF: + a_line = ast_sdp_a_alloc("T38FaxRateManagement", "localTCF"); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } + break; + } - switch (ast_udptl_get_error_correction_scheme(udptl->instance)) { - case UDPTL_ERROR_CORRECTION_NONE: - break; - case UDPTL_ERROR_CORRECTION_FEC: - a_line = ast_sdp_a_alloc("T38FaxUdpEC", "t38UDPFEC"); + snprintf(tmp, sizeof(tmp), "%u", ast_udptl_get_local_max_datagram(udptl->instance)); + a_line = ast_sdp_a_alloc("T38FaxMaxDatagram", tmp); if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { ast_sdp_a_free(a_line); ast_sdp_m_free(m_line); return -1; } - break; - case UDPTL_ERROR_CORRECTION_REDUNDANCY: - a_line = ast_sdp_a_alloc("T38FaxUdpEC", "t38UDPRedundancy"); - if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { - ast_sdp_a_free(a_line); - ast_sdp_m_free(m_line); - return -1; + + switch (ast_udptl_get_error_correction_scheme(udptl->instance)) { + case UDPTL_ERROR_CORRECTION_NONE: + break; + case UDPTL_ERROR_CORRECTION_FEC: + a_line = ast_sdp_a_alloc("T38FaxUdpEC", "t38UDPFEC"); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } + break; + case UDPTL_ERROR_CORRECTION_REDUNDANCY: + a_line = ast_sdp_a_alloc("T38FaxUdpEC", "t38UDPRedundancy"); + if (!a_line || ast_sdp_m_add_a(m_line, a_line)) { + ast_sdp_a_free(a_line); + ast_sdp_m_free(m_line); + return -1; + } + break; } - break; } if (ast_sdp_add_m(sdp, m_line)) { diff --git a/tests/test_sdp.c b/tests/test_sdp.c index 408888fc00..79b9e7b2c9 100644 --- a/tests/test_sdp.c +++ b/tests/test_sdp.c @@ -630,7 +630,7 @@ AST_TEST_DEFINE(sdp_to_topology) } if (ast_stream_topology_get_count(topology) != 3) { - ast_test_status_update(test, "Unexpected topology count '%d'. Expecting 2\n", + ast_test_status_update(test, "Unexpected topology count '%d'. Expecting 3\n", ast_stream_topology_get_count(topology)); res = AST_TEST_FAIL; goto end; @@ -669,11 +669,9 @@ static int validate_merged_sdp(struct ast_test *test, const struct ast_sdp *sdp) } m_line = ast_sdp_get_m(sdp, 0); - if (validate_m_line(test, m_line, "audio", 1)) { return -1; } - if (validate_rtpmap(test, m_line, "PCMU")) { return -1; } @@ -682,29 +680,29 @@ static int validate_merged_sdp(struct ast_test *test, const struct ast_sdp *sdp) if (!validate_rtpmap(test, m_line, "PCMA")) { return -1; } - if (!validate_rtpmap(test, m_line, "G722")) { return -1; } - if (!validate_rtpmap(test, m_line, "opus")) { return -1; } m_line = ast_sdp_get_m(sdp, 1); - if (validate_m_line(test, m_line, "video", 1)) { return -1; } - if (validate_rtpmap(test, m_line, "VP8")) { return -1; } - if (!validate_rtpmap(test, m_line, "H264")) { return -1; } + m_line = ast_sdp_get_m(sdp, 2); + if (validate_m_line(test, m_line, "image", 1)) { + return -1; + } + return 0; } @@ -719,10 +717,12 @@ AST_TEST_DEFINE(sdp_merge_symmetric) static const struct sdp_format offerer_formats[] = { { AST_MEDIA_TYPE_AUDIO, "ulaw,alaw,g722,opus" }, { AST_MEDIA_TYPE_VIDEO, "h264,vp8" }, + { AST_MEDIA_TYPE_IMAGE, "t38" }, }; static const struct sdp_format answerer_formats[] = { { AST_MEDIA_TYPE_AUDIO, "ulaw" }, { AST_MEDIA_TYPE_VIDEO, "vp8" }, + { AST_MEDIA_TYPE_IMAGE, "t38" }, }; switch(cmd) { @@ -795,8 +795,10 @@ AST_TEST_DEFINE(sdp_merge_crisscross) static const struct sdp_format offerer_formats[] = { { AST_MEDIA_TYPE_AUDIO, "ulaw,alaw,g722,opus" }, { AST_MEDIA_TYPE_VIDEO, "h264,vp8" }, + { AST_MEDIA_TYPE_IMAGE, "t38" }, }; static const struct sdp_format answerer_formats[] = { + { AST_MEDIA_TYPE_IMAGE, "t38" }, { AST_MEDIA_TYPE_VIDEO, "vp8" }, { AST_MEDIA_TYPE_AUDIO, "ulaw" }, }; @@ -862,6 +864,153 @@ end: return res; } +static int validate_merged_sdp_asymmetric(struct ast_test *test, const struct ast_sdp *sdp, int is_offer) +{ + struct ast_sdp_m_line *m_line; + const char *side = is_offer ? "Offer side" : "Answer side"; + + if (!sdp) { + ast_test_status_update(test, "%s does not have a SDP\n", side); + return -1; + } + + /* Stream 0 */ + m_line = ast_sdp_get_m(sdp, 0); + if (validate_m_line(test, m_line, "audio", 1)) { + return -1; + } + if (!m_line->port) { + ast_test_status_update(test, "%s stream %d does%s have a port\n", side, 0, "n't"); + return -1; + } + if (validate_rtpmap(test, m_line, "PCMU")) { + return -1; + } + + /* The other audio formats should *NOT* be present */ + if (!validate_rtpmap(test, m_line, "PCMA")) { + return -1; + } + if (!validate_rtpmap(test, m_line, "G722")) { + return -1; + } + if (!validate_rtpmap(test, m_line, "opus")) { + return -1; + } + + /* The remaining streams should be declined */ + + /* Stream 1 */ + m_line = ast_sdp_get_m(sdp, 1); + if (validate_m_line(test, m_line, "audio", 1)) { + return -1; + } + if (m_line->port) { + ast_test_status_update(test, "%s stream %d does%s have a port\n", side, 1, ""); + return -1; + } + + /* Stream 2 */ + m_line = ast_sdp_get_m(sdp, 2); + if (validate_m_line(test, m_line, "video", 1)) { + return -1; + } + if (m_line->port) { + ast_test_status_update(test, "%s stream %d does%s have a port\n", side, 2, ""); + return -1; + } + + /* Stream 3 */ + m_line = ast_sdp_get_m(sdp, 3); + if (validate_m_line(test, m_line, "image", 1)) { + return -1; + } + if (m_line->port) { + ast_test_status_update(test, "%s stream %d does%s have a port\n", side, 3, ""); + return -1; + } + + return 0; +} + +AST_TEST_DEFINE(sdp_merge_asymmetric) +{ + enum ast_test_result_state res = AST_TEST_PASS; + struct ast_sdp_state *sdp_state_offerer = NULL; + struct ast_sdp_state *sdp_state_answerer = NULL; + const struct ast_sdp *offerer_sdp; + const struct ast_sdp *answerer_sdp; + + static const struct sdp_format offerer_formats[] = { + { AST_MEDIA_TYPE_AUDIO, "ulaw,alaw,g722,opus" }, + { AST_MEDIA_TYPE_AUDIO, "ulaw" }, + { AST_MEDIA_TYPE_VIDEO, "h261" }, + { AST_MEDIA_TYPE_IMAGE, "t38" }, + }; + static const struct sdp_format answerer_formats[] = { + { AST_MEDIA_TYPE_AUDIO, "ulaw" }, + }; + + switch(cmd) { + case TEST_INIT: + info->name = "sdp_merge_asymmetric"; + info->category = "/main/sdp/"; + info->summary = "Merge two SDPs with an asymmetric number of streams"; + info->description = + "SDP 1 offers a four stream topology: Audio,Audio,Video,T.38\n" + "SDP 2 only has a single audio stream topology\n" + "We ensure that both local SDPs have the expected stream types and\n" + "the expected declined streams"; + return AST_TEST_NOT_RUN; + case TEST_EXECUTE: + break; + } + + sdp_state_offerer = build_sdp_state(ARRAY_LEN(offerer_formats), offerer_formats, NULL); + if (!sdp_state_offerer) { + res = AST_TEST_FAIL; + goto end; + } + + sdp_state_answerer = build_sdp_state(ARRAY_LEN(answerer_formats), answerer_formats, NULL); + if (!sdp_state_answerer) { + res = AST_TEST_FAIL; + goto end; + } + + offerer_sdp = ast_sdp_state_get_local_sdp(sdp_state_offerer); + if (!offerer_sdp) { + res = AST_TEST_FAIL; + goto end; + } + + ast_sdp_state_set_remote_sdp(sdp_state_answerer, offerer_sdp); + answerer_sdp = ast_sdp_state_get_local_sdp(sdp_state_answerer); + if (!answerer_sdp) { + res = AST_TEST_FAIL; + goto end; + } + + ast_sdp_state_set_remote_sdp(sdp_state_offerer, answerer_sdp); + +#if defined(XXX_TODO_NEED_TO_HANDLE_DECLINED_STREAMS_ON_OFFER_SIDE) + /* Get the offerer SDP again because it's now going to be the joint SDP */ + offerer_sdp = ast_sdp_state_get_local_sdp(sdp_state_offerer); + if (validate_merged_sdp_asymmetric(test, offerer_sdp, 1)) { + res = AST_TEST_FAIL; + } +#endif + if (validate_merged_sdp_asymmetric(test, answerer_sdp, 0)) { + res = AST_TEST_FAIL; + } + +end: + ast_sdp_state_free(sdp_state_offerer); + ast_sdp_state_free(sdp_state_answerer); + + return res; +} + static int validate_ssrc(struct ast_test *test, struct ast_sdp_m_line *m_line, struct ast_rtp_instance *rtp) { @@ -976,6 +1125,7 @@ static int unload_module(void) AST_TEST_UNREGISTER(sdp_to_topology); AST_TEST_UNREGISTER(sdp_merge_symmetric); AST_TEST_UNREGISTER(sdp_merge_crisscross); + AST_TEST_UNREGISTER(sdp_merge_asymmetric); AST_TEST_UNREGISTER(sdp_ssrc_attributes); return 0; @@ -990,6 +1140,7 @@ static int load_module(void) AST_TEST_REGISTER(sdp_to_topology); AST_TEST_REGISTER(sdp_merge_symmetric); AST_TEST_REGISTER(sdp_merge_crisscross); + AST_TEST_REGISTER(sdp_merge_asymmetric); AST_TEST_REGISTER(sdp_ssrc_attributes); return AST_MODULE_LOAD_SUCCESS; From b8659be9b0028aa2655527772852ae6d740c0fb6 Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Thu, 27 Apr 2017 19:37:53 -0500 Subject: [PATCH 4/4] SDP: Make process possible multiple fmtp attributes per rtpmap. Change-Id: Ie7511008d82b59590e0eb520a21b5e1da4bd7349 --- include/asterisk/sdp.h | 70 ++++++++++++++++++++++++++-- main/sdp.c | 102 ++++++++++++++++++++++++++++++----------- tests/test_sdp.c | 73 +++++++++++++++++++++++++++-- 3 files changed, 210 insertions(+), 35 deletions(-) diff --git a/include/asterisk/sdp.h b/include/asterisk/sdp.h index 8aa9e3b7fa..224a0e5a30 100644 --- a/include/asterisk/sdp.h +++ b/include/asterisk/sdp.h @@ -561,7 +561,40 @@ struct ast_sdp *ast_sdp_alloc(struct ast_sdp_o_line *o_line, struct ast_sdp_t_line *t_line); /*! - * \brief Find an attribute on the top-level SDP + * \brief Find the first attribute match index in the top-level SDP + * + * \note This will not search within streams for the given attribute. + * + * \param sdp The SDP in which to search + * \param attr_name The name of the attribute to search for + * \param payload Optional payload number to search for. If irrelevant, set to -1 + * + * \retval index of attribute line on success. + * \retval -1 on failure or not found. + * + * \since 15.0.0 + */ +int ast_sdp_find_a_first(const struct ast_sdp *sdp, const char *attr_name, int payload); + +/*! + * \brief Find the next attribute match index in the top-level SDP + * + * \note This will not search within streams for the given attribute. + * + * \param sdp The SDP in which to search + * \param last The last matching index found + * \param attr_name The name of the attribute to search for + * \param payload Optional payload number to search for. If irrelevant, set to -1 + * + * \retval index of attribute line on success. + * \retval -1 on failure or not found. + * + * \since 15.0.0 + */ +int ast_sdp_find_a_next(const struct ast_sdp *sdp, int last, const char *attr_name, int payload); + +/*! + * \brief Find an attribute in the top-level SDP * * \note This will not search within streams for the given attribute. * @@ -578,9 +611,40 @@ struct ast_sdp_a_line *ast_sdp_find_attribute(const struct ast_sdp *sdp, const char *attr_name, int payload); /*! - * \brief Find an attribute on an SDP stream (m-line) + * \brief Find the first attribute match index in an SDP stream (m-line) * - * \param sdp The SDP in which to search + * \param m_line The SDP m-line in which to search + * \param attr_name The name of the attribute to search for + * \param payload Optional payload number to search for. If irrelevant, set to -1 + * + * \retval index of attribute line on success. + * \retval -1 on failure or not found. + * + * \since 15.0.0 + */ +int ast_sdp_m_find_a_first(const struct ast_sdp_m_line *m_line, const char *attr_name, + int payload); + +/*! + * \brief Find the next attribute match index in an SDP stream (m-line) + * + * \param m_line The SDP m-line in which to search + * \param last The last matching index found + * \param attr_name The name of the attribute to search for + * \param payload Optional payload number to search for. If irrelevant, set to -1 + * + * \retval index of attribute line on success. + * \retval -1 on failure or not found. + * + * \since 15.0.0 + */ +int ast_sdp_m_find_a_next(const struct ast_sdp_m_line *m_line, int last, + const char *attr_name, int payload); + +/*! + * \brief Find an attribute in an SDP stream (m-line) + * + * \param m_line The SDP m-line in which to search * \param attr_name The name of the attribute to search for * \param payload Optional payload number to search for. If irrelevant, set to -1 * diff --git a/main/sdp.c b/main/sdp.c index 019c6699f1..bfb83e82fe 100644 --- a/main/sdp.c +++ b/main/sdp.c @@ -508,44 +508,81 @@ int ast_sdp_m_add_format(struct ast_sdp_m_line *m_line, const struct ast_sdp_opt || sdp_m_add_fmtp(m_line, format, rtp_code) ? -1 : 0; } -static struct ast_sdp_a_line *sdp_find_attribute_common(const struct ast_sdp_a_lines *a_lines, +static int sdp_find_a_common(const struct ast_sdp_a_lines *a_lines, int start, const char *attr_name, int payload) { struct ast_sdp_a_line *a_line; - int i; + int idx; + + ast_assert(-1 <= start); - for (i = 0; i < AST_VECTOR_SIZE(a_lines); ++i) { + for (idx = start + 1; idx < AST_VECTOR_SIZE(a_lines); ++idx) { int a_line_payload; - a_line = AST_VECTOR_GET(a_lines, i); + a_line = AST_VECTOR_GET(a_lines, idx); if (strcmp(a_line->name, attr_name)) { continue; } if (payload >= 0) { int sscanf_res; + sscanf_res = sscanf(a_line->value, "%30d", &a_line_payload); if (sscanf_res == 1 && payload == a_line_payload) { - return a_line; + return idx; } } else { - return a_line; + return idx; } } - return NULL; + return -1; +} + +int ast_sdp_find_a_first(const struct ast_sdp *sdp, const char *attr_name, int payload) +{ + return sdp_find_a_common(sdp->a_lines, -1, attr_name, payload); +} + +int ast_sdp_find_a_next(const struct ast_sdp *sdp, int last, const char *attr_name, int payload) +{ + return sdp_find_a_common(sdp->a_lines, last, attr_name, payload); } struct ast_sdp_a_line *ast_sdp_find_attribute(const struct ast_sdp *sdp, const char *attr_name, int payload) { - return sdp_find_attribute_common(sdp->a_lines, attr_name, payload); + int idx; + + idx = ast_sdp_find_a_first(sdp, attr_name, payload); + if (idx < 0) { + return NULL; + } + return ast_sdp_get_a(sdp, idx); +} + +int ast_sdp_m_find_a_first(const struct ast_sdp_m_line *m_line, const char *attr_name, + int payload) +{ + return sdp_find_a_common(m_line->a_lines, -1, attr_name, payload); +} + +int ast_sdp_m_find_a_next(const struct ast_sdp_m_line *m_line, int last, + const char *attr_name, int payload) +{ + return sdp_find_a_common(m_line->a_lines, last, attr_name, payload); } struct ast_sdp_a_line *ast_sdp_m_find_attribute(const struct ast_sdp_m_line *m_line, const char *attr_name, int payload) { - return sdp_find_attribute_common(m_line->a_lines, attr_name, payload); + int idx; + + idx = ast_sdp_m_find_a_first(m_line, attr_name, payload); + if (idx < 0) { + return NULL; + } + return ast_sdp_m_get_a(m_line, idx); } struct ast_sdp_rtpmap *ast_sdp_rtpmap_alloc(int payload, const char *encoding_name, @@ -644,17 +681,8 @@ static struct ast_sdp_rtpmap *sdp_payload_get_rtpmap(const struct ast_sdp_m_line return ast_sdp_a_get_rtpmap(rtpmap_attr); } -/*! - * \brief Find and process fmtp attributes for a given payload - * - * \param m_line The stream on which to search for the fmtp attribute - * \param payload The specific fmtp attribute to search for - * \param codecs The current RTP codecs that have been built up - */ -static void process_fmtp(const struct ast_sdp_m_line *m_line, int payload, - struct ast_rtp_codecs *codecs) +static void process_fmtp_value(const char *value, int payload, struct ast_rtp_codecs *codecs) { - struct ast_sdp_a_line *attr; char *param; char *param_start; char *param_end; @@ -662,13 +690,11 @@ static void process_fmtp(const struct ast_sdp_m_line *m_line, int payload, struct ast_format *replace; struct ast_format *format; - attr = ast_sdp_m_find_attribute(m_line, "fmtp", payload); - if (!attr) { - return; - } - - /* Extract the "a=fmtp:%d %s" attribute parameter string after the payload type. */ - param_start = ast_skip_nonblanks(attr->value);/* Skip payload type */ + /* + * Extract the "a=fmtp:%d %s" attribute parameter string value which + * starts after the colon. + */ + param_start = ast_skip_nonblanks(value);/* Skip payload type */ param_start = ast_skip_blanks(param_start); param_end = ast_skip_nonblanks(param_start); if (param_end == param_start) { @@ -693,6 +719,28 @@ static void process_fmtp(const struct ast_sdp_m_line *m_line, int payload, ao2_ref(format, -1); } +/*! + * \brief Find and process all fmtp attribute lines for a given payload + * + * \param m_line The stream on which to search for the fmtp attributes + * \param payload The specific fmtp attribute to search for + * \param codecs The current RTP codecs that have been built up + */ +static void process_fmtp_lines(const struct ast_sdp_m_line *m_line, int payload, + struct ast_rtp_codecs *codecs) +{ + const struct ast_sdp_a_line *a_line; + int idx; + + idx = ast_sdp_m_find_a_first(m_line, "fmtp", payload); + for (; 0 <= idx; idx = ast_sdp_m_find_a_next(m_line, idx, "fmtp", payload)) { + a_line = ast_sdp_m_get_a(m_line, idx); + ast_assert(a_line != NULL); + + process_fmtp_value(a_line->value, payload, codecs); + } +} + /* * Needed so we don't have an external function referenced as data. * The dynamic linker doesn't handle that very well. @@ -765,7 +813,7 @@ static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line, if (!ast_rtp_codecs_payloads_set_rtpmap_type_rate(codecs, NULL, payload, m_line->type, rtpmap->encoding_name, options, rtpmap->clock_rate)) { /* Successfully mapped the payload type to format */ - process_fmtp(m_line, payload, codecs); + process_fmtp_lines(m_line, payload, codecs); } ast_sdp_rtpmap_free(rtpmap); } diff --git a/tests/test_sdp.c b/tests/test_sdp.c index 79b9e7b2c9..7eef3f741e 100644 --- a/tests/test_sdp.c +++ b/tests/test_sdp.c @@ -276,6 +276,7 @@ AST_TEST_DEFINE(find_attr) enum ast_test_result_state res = AST_TEST_PASS; struct ast_sdp_m_line *m_line; struct ast_sdp_a_line *a_line; + int idx; switch(cmd) { case TEST_INIT: @@ -283,7 +284,7 @@ AST_TEST_DEFINE(find_attr) info->category = "/main/sdp/"; info->summary = "Ensure that finding attributes works as expected"; info->description = - "An SDP m-line is created, and two attributes are added.\n" + "A SDP m-line is created, and attributes are added.\n" "We then attempt a series of attribute-finding calls that are expected to work\n" "followed by a series of attribute-finding calls that are expected fo fail."; return AST_TEST_NOT_RUN; @@ -302,6 +303,12 @@ AST_TEST_DEFINE(find_attr) goto end; } ast_sdp_m_add_a(m_line, a_line); + a_line = ast_sdp_a_alloc("foo", "0 bee"); + if (!a_line) { + res = AST_TEST_FAIL; + goto end; + } + ast_sdp_m_add_a(m_line, a_line); a_line = ast_sdp_a_alloc("baz", "howdy"); if (!a_line) { @@ -312,21 +319,77 @@ AST_TEST_DEFINE(find_attr) /* These should work */ a_line = ast_sdp_m_find_attribute(m_line, "foo", 0); - if (!a_line) { + if (!a_line || strcmp(a_line->value, "0 bar")) { ast_test_status_update(test, "Failed to find attribute 'foo' with payload '0'\n"); res = AST_TEST_FAIL; } a_line = ast_sdp_m_find_attribute(m_line, "foo", -1); - if (!a_line) { + if (!a_line || strcmp(a_line->value, "0 bar")) { ast_test_status_update(test, "Failed to find attribute 'foo' with unspecified payload\n"); res = AST_TEST_FAIL; } a_line = ast_sdp_m_find_attribute(m_line, "baz", -1); - if (!a_line) { + if (!a_line || strcmp(a_line->value, "howdy")) { ast_test_status_update(test, "Failed to find attribute 'baz' with unspecified payload\n"); res = AST_TEST_FAIL; } + idx = ast_sdp_m_find_a_first(m_line, "foo", 0); + if (idx < 0) { + ast_test_status_update(test, "Failed to find first attribute 'foo' with payload '0'\n"); + res = AST_TEST_FAIL; + goto end; + } + a_line = ast_sdp_m_get_a(m_line, idx); + if (!a_line || strcmp(a_line->value, "0 bar")) { + ast_test_status_update(test, "Find first attribute 'foo' with payload '0' didn't match\n"); + res = AST_TEST_FAIL; + } + idx = ast_sdp_m_find_a_next(m_line, idx, "foo", 0); + if (idx < 0) { + ast_test_status_update(test, "Failed to find next attribute 'foo' with payload '0'\n"); + res = AST_TEST_FAIL; + goto end; + } + a_line = ast_sdp_m_get_a(m_line, idx); + if (!a_line || strcmp(a_line->value, "0 bee")) { + ast_test_status_update(test, "Find next attribute 'foo' with payload '0' didn't match\n"); + res = AST_TEST_FAIL; + } + idx = ast_sdp_m_find_a_next(m_line, idx, "foo", 0); + if (0 <= idx) { + ast_test_status_update(test, "Find next attribute 'foo' with payload '0' found too many\n"); + res = AST_TEST_FAIL; + } + + idx = ast_sdp_m_find_a_first(m_line, "foo", -1); + if (idx < 0) { + ast_test_status_update(test, "Failed to find first attribute 'foo' with unspecified payload\n"); + res = AST_TEST_FAIL; + goto end; + } + a_line = ast_sdp_m_get_a(m_line, idx); + if (!a_line || strcmp(a_line->value, "0 bar")) { + ast_test_status_update(test, "Find first attribute 'foo' with unspecified payload didn't match\n"); + res = AST_TEST_FAIL; + } + idx = ast_sdp_m_find_a_next(m_line, idx, "foo", -1); + if (idx < 0) { + ast_test_status_update(test, "Failed to find next attribute 'foo' with unspecified payload\n"); + res = AST_TEST_FAIL; + goto end; + } + a_line = ast_sdp_m_get_a(m_line, idx); + if (!a_line || strcmp(a_line->value, "0 bee")) { + ast_test_status_update(test, "Find next attribute 'foo' with unspecified payload didn't match\n"); + res = AST_TEST_FAIL; + } + idx = ast_sdp_m_find_a_next(m_line, idx, "foo", -1); + if (0 <= idx) { + ast_test_status_update(test, "Find next attribute 'foo' with unspecified payload found too many\n"); + res = AST_TEST_FAIL; + } + /* These should fail */ a_line = ast_sdp_m_find_attribute(m_line, "foo", 1); if (a_line) { @@ -345,7 +408,7 @@ AST_TEST_DEFINE(find_attr) } a_line = ast_sdp_m_find_attribute(m_line, "wibble", -1); if (a_line) { - ast_test_status_update(test, "Found non-existent attribute 'foo' with unspecified payload\n"); + ast_test_status_update(test, "Found non-existent attribute 'wibble' with unspecified payload\n"); res = AST_TEST_FAIL; }