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?
