[Asterisk-code-review] res format attr h264: Do not reset string buffer. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged.

Change subject: res_format_attr_h264: Do not reset string buffer.
......................................................................


res_format_attr_h264: Do not reset string buffer.

When no parameter is present, Asterisk does not generate the line fmtp, as
expected. However, because a buffer was reset, even rtpmap and fmtp of previous
media codecs got removed. Now, Asterisk does not reset other codecs in case of
no parameter for H.264.

ASTERISK-25573 #close

Change-Id: I93811331f4a28c45418a9e14ee46c0debd47a286
---
M res/res_format_attr_h264.c
1 file changed, 6 insertions(+), 10 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, approved



diff --git a/res/res_format_attr_h264.c b/res/res_format_attr_h264.c
index 9527469..ccf2176 100644
--- a/res/res_format_attr_h264.c
+++ b/res/res_format_attr_h264.c
@@ -236,7 +236,7 @@
 	if (field != H264_ATTR_KEY_UNSET) {	\
 		if (added) {	\
 			ast_str_append(str, 0, ";");	\
-		} else {	\
+		} else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {	\
 			added = 1;	\
 		}	\
 		ast_str_append(str, 0, "%s=%u", name, field);	\
@@ -247,7 +247,7 @@
 	if (field) {	\
 		if (added) {	\
 			ast_str_append(str, 0, ";");	\
-		} else {	\
+		} else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {	\
 			added = 1;	\
 		}	\
 		ast_str_append(str, 0, "%s=%u", name, field);	\
@@ -262,8 +262,6 @@
 	if (!attr) {
 		return;
 	}
-
-	ast_str_append(str, 0, "a=fmtp:%u ", payload);
 
 	APPEND_IF_NONZERO(attr->MAX_MBPS, str, "max-mbps");
 	APPEND_IF_NONZERO(attr->MAX_FS, str, "max-fs");
@@ -287,7 +285,7 @@
 	if (attr->PROFILE_IDC && attr->PROFILE_IOP && attr->LEVEL) {
 		if (added) {
 			ast_str_append(str, 0, ";");
-		} else {
+		} else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {
 			added = 1;
 		}
 		ast_str_append(str, 0, "profile-level-id=%02X%02X%02X", attr->PROFILE_IDC, attr->PROFILE_IOP, attr->LEVEL);
@@ -296,15 +294,13 @@
 	if (!ast_strlen_zero(attr->SPS) && !ast_strlen_zero(attr->PPS)) {
 		if (added) {
 			ast_str_append(str, 0, ";");
-		} else {
+		} else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {
 			added = 1;
 		}
-		ast_str_append(str, 0, ";sprop-parameter-sets=%s,%s", attr->SPS, attr->PPS);
+		ast_str_append(str, 0, "sprop-parameter-sets=%s,%s", attr->SPS, attr->PPS);
 	}
 
-	if (!added) {
-		ast_str_reset(*str);
-	} else {
+	if (added) {
 		ast_str_append(str, 0, "\r\n");
 	}
 

-- 
To view, visit https://gerrit.asterisk.org/1661
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I93811331f4a28c45418a9e14ee46c0debd47a286
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>