There's no need to have the variable as signed anymore since the loop
was modified in the previous patch. osmo_amr_bytes() returns a size_t
(unsigned).
Change-Id: I8aa6b5f6d3334e152a62b7c28aac3f881f027894
oa_payload_len can be 2 if osmo_amr_bytes() returns 0 (it will return 0
when FT NO_DATA is supported). In tha case, the loop condition
(oa_payload_len - 3) (signed) is compared against unsigned i which ends
up accessing i=256.
Change-Id: I1e513f493d7883a03acbfa3d9744ec63657810b3
Current internal use of APIs marked as deprecated seems to be causing
issues when building on debian unstable.
Simply rearrange the init code to an internal helper function to avoid
code duplication while still keeping the old deprecated APIs working as
they used to, without getting deprecation warnings at buildtime.
Related: OS#5677
Change-Id: Ie8e168740c0421edd96013620256aab0306dc6c5
We implicitly rely on conn->srv availability in several functions.
Let's ensure it's available in function creating conn.
Change-Id: If494eac5dcce6c5ae30e928b92e57067d5681a42
This allows easily identifying and following state and lifecycle of
CIDs when looking at logs.
Related: SYS#6161
Change-Id: I6a3113dfaef0adbb20162985e3b7d57c46dbc016
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
If the last RTP we received from the user is a Marker bit (which is
expected to be the first in the batch), then there's no need to keep
marking the forged RTP packets to fill holes also as Marker bit.
Change-Id: I5cef6185afbfce748473a096e8ebabd9c9628e12
In the event we need to fill in the batch with intermediate lost RTP
packets, we must do so before handling the last one.
Change-Id: Ib5dd7b1fa6213c96ece803b781a7ef1cf102a1d4
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
We need to filter duplicate messages before checking the Mark bit and
returning 1 to tell the user to deliver the current batchset content.
Otherwise, a repeated RTP packet with an M bit would trigger delivery,
which is wrong.
Change-Id: I4a01b0935f112d650d8fc161b3389eda6c8e75ec
This allows incrementally gathering all the information once and only
once. While doing so, this patch tends to move the decoding/parsing of
the incoming RTP packet further towards the start of the code, hence
detecing errors in the input data early in the process and avoiding
touching internal state in this case.
Change-Id: I55e0d2772e7054c0d734f5918c6938a5c8a6e781
A lof of mess is inherited from the fact that there's a public object
(struct osmux_in_handle) which internally uses a private object (struct
osmux_link).
The struct osmux_in_handle fields are no longer expected to be accessed
or allocated by the user, hence the struct is only kept in the header
for backward compatibility with older versions.
At some point, both structs can be moved to the C file and merged,
simplifying the code further.
For now, let's simply add a backpointer from struct osmux_link to the
related struct osmux_in_handle, so that we can pass 1 pointer and have
all fields accessible in the code.
This allows further simplifying the code.
We could also add an extra backpointer from osmux_circuit to osmux_link,
but that's not really estimated as needed as of now since we can easily
pass the osmux_link pointer down the called functions.
Change-Id: I5c226c5c809a240f9eb80aa2e83679adc717d717
This struct matches the lifecycle of the link, holds all the circuits
acting as a trunk. The previous name was utterly misleading since "batch"
usually is per circuit group of AMR frames with an osmux header.
Change-Id: Ib118fd2c1fead78655756156ce5d4ce4a6d080df
Unfortunately "-std=c99" is not sufficient to make gcc ignore code that
uses constructs of earlier C standards, which were abandoned in C99.
See https://lwn.net/ml/fedora-devel/Y1kvF35WozzGBpc8@redhat.com/ for
some related discussion.
Change-Id: Ied224520b9a09743bd19818ab00cbdf915baef34
This API allows retrieving back the private pointer set previously by
osmux_xfrm_input_set_deliver_cb().
Change-Id: I95433b18802f73fa70e758f4aa02128eee940d88
Until now, the osmux_in_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, openbsci, osmo-bts) can still work fine, since there's no
change on the struct osmux_in_handle. However, they will somehow break
next time thestruct is changed until they are ported to the same API
(easy to do).
Change-Id: I752ab031f935f04731bb1a354333f1682a1aa5bd
Related: SYS#5987
Replacing this one with the newer API was missed a few commits ago when
this API was marked as deprectated. Do it now.
Change-Id: Ia0958dfae951d82feafe427eff2112d327d3b0a4