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).
+ } 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.
+ /* Compute current per-packet timestamp delta */
+ if (state->initialized && seq != state->max_seq) {
state->initialized == TRUE in this else branc.
+ int corr_timestamp = state->timestamp_offset +
timestamp;
+ tsdelta =
+ (int32_t)(corr_timestamp - state->last_timestamp) /
+ (int16_t)(seq - state->max_seq);
great!
+ /* 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?