[Asterisk-code-review] res format attr opus: Update to latest RFC 7587. (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: res_format_attr_opus: Update to latest RFC 7587.
......................................................................


Patch Set 3: Code-Review-1

(8 comments)

https://gerrit.asterisk.org/#/c/1674/3/res/res_format_attr_opus.c
File res/res_format_attr_opus.c:

Line 84: 	if (original) {
       : 		*attr = *original;
       : 	}
Since the defaults are no longer zero any more there should be an else clause:

if () {
} else {
  *attr = default_opus_attr;
}


Line 109: 		attr->maxplayrate = 48000;
Rather than using the magic number here, how about:
attr->maxplayrate = default_opus_attr.maxplayrate;


Line 115: 		attr->spropmaxcapturerate = 48000;
Similar here


Line 121: 		attr->maxbitrate = 510000;
Similar here


Line 124: 	if (attributes == strstr(attributes, "stereo=1")) {
Rather than potentially scanning the whole attributes string for something that we only care starts the string, how about:
if (!strncmp(attributes, "stereo=1", 8))


Line 170: 	if (48000 != attr->maxplayrate) {
Eliminate the magic number:
if (default_opus_attr.maxplayrate != attr->maxplayrate)


Line 179: 	if (48000 != attr->spropmaxcapturerate) {
Similar here


Line 188: 	if (510000 != attr->maxbitrate) {
Similar here


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae85ba3e5960bfd5d51cf65bcffad00dd4875a73
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
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>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes