[PATCH 2/4] mgcp/rtp: Add counter for invalid RTP timestamp deltas

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Holger Hans Peter Freyther holger at freyther.de
Thu Nov 21 19:30:12 UTC 2013


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?





More information about the OpenBSC mailing list