<p>Joshua Colp <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15386">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
Kevin Harwell: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">res_format_attr_h263: Generate valid SDP fmtp for H.263+.<br><br>Fixed:<br>* RFC 4629 does not allow the value "0" for MPI, K, and N.<br>* Allow value "0" for PAR.<br>* BPP is printed only when specified because "0" has a meaning.<br><br>New:<br>* Added CPCF and MaxBR.<br>* Some implementations provide CIF without MPI: a=fmtp:xx CIF;F=1<br> Although a violation of RFC 3555 section 3, we can support that.<br><br>Changed:<br>* Resorts the CIFs from large to small which partly fixes ASTERISK~29267.<br><br>Change-Id: I95a650c715007b8dde11a77cb37d9c6c123a441e<br>---<br>M res/res_format_attr_h263.c<br>1 file changed, 123 insertions(+), 9 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/res/res_format_attr_h263.c b/res/res_format_attr_h263.c</span><br><span>index b49f1d6..5246725 100644</span><br><span>--- a/res/res_format_attr_h263.c</span><br><span>+++ b/res/res_format_attr_h263.c</span><br><span>@@ -36,6 +36,9 @@</span><br><span> #include "asterisk/module.h"</span><br><span> #include "asterisk/format.h"</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! \brief Value that indicates an attribute is actually unset */</span><br><span style="color: hsl(120, 100%, 40%);">+#define H263_ATTR_KEY_UNSET UINT8_MAX</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct h263_attr {</span><br><span> unsigned int SQCIF; /*!< Minimum picture interval for SQCIF resolution */</span><br><span> unsigned int QCIF; /*!< Minimum picture interval for QCIF resolution */</span><br><span>@@ -46,6 +49,14 @@</span><br><span> unsigned int CUSTOM_XMAX; /*!< Custom resolution (Xmax) */</span><br><span> unsigned int CUSTOM_YMAX; /*!< Custom resolution (Ymax) */</span><br><span> unsigned int CUSTOM_MPI; /*!< Custom resolution (MPI) */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF; /*!< Custom Picture Clock Frequency */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_2;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_3;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_4;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_5;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_6;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_7;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int CPCF_MPI;</span><br><span> unsigned int F; /*!< F annex support */</span><br><span> unsigned int I; /*!< I annex support */</span><br><span> unsigned int J; /*!< J annex support */</span><br><span>@@ -60,6 +71,7 @@</span><br><span> unsigned int PAR_HEIGHT; /*!< Pixel aspect ratio (height) */</span><br><span> unsigned int BPP; /*!< Bits per picture maximum */</span><br><span> unsigned int HRD; /*!< Hypothetical reference decoder status */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int MaxBR; /*!< Vendor Specific: CounterPath Bria (Solo) */</span><br><span> };</span><br><span> </span><br><span> static void h263_destroy(struct ast_format *format)</span><br><span>@@ -123,6 +135,14 @@</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, CUSTOM_XMAX);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, CUSTOM_YMAX);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, CUSTOM_MPI);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_2);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_3);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_4);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_5);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_6);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_7);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, CPCF_MPI);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, F);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, I);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, J);</span><br><span>@@ -137,6 +157,7 @@</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, PAR_HEIGHT);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, BPP);</span><br><span> DETERMINE_JOINT(attr, attr1, attr2, HRD);</span><br><span style="color: hsl(120, 100%, 40%);">+ DETERMINE_JOINT(attr, attr1, attr2, MaxBR);</span><br><span> </span><br><span> return cloned;</span><br><span> }</span><br><span>@@ -153,27 +174,54 @@</span><br><span> }</span><br><span> attr = ast_format_get_attribute_data(cloned);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ attr->BPP = H263_ATTR_KEY_UNSET;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->MaxBR = H263_ATTR_KEY_UNSET;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->PAR_WIDTH = H263_ATTR_KEY_UNSET;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->PAR_HEIGHT = H263_ATTR_KEY_UNSET;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> while ((attrib = strsep(&attribs, ";"))) {</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned int val, val2 = 0, val3 = 0, val4 = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int val, val2 = 0, val3 = 0, val4 = 0, val5 = 0, val6 = 0, val7 = 0, val8 = 0;</span><br><span> </span><br><span> attrib = ast_strip(attrib);</span><br><span> </span><br><span> if (sscanf(attrib, "SQCIF=%30u", &val) == 1) {</span><br><span> attr->SQCIF = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(attrib, "SQCIF") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->SQCIF = 1;</span><br><span> } else if (sscanf(attrib, "QCIF=%30u", &val) == 1) {</span><br><span> attr->QCIF = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(attrib, "QCIF") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->QCIF = 1;</span><br><span> } else if (sscanf(attrib, "CIF=%30u", &val) == 1) {</span><br><span> attr->CIF = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(attrib, "CIF") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CIF = 1;</span><br><span> } else if (sscanf(attrib, "CIF4=%30u", &val) == 1) {</span><br><span> attr->CIF4 = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(attrib, "CIF4") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CIF4 = 1;</span><br><span> } else if (sscanf(attrib, "CIF16=%30u", &val) == 1) {</span><br><span> attr->CIF16 = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(attrib, "CIF16") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CIF16 = 1;</span><br><span> } else if (sscanf(attrib, "VGA=%30u", &val) == 1) {</span><br><span> attr->VGA = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (strcmp(attrib, "VGA") == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->VGA = 1;</span><br><span> } else if (sscanf(attrib, "CUSTOM=%30u,%30u,%30u", &val, &val2, &val3) == 3) {</span><br><span> attr->CUSTOM_XMAX = val;</span><br><span> attr->CUSTOM_YMAX = val2;</span><br><span> attr->CUSTOM_MPI = val3;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (sscanf(attrib, "CPCF=%30u,%30u,%30u,%30u,%30u,%30u,%30u,%30u",</span><br><span style="color: hsl(120, 100%, 40%);">+ &val, &val2, &val3, &val4, &val5, &val6, &val7, &val8) == 8) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF = val;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_2 = val2;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_3 = val3;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_4 = val4;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_5 = val5;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_6 = val6;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_7 = val7;</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_MPI = val8;</span><br><span> } else if (sscanf(attrib, "F=%30u", &val) == 1) {</span><br><span> attr->F = val;</span><br><span> } else if (sscanf(attrib, "I=%30u", &val) == 1) {</span><br><span>@@ -198,34 +246,85 @@</span><br><span> attr->P_SUB2 = val2;</span><br><span> attr->P_SUB3 = val3;</span><br><span> attr->P_SUB4 = val4;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (sscanf(attrib, "MAXBR=%30u", &val) == 1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->MaxBR = val;</span><br><span> }</span><br><span> }</span><br><span> </span><br><span> return cloned;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define APPEND_IF_NOT_H263_UNSET(field, str, name) do { \</span><br><span style="color: hsl(120, 100%, 40%);">+ if (field != H263_ATTR_KEY_UNSET) { \</span><br><span style="color: hsl(120, 100%, 40%);">+ if (added) { \</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, ";"); \</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) { \</span><br><span style="color: hsl(120, 100%, 40%);">+ added = 1; \</span><br><span style="color: hsl(120, 100%, 40%);">+ } \</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, "%s=%u", name, field); \</span><br><span style="color: hsl(120, 100%, 40%);">+ } \</span><br><span style="color: hsl(120, 100%, 40%);">+} while (0)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#define APPEND_IF_NONZERO(field, str, name) do { \</span><br><span style="color: hsl(120, 100%, 40%);">+ if (field) { \</span><br><span style="color: hsl(120, 100%, 40%);">+ if (added) { \</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, ";"); \</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) { \</span><br><span style="color: hsl(120, 100%, 40%);">+ added = 1; \</span><br><span style="color: hsl(120, 100%, 40%);">+ } \</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, "%s=%u", name, field); \</span><br><span style="color: hsl(120, 100%, 40%);">+ } \</span><br><span style="color: hsl(120, 100%, 40%);">+} while (0)</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static void h263_generate_sdp_fmtp(const struct ast_format *format, unsigned int payload, struct ast_str **str)</span><br><span> {</span><br><span> struct h263_attr *attr = ast_format_get_attribute_data(format);</span><br><span style="color: hsl(120, 100%, 40%);">+ int added = 0;</span><br><span> </span><br><span> if (!attr) {</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- ast_str_append(str, 0, "a=fmtp:%u SQCIF=%u;QCIF=%u;CIF=%u;CIF4=%u;CIF16=%u;VGA=%u;F=%u;I=%u;J=%u;T=%u;K=%u;N=%u;BPP=%u;HRD=%u",</span><br><span style="color: hsl(0, 100%, 40%);">- payload, attr->SQCIF, attr->QCIF, attr->CIF, attr->CIF4, attr->CIF16, attr->VGA, attr->F, attr->I, attr->J,</span><br><span style="color: hsl(0, 100%, 40%);">- attr->T, attr->K, attr->N, attr->BPP, attr->HRD);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (attr->CPCF) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (added) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, ";");</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ added = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, "CPCF=%u,%u,%u,%u,%u,%u,%u,%u", attr->CPCF, attr->CPCF_2, attr->CPCF_3,</span><br><span style="color: hsl(120, 100%, 40%);">+ attr->CPCF_4, attr->CPCF_5, attr->CPCF_6, attr->CPCF_7, attr->CPCF_MPI);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->CIF16, str, "CIF16");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->CIF4, str, "CIF4");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->VGA, str, "VGA");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->CIF, str, "CIF");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->QCIF, str, "QCIF");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->SQCIF, str, "SQCIF");</span><br><span> </span><br><span> if (attr->CUSTOM_XMAX && attr->CUSTOM_YMAX && attr->CUSTOM_MPI) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_str_append(str, 0, ";CUSTOM=%u,%u,%u", attr->CUSTOM_XMAX, attr->CUSTOM_YMAX, attr->CUSTOM_MPI);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (added) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, ";");</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ added = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, "CUSTOM=%u,%u,%u", attr->CUSTOM_XMAX, attr->CUSTOM_YMAX, attr->CUSTOM_MPI);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (attr->PAR_WIDTH && attr->PAR_HEIGHT) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_str_append(str, 0, ";PAR=%u:%u", attr->PAR_WIDTH, attr->PAR_HEIGHT);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->F, str, "F");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->I, str, "I");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->J, str, "J");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->T, str, "T");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->K, str, "K");</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->N, str, "N");</span><br><span> </span><br><span> if (attr->P_SUB1) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_str_append(str, 0, ";P=%u", attr->P_SUB1);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (added) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, ";");</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ added = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, "P=%u", attr->P_SUB1);</span><br><span> if (attr->P_SUB2) {</span><br><span> ast_str_append(str, 0, ",%u", attr->P_SUB2);</span><br><span> }</span><br><span>@@ -237,6 +336,21 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (attr->PAR_WIDTH != H263_ATTR_KEY_UNSET && attr->PAR_HEIGHT != H263_ATTR_KEY_UNSET) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (added) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, ";");</span><br><span style="color: hsl(120, 100%, 40%);">+ } else if (0 < ast_str_append(str, 0, "a=fmtp:%u ", payload)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ added = 1; \</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ ast_str_append(str, 0, "PAR=%u:%u", attr->PAR_WIDTH, attr->PAR_HEIGHT);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NOT_H263_UNSET(attr->BPP, str, "BPP");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NONZERO(attr->HRD, str, "HRD");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ APPEND_IF_NOT_H263_UNSET(attr->MaxBR, str, "MaxBR");</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> ast_str_append(str, 0, "\r\n");</span><br><span> </span><br><span> return;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15386">change 15386</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/15386"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I95a650c715007b8dde11a77cb37d9c6c123a441e </div>
<div style="display:none"> Gerrit-Change-Number: 15386 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Alexander Traud <pabstraud@compuserve.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>