[Asterisk-code-review] translate: Fix transcoding while different in frame size. (asterisk[11])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: translate: Fix transcoding while different in frame size.
......................................................................


Patch Set 3: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/1156/3/codecs/codec_gsm.c
File codecs/codec_gsm.c:

Line 154: 		if (pvt->samples) {
        : 			memmove(tmp->buf, tmp->buf + GSM_SAMPLES, pvt->samples * 2);
        : 		}
This should not need to be done for every time through the loop.  If tmp->buf has more than two frames worth of data in it you wind up moving the samples more than once.  Moving memory can get expensive.  You should only need to do the memmove() once when the while loop has completed on any remaining bits of buffer.

This finding applies to the other codecs you have modified as well since you have changed the code in a similar manner.


https://gerrit.asterisk.org/#/c/1156/3/codecs/codec_ilbc.c
File codecs/codec_ilbc.c:

Line 169: 		if (pvt->samples) {
        : 			memmove(tmp->buf, tmp->buf + ILBC_SAMPLES, pvt->samples * 2);
        : 		}
Same here about moving memory more than once.


https://gerrit.asterisk.org/#/c/1156/3/codecs/codec_lpc10.c
File codecs/codec_lpc10.c:

Line 181: 		if (pvt->samples) {
        : 			memmove(tmp->buf, tmp->buf + LPC10_SAMPLES_PER_FRAME, pvt->samples * 2);
        : 		}
        : 
Same here about moving memory more than once.


https://gerrit.asterisk.org/#/c/1156/3/codecs/codec_speex.c
File codecs/codec_speex.c:

Line 297: 		if (pvt->samples) {
        : 			memmove(tmp->buf, tmp->buf + tmp->framesize, pvt->samples * 2);
        : 		}
Same here about moving memory more than once.


Line 308: 		} else if (!tmp->silent_state) {
        : 			tmp->silent_state = 1;
        : 			speex_bits_reset(&tmp->bits);
        : 			memset(&pvt->f, 0, sizeof(pvt->f));
        : 			pvt->f.frametype = AST_FRAME_CNG;
        : 			pvt->f.samples = tmp->framesize;
        : 			/* XXX what now ? format etc... */
        : 		}
If this path is taken you are messing up the frame list because you don't have a new current created to add to the list.  You need to ast_frisolate() a new current from pvt->f for the AST_FRAME_CNG.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I84b59f7a745955820f10e20f5999eb69495a68b9
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes