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?