libosmocore[master]: core/conv: add optimized Viterbi decoding

tnt gerrit-no-reply at lists.osmocom.org
Thu Jan 5 20:46:34 UTC 2017


Patch Set 4: Code-Review-1

(4 comments)

See my comments on http://lists.osmocom.org/pipermail/openbsc/2015-July/000315.html

Those don't seem to have been addresses here.

https://gerrit.osmocom.org/#/c/1337/4/src/viterbi.c
File src/viterbi.c:

Line 14:  * Lesser General Public License for more details.
> Does this license compatible with libosmocore?
It'd be better to get the original author permission to relicense it to the same one if possible.


Line 326: 	trellis = (struct vtrellis *) calloc(1, sizeof(struct vtrellis));
> Here we can use the static allocation too.
Nope ... treillis is returned to the caller and the library has to support several callers at onces from different threads, there can be _NO_ shared global objects.


PS4, Line 426: if (dec->recursive)
             : 		_traceback_rec(dec, state, out, len);
             : 	else
             : 		_traceback(dec, state, out, len);
> What to do with "_"?
As long as the naming is consistent within a file, it's fine for static function.


Line 457: 	dec = (struct vdecoder *) calloc(1, sizeof(struct vdecoder));
> I suggest to use static memory allocation here. One can be declared inside 
Again, you can't have global static objects ... ever.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I74d355274b4176a7d924f91ef3c96912ce338fb2
Gerrit-PatchSet: 4
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: Alexander Chemeris <Alexander.Chemeris at gmail.com>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Tom Tsou <tom at tsou.cc>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: tnt <tnt at 246tNt.com>
Gerrit-HasComments: Yes


More information about the gerrit-log mailing list