[Asterisk-code-review] channel: write to a stream on multi-frame writes (asterisk[certified/16.3])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14069 )

Change subject: channel: write to a stream on multi-frame writes
......................................................................

channel: write to a stream on multi-frame writes

If a frame handling routine returns a list of frames (vs. a single frame)
those frames are never passed to a tech's write_stream handler even if one is
available. For instance, if a codec translation occurred and that codec
returned multiple frames then those particular frames were always only sent
to the tech's "write" handler. If that tech (pjsip for example) was stream
capable then those frames were essentially ignored. Thus resulting in bad
audio.

This patch makes it so the "write_stream" handler is appropriately called
for all cases, and for all frames if available.

ASTERISK-28795 #close

Change-Id: I868faea0b73a07ed5a32c2b05bb9cf4b586f739d
---
M main/channel.c
1 file changed, 16 insertions(+), 23 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/main/channel.c b/main/channel.c
index cb413ff..7eb2546 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -5070,6 +5070,18 @@
 	adjust_frame_for_plc(chan, frame, datastore);
 }
 
+static int tech_write(struct ast_channel *chan, struct ast_stream *stream,
+			struct ast_stream *default_stream, struct ast_frame *frame)
+{
+	if (ast_channel_tech(chan)->write_stream) {
+		return stream ? ast_channel_tech(chan)->write_stream(
+			chan, ast_stream_get_position(stream), frame) : 0;
+	}
+
+	return ((stream == default_stream) && ast_channel_tech(chan)->write) ?
+		ast_channel_tech(chan)->write(chan, frame) : 0;
+}
+
 int ast_write(struct ast_channel *chan, struct ast_frame *fr)
 {
 	return ast_write_stream(chan, -1, fr);
@@ -5223,17 +5235,7 @@
 		break;
 	case AST_FRAME_MODEM:
 		CHECK_BLOCKING(chan);
-		if (ast_channel_tech(chan)->write_stream) {
-			if (stream) {
-				res = ast_channel_tech(chan)->write_stream(chan, ast_stream_get_position(stream), fr);
-			} else {
-				res = 0;
-			}
-		} else if ((stream == default_stream) && ast_channel_tech(chan)->write) {
-			res = ast_channel_tech(chan)->write(chan, fr);
-		} else {
-			res = 0;
-		}
+		res = tech_write(chan, stream, default_stream, fr);
 		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;
 	case AST_FRAME_VOICE:
@@ -5400,7 +5402,8 @@
 				next = AST_LIST_NEXT(cur, frame_list);
 				AST_LIST_NEXT(cur, frame_list) = NULL;
 				if (!skip) {
-					if ((res = ast_channel_tech(chan)->write(chan, cur)) < 0) {
+					res = tech_write(chan, stream, default_stream, cur);
+					if (res < 0) {
 						ast_channel_softhangup_internal_flag_add(chan, AST_SOFTHANGUP_DEV);
 						skip = 1;
 					} else if (next) {
@@ -5417,17 +5420,7 @@
 			/* reset f so the code below doesn't attempt to free it */
 			f = NULL;
 		} else {
-			if (ast_channel_tech(chan)->write_stream) {
-				if (stream) {
-					res = ast_channel_tech(chan)->write_stream(chan, ast_stream_get_position(stream), f);
-				} else {
-					res = 0;
-				}
-			} else if ((stream == default_stream) && ast_channel_tech(chan)->write) {
-				res = ast_channel_tech(chan)->write(chan, f);
-			} else {
-				res = 0;
-			}
+			res = tech_write(chan, stream, default_stream, f);
 		}
 		ast_clear_flag(ast_channel_flags(chan), AST_FLAG_BLOCKING);
 		break;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14069
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: certified/16.3
Gerrit-Change-Id: I868faea0b73a07ed5a32c2b05bb9cf4b586f739d
Gerrit-Change-Number: 14069
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200402/9bf9012c/attachment-0001.html>