libosmocore[master]: gsm0808: add function to translate perm speech to speech cod...

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:22:18 UTC 2017


Patch Set 3: Code-Review-1

(5 comments)

https://gerrit.osmocom.org/#/c/2830/3/include/osmocom/gsm/gsm0808_utils.h
File include/osmocom/gsm/gsm0808_utils.h:

Line 77:  * representation we use in struct gsm0808_speech_codec */
Typically we don't place comments with the function declarations in .h files. Not sure why this file has comments for each. The API doc belongs in the .c file as proper doxygen comments, and shouldn't be duplicated in the .h file.


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

Line 659:  * representation we use in struct gsm0808_speech_codec */
Make this a proper doxygen comment, you will see that the other functions have them now when you rebase this patch onto current master.


Line 665: 	 * (See also 3GPP TS 48.008, 3.2.2.11 and 3.2.2.103) */
Does this comment rather belong joined to above API doc comment?


Line 670: 		break;
after a 'return' no 'break' necessary. Drop those.


Line 694: 		break;
in another patch you had a default: OSMO_ASSERT(false) ... why not here? Are we checking the -EINVAL return value at the caller?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib26a9c20864459b2baaa04f49b6e7902ba44b7cb
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