22 Nov
2013
22 Nov
'13
10:19 a.m.
On 11/21/2013 08:30 PM, Holger Hans Peter Freyther wrote:
It does ;-)
The latter, but dSeqNo == 0 => dTS == 0 should be checked, too.
Yes, I didn't find a clean solution without code duplication yesterday.
But I think it should be possible by moving this into a separate
function which does checking and tsdelta computation. This makes it
easier to improve the tests (e.g. the dSeqNo check above or more complex
packet duration stuff) and should help to clean up mgcp_patch_and_count,
too.
I'll update the patch accordingly.
Jacob
On Thu, Nov 21, 2013 at 07:05:43PM +0100, Jacob
Erlbeck wrote:
Thank you for the patch. I have one wish for you to move the
code to separate method (with the side effect on the data structure).
Ok, see remark at the end of the mail.
+ } else {
I would prefer if we could move this code into another method and
re-use it later on. Sure this requires to pass "Input"/"Output" into
the function and the right counters. But I think I prefer this over
the code clone.
You just happened to be late to if/elseif/else and I think it is
getting a bit crowded in this method. + /* Check again, whether the timestamps are
still valid */
+ if (last_tsdelta && seq != state->max_seq) {
seq == state->max_seq => retransmission or just the guard against
the division by 0? + tsdelta = (timestamp -
state->last_timestamp) /
+ (seq - state->max_seq);
okay subtle change as the timestamp_offset has already been applied? My
idea of taking this code out might be difficult. Can you try anyway?