[Asterisk-code-review] translate: Re-enables native Packet-Loss Concealment (PLC) f... (asterisk[master])

[Jaco Kroon]

Jaco Kroon has posted comments on this change.

Change subject: translate: Re-enables native Packet-Loss Concealment (PLC) for iLBC and Speex.
......................................................................


Patch Set 9:

(2 comments)

https://gerrit.asterisk.org/#/c/1820/9/main/translate.c
File main/translate.c:

PS9, Line 548: /* Out-of-order packet - more precise: late packet */
> Not unless jitter buffering occurs prior to this point - and jitter bufferi
But we're not guaranteed of jitter buffering at all, and this still only considers the first frame being received, even with JB we could receive eg frames 1, 2, 4 and 5 in a single go, so we need to insert an empty frame for 3 which this patch won't do.

So I really thing this needs to be done for each frame in the list separately.  I should have some time today, will see if I can possibly propose some actual alternative code instead of just hand-wave suggestions.


PS9, Line 617: temp = ast_frdup(&missed); /* does not duplicate LIST_NEXT */
> I think I'm ok with this patch as long as we get the potential memory leak 
How about just pushing these frames into path right here?  why queue them for later?  Can't we then just call framein(path, &temp) right here?  Just reverse the direction of the loop, in other words, we know the number of missing frames, and for that count iterate deducting the remaining number of missing frames from the incoming frame's sequency number (modulo 2^16)?  That way we can even avoid the memory allocation completely.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfcf0670e094e9718d82fd9920f1fb2dae122006
Gerrit-PatchSet: 9
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Jaco Kroon <jaco at uls.co.za>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes