libosmocore[master]: gsm0808: add function to extrapolate speech codec

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Wed Jun 14 23:31:55 UTC 2017


Patch Set 3: Code-Review-1

(7 comments)

https://gerrit.osmocom.org/#/c/2831/3//COMMIT_MSG
Commit Message:

Line 17: This patch adds a function that can be used as a helper to fill
(generally in commit logs, rather use the imperative form: "Add function foo()." It is also good practice to put relevant information in the API doc and limit the commit log to reasons / decision process)


https://gerrit.osmocom.org/#/c/2831/3/src/gsm/gsm0808_utils.c
File src/gsm/gsm0808_utils.c:

Line 701: /* Extrapolate a speech codec field from a given permitted speech parameter */
Need proper API doc, as you will see for other functions now committed on master.


Line 707: 	 * codec that field that consistantly matches the channel type
"that field that"?

"consistently"


Line 708: 	 * configuration. This will basically reflect a non-transcoding-
"This reflects a"


Line 710: 	 * stream may be differ from the codec used on the air interface) */
"may differ from"

seems to me this comment belongs in the API doc


Line 722: 	/* Depending on the, pick a default codc configuration, that
on the?

No comma before "that"


Line 743: 	}
missing "default:" case to return -EINVAL?


-- 
To view, visit https://gerrit.osmocom.org/2831
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I257c972e9fdf0dfe940a8d483447085bd62e50a2
Gerrit-PatchSet: 3
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list