Previously, if RTP jumps were detected in the incoming RTP stream and
osmux state for that circuit was to start the next batch, the hole would
not been filled during queueing time and instead the encoder would have
set the M bit in the osmuxhdr to announce a sync point.
For small holes (eg less than the batch factor) it makes sense to start
filling the batch with crafted RTP packets in order to avoid the encoder
later on setting the M bit and hence avoid the peer receiving the Osmux
frame having to start a new syncrhonization point.
Related: SYS#6161
Change-Id: I9596501adf5b7b91983618c92c7b1792ee9461a3
So far only small intra-batch seqnum jumps are filled in with forged RTP
packets when storing the incoming RTP packets.
Under some conditions, holes may still exist in the queue of RTP packets
for a stream:
* Seqnum detected when first incoming RTP in batch is queued (this can
be improved in the future).
* Big seqnum jumps > batch_factor or simply filling out of bounds for currently
enqueued batch.
Specially the second case can come from long network dropouts, or simply
due to a bug in the RTP being feed to osmux layer (be it from local code
or peer). In that case (long jumps) we don't want to generate tons of
packets filling in several entire batches (potentially incredibly big
amount of batches).
Instead, in these scenarios, simply let the osmux peer know there was a
jump by setting the M bit on the next osmux header for that circuit
after the seq jump has been detected.
Related: SYS#6161
Change-Id: I05b1eae400cb60d1f4e927f853619d5ff470163f
osmux_link_add() is renamed to osmux_link_handle_rtp_req(), and the last
part of it is split out and kept as osmux_link_add().
hence osmux_link_handle_rtp_req() does proper input checking (like
duplicates, holes, etc.) while osmux_link_add() expects all that to be
sorted out.
Reuse osmux_link_add() in osmux_replay_lost_packets() to properly update
the link state of the to-be-transmitted packet, circuit state, etc.
Change-Id: I4ea435bfb2490a375ad3e5068ee926e48b53cf5c
This test shows that there's 2 bugs in the osmux_input code:
* It should be transmitting 2 osmux frames instead of 1
* Once it is fixed to transmitt 2 osmux frames, it should set the M bit of the 2nd
generated osmux header after the seqnum jump in order to announce a jump in the
stream to the peer receiving osmux.
The bugs are fixed in follow-up patches.
Related: SYS#6161
Change-Id: I521c2e97a739e8a824b16f06ec2a578333388247
When RTP packet provided by user to osmux layer, it may contain a seq
gap, and osmux will refill the packets to avoid losing that information
on the other side when it receives the batch.
If out of order UDP packets are received, it can happen that we first
detect a gap and later on we receive the previous RTP packet, which we
is then detected as duplicate because it was previously forged to fill
the hole. In that case, let's better keep the incoming packet instead of
the potentially internall forged one which doesn't contain the real AMR
payload.
Related: SYS#6161
Change-Id: I82e11ef3dcd20ffea33c94ed83abcedf0f186871
It was not being used anywhere, yet older applications used to set it
(always to 0, which was the default value applied internally).
Let's make use of it and apply it as first seqnum to be used on a
circuit.
This value is applied upon call to osmux_xfrm_input_open_circuit(),
hence it can be set independently for every new circuit.
Change-Id: Ia26fcba5d7364a5744b2d64d0542a2b3880eee34
The wrap around case was not properly considered in the condition
setting the Marker bit. Let's fix it.
Related: SYS#5987
Change-Id: I6e01f29a6239f930c9be2bcb2efe447e5de8fedf
This test shows that there's a bug where the first RTP packet extracted
from the received osmux batch with seqnum=0 has the M bit marked for no
good reason.
Related: SYS#5987
Change-Id: Ida658c681e84878f209ab4965d8aa821a570a580
There's no real use in having a global seqnum. The seqnum, as specified
by the osmux protocol specification, relates to that of the RTP
seq+timestamp, hence it is per circuit.
Having it per circuit allows detecting gaps and lost batches on the
receiver side, applying the Marker bit on the re-assembled RTP packets.
As a resut of the fix, tests/osmux/osmux_test part validating Marker bit
started failing. It failed because it was wrongly written to start with,
since it used only one osmux_out_handle for the 4 CIDs in use, which was
wrong, since each CID must have its own osmux_out_handle as state is
kept there per circuit.
Related: SYS#5987
Change-Id: I562de6a871d8a35475c314ca107c2a12b55d6683
The value of the first RTP packet in the batch was being encoded,
instead of the last one (as documented in the Osmux protocol
specification).
Related: SYS#5987
Change-Id: I06f3d07a05181d938c22bbd0da7172b5dad6659a
That file focuses on testing the osmux_output_* API, ie receiving Osmux
frames and decoding it into batches and later on into rtp packets.
Change-Id: I7e6eda1e2e676673771bc6a7c3682d07ac1d344e
Let's not make the tests more difficult to maintain and extend by
allowing them to run in real clock time, there's no real need for it.
Change-Id: I549a4722d63d700b54b146260131a68e656b843e
Until now, the osmux_out_handle was allocated by the client, and passed
to the API to initialize it. This makes it really hard to improve the
implementation without breaking the ABI.
Let's break the ABI now one last time (hopefully) by allocating the
struct through an API. With only this change, the already built users
(osmo-mgw, openbsc) can still work fine, since there's no change on the
struct osmux_out_handle. However, they will somehow break next time the
struct is changed until they are ported to the same API (easy to do).
Related: OS#5987
Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34
osmux implementation randomizes those values. It seems build in OBS
sometimes provide different values than the ones expected in the test
result. Let's hardcode them to make sure we always have the same values
regarless of the random() implementation.
Values chosen are the one matching the current expected test output so
it doesn't need any change.
Change-Id: Icc553c83ddff41900ae3d5990a655c29c9073e01
Currently osmux related code uses both gettimeofday on some parts and
clock_gettime(CLOCK_MONOTONIC) on others (for different purposes).
Let's fake both clocks and not only the one used by gettimeofday API.
Change-Id: I4e1a0eb4f8c4ea1bc0f963afa18b116d3af9978c
Previously payload_type was always hardcoded to 98 for generated rtp
packets from incoming osmux frame.
Change-Id: I5cbeb494a8932953d9fd2dc24dacf8cd97fd84e4
Until this patch, we didn't notify in any way to the RTP reader when an
Osmux frame was lost. Instead, we updated the seq×tamp as if there
was no lost, and as a result the RTP reader would only see a steady
increase of delay every time an osmux frame was lost.
As the batch_factor for the lost packet is unknown, we cannot assume any
number of amr payloads lost, and thus we cannot simply increment seq and
timestamp for a specific amount. Instead, the only viable solution seems
to set the M marker bit in the first rtp packet generated after a
non-consecutive osmux frame is received.
The implementation may act differently with the first generated RTP
packet based on the first osmux seq number used for the stream. In case
0 it's used as first osmux seq number, M will be set depending on
request from original RTP packet having the M bit set. If it's not 0,
the first RTP packer will unconditionally have the M bit. That's not an
issue because it's anyway expect for receiver to sync on the first
packet.
Related: OS#3185
Change-Id: I2efed6d726a1b8e77e686c7a5fe1940d3f4901a7
This test is aimed at testing several specific scenarios related to how
osmux manages in/out of osmux/rtp packets over time.
Change-Id: I3bf59276424ea87c4e66a6ff46de1e3e9a06a904
SNPRINTF_BUFFER_SIZE() looks too complex, previous version maintains two
different variables to account for the remaining space in the buffer,
one of them is always decremented based on what snprintf() returns,
which may result in underflow. These variables are swapped - not used
consistently - all over this code.
Replace this macro by a simplified version, with one single parameter to
account for remaining space. This macro also deals with two corner
cases:
1) snprintf() fails, actually never happens in practise, but
documentation indicates it may return -1, so let's catch this case
from here to stick to specs.
2) There is not enough space in the buffer, in that case, keep
increasing offset, so we know how much would have been printed, just
like snprintf() does.
Thanks to Pau Espin for reporting, and Holger for clues on this.
I have run osmux_test and, at quick glance, it looks good.
Change-Id: I5b5d6ec57a02f57c23b1ae86dbd894bad28ea797
The buffer for osmux_test is increased as the former doesn't seem to be
able to cope with the whole output.
Change-Id: Ic838dd9d7ad89b4510ccfa58c0390c69a075b616
According to RFC4867 (RTP payload format for AMR):
"The RTP header marker bit (M) SHALL be set to 1 if the first frameblock
carried in the packet contains a speech frame which is the first in a
talkspurt. For all other packets the marker bit SHALL be set to zero (M=0)."
This information bit provides a way for the receiver to better
synchronize the delay with ther sender.
This is specially useful if AMR DTX features are supported and
enabled on the sender.
Change-Id: I0315658159429603f1d80a168718b026015060e9
Introduce a local #define to disable the real-time constraint from osmux-test.
It would make sense to remove this completely, but in case anyone may be
interested in the timing on a specific platform, I've just #defined it away.
The real-time constraint to pass or fail the test is a bad idea in terms of our
build server. Whenever the server is loaded, the tests will fail for no reason,
like here: https://gerrit.osmocom.org/474
The real time to calculate is highly dependent also on the hardware platform.
The arbitrarity of the time constraint is sort of proven by dd24cdd95f
which simply doubles the time to pass the test.
Change-Id: Ic1da4bd22411652334f73195b2e37853e0738906
With the fan-out approach to test multi-batch added int (078d532 tests:
osmux: test multi-batch support), this doesn't need the explicit
skip as there are already gaps to be filled.
Pau Espin
Neels Hofmeyr
Neels Hofmeyr