stream: Create matching joint topology

[btriller]

When creating joint topologies, resolved topologies have to have equal stream counts. Currently this only happens if first of the two given topologies has equal or more streams.
This change adds remaining streams of second topology to the joint topology.

[btriller]

cherry-pick-to: 21
cherry-pick-to: 20
cherry-pick-to: 18

[maximilianfridrich]

While developing, I recommend building Asterisk with the --enable-dev-mode flag (./configure --enable-dev-mode).

[maximilianfridrich]

This change adds remaining streams of second topology to the joint topology.

Isn't that what operation: union does? Have you tried doing codec_prefs_* = ..., operation: union, ...?

[btriller]

This change adds remaining streams of second topology to the joint topology.

Isn't that what operation: union does? Have you tried doing codec_prefs_* = ..., operation: union, ...?

This is about streams (m=audio/video lines) in SDP. union operation works on codecs/formats in these streams.

[maximilianfridrich]

That makes sense, sorry for the confusion.

[maximilianfridrich]

Looks good to me. I also ran the ACN testsuite PR with it and there's no issues.

@btriller Could you describe a scenario (with configured endpoint settings, SDP offers and answers) that failed before this change and now works? Then I can add it to the testsuite to ensure it works and will continue to work.

[btriller]

Looks good to me. I also ran the ACN testsuite PR with it and there's no issues.

@btriller Could you describe a scenario (with configured endpoint settings, SDP offers and answers) that failed before this change and now works? Then I can add it to the testsuite to ensure it works and will continue to work.

Attached scenario fails. Without this PR's change, Asterisk emits a reINVITE after answering to alice (caller). Asterisk logs following:

[Nov  3 14:09:32] WARNING[3743379] res_pjsip_session.c: PJSIP/alice-00000000: Local SDP contains 1 media streams while we expected it to contain 2

testsuite.txt

[maximilianfridrich]

Thank you for providing the test case!

Without this PR's change, Asterisk emits a reINVITE after answering to alice (caller)

You're right, I can reproduce that. With your change the reINVITE is gone. Unfortunately, the SIPP scenario still fails for me - the caller receives all codecs on the SDP answer and I am not sure yet why. I don't think this has anything to do with this PR though.

[btriller]

Thank you for providing the test case!

Without this PR's change, Asterisk emits a reINVITE after answering to alice (caller)

You're right, I can reproduce that. With your change the reINVITE is gone. Unfortunately, the SIPP scenario still fails for me - the caller receives all codecs on the SDP answer and I am not sure yet why. I don't think this has anything to do with this PR though.

I get No common codecs between channels and transcoding allowed. Still answering call. for the video stream.

@maximilianfridrich Maybe like that?

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 706d19bf09..03095d5911 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -775,6 +775,9 @@ static int chan_pjsip_answer_with_stream_topology(struct ast_channel *ast, struc
 		}
 
 		for (index = 0; index < ast_stream_topology_get_count(resolved_top); ++index) {
+			if (ast_stream_get_state(ast_stream_topology_get_stream(resolved_top, index)) == AST_STREAM_STATE_REMOVED) {
+				continue;
+			}
 			if (ast_format_cap_empty(ast_stream_get_formats(ast_stream_topology_get_stream(resolved_top, index)))) {
 				if (prefs->transcode == CODEC_NEGOTIATION_TRANSCODE_ALLOW) {
 					ast_log(LOG_DEBUG, "%s No common codecs between channels and transcoding allowed. Still answering call.\n",

[btriller]

diff --git a/channels/chan_pjsip.c b/channels/chan_pjsip.c
index 706d19bf09..03095d5911 100644
--- a/channels/chan_pjsip.c
+++ b/channels/chan_pjsip.c
@@ -775,6 +775,9 @@ static int chan_pjsip_answer_with_stream_topology(struct ast_channel *ast, struc
 		}
 
 		for (index = 0; index < ast_stream_topology_get_count(resolved_top); ++index) {
+			if (ast_stream_get_state(ast_stream_topology_get_stream(resolved_top, index)) == AST_STREAM_STATE_REMOVED) {
+				continue;
+			}
 			if (ast_format_cap_empty(ast_stream_get_formats(ast_stream_topology_get_stream(resolved_top, index)))) {
 				if (prefs->transcode == CODEC_NEGOTIATION_TRANSCODE_ALLOW) {
 					ast_log(LOG_DEBUG, "%s No common codecs between channels and transcoding allowed. Still answering call.\n",

You added this patch here https://github.com/asterisk/asterisk/compare/b85addb8d96ac646dbe26ccd79fe3fd9708d2b38..317a31e6bf2ea679a7bacab46d3b6d5cfabc5dd0 but removed it https://github.com/asterisk/asterisk/compare/317a31e6bf2ea679a7bacab46d3b6d5cfabc5dd0..eb5b500f70ed85d6f177e8b44c1d7455c2167c5a.
Was removal intended? @maximilianfridrich

[maximilianfridrich]

Was removal intended?

Yes, that was intended. This change broke many test cases of channels/pjsip/codec_negotiation while your provided test case was still failing. I haven't managed to make that test pass yet. Have you managed to make it pass?

[asteriskteam]

This PR has been marked stale because it has been in "Changes Requested" or "submitter-action-required" state for 28 days or more. Please make the requested changes within 14 days or the PR will be closed.

[asteriskteam]

This PR has been closed because it has been in "Changes Requested" or "submitter-action-required" state for more than 42 days.

[btriller]

Requested change has been implemented (https://github.com/asterisk/asterisk/compare/c41889f859c483e31c7c4a49e7b8df210a5f2239..42c0599b27f572fca35950d32628a309a9c0550f)