Previously payload_type was always hardcoded to 98 for generated rtp
packets from incoming osmux frame.
Change-Id: I5cbeb494a8932953d9fd2dc24dacf8cd97fd84e4
This message is expected as all code filling batches call
osmux_batch_enqueue() and checks for error to know if it must tell the
user of the lib to call osmux_xfrm_input_deliver.
Change-Id: I3d8227f2281f6ca92fd2502d3e328765dc7ecfe9
In Change-Id: I2efed6d726a1b8e77e686c7a5fe1940d3f4901a7 we're adding a
new member to 'struct osmux_out_handle' which is not initialized.
Rather than initializing this single new member, let's do a memset()
over the entire osmux_out_handle at the beginnign of
osmux_xfrm_output_init().
Change-Id: I751e9414c6de2413a9f977e5ae5655ebfd114f45
Closes: OS#3219
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
With old implementation, in conditions with jitter we could end up
scheduling RTP generated packets from two consecutive osmux frames in an
interleaved way (from seq field point of view).
This new implementation should make it easier for any RTP
reader/playback to have better results in those conditions.
Old APIs osmux_xfm_output and osmux_tx_sched are marked as deprecated in
favour of the new one, which has a better control of generated RTP
packets. However, they are still usable despite the implementation changes
done to support the new API.
Related: OS#3180
Change-Id: I4e05ff141eb4041128ae77812bbcfe84ed4c02de
Previous implementation handled all types as if they were Osmux AMR
frames. For Dummy frames, we account the padding but we don't care about
the padding content. For Signalling ones, as they are not in the
specification yet, it is better avoid using unespecified fields and
return an error because it's still not known how extra data will be
handled in the input msgb.
Change-Id: I48565472b47c2a0e5db50881fbb005537af8c70d
The current code still expects to parse only AMR osmux frames, but that
will be fixed in following patches.
Change-Id: Ic2f4d1d3cc88af912bb43c8ecd90eacc6ff7190f
This patch inconditionally initializes the buffer we get to
nul-terminate it, whenever possible. It's a very simple solution to
catch three overly corner cases:
1) snprintf() returns -1, very much unlikely in practise.
2) msg->len == 0: In such case, I would expect this function is never
called with an empty message, but let's be safe in this case too.
3) If your buffer is empty, it doesn't nul-terminate the buffer.
Change-Id: I97e517f2d98e83894ea707c63489559302ff6bd2
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
This commit should fix a bug present if for instance batch_factor < 8
and osmux_batch_enqueue is called from osmux_replay_lost_packets and
enough packets were lost from last received packet.
Change-Id: I5d643810949aeca4762f0cad05eed534d35087f7
Use this function in osmux_batch_del_circuit() since msgs are stored in a list
per circuit. After the circuit is free()d the msgs are lost.
Before this patch any messages enqueued inside a batch when the circiut is
deleted were leaking.
Change-Id: Ib0311652183332d0475bf7347023d518d38487ef
Ticket: OS#1733
Reviewed-on: https://gerrit.osmocom.org/120
Tested-by: Jenkins Builder
Reviewed-by: Holger Freyther <holger@freyther.de>
This new function allows you to create a circuit on an existing input handle.
We don't create the circuit anymore from the first packet seen, instead the
client application is in full control of opening and closing the circuit.
This change includes a new feature to pad a circuit until we see the first
packet that contains voice data. This is useful to preallocate bandwidth on
satellite links such as Iridium/OpenPort.
Add this new function to explicitly remove an existing circuit. Thus, the
client application (openbsc) is in full control to release circuits.
Before this patch, the circuit object was added when the first RTP messages was
seen, and it was removed when transforming the list of pending RTP messages to
the Osmux message (once the timer expired).
Moreover, check circuit->nmsgs to account bytes that are consumed by the osmux
header, given that !circuit doesn't mean "this is the first packet" anymore.
Add a new field to count the number of messages in the batch that are pending
to be transformed to osmux. Use this new field to check when to enable the
osmux timer.
The follow up patch keeps circuit objects in the batch until they are closed,
so we won't be able to rely on this to know when to enable the timer anymore.
I think this is a better name for this object. Basically, an input handle
represents a batch that is composed of one or more circuit objects.
Each circuit object contains a list of RTP messages that are pending to be
converted to the osmux format in one single batch.
This new structure serves as container of the internal osmux state during
transformation from RTP AMR to the compressed osmux format.
This reduces the footprint of several functions and it makes them easier to
extend if we need to pass new information between functions.
Make sure we don't release a osmux handle with an armed batch timer.
==9753== Invalid read of size 4
==9753== at 0x4043CA2: rb_first (rbtree.c:293)
==9753== by 0x403E172: osmo_timers_check (timer.c:256)
==9753== by 0x403E69E: osmo_select_main (select.c:124)
==9753== by 0x804EBD3: main (bsc_nat.c:1613)
==9753== Address 0x4302670 is 56 bytes inside a block of size 108 free'd
==9753== at 0x4023B6A: free (vg_replace_malloc.c:366)
==9753== by 0x40494CF: talloc_free (talloc.c:609)
==9753== by 0x40AB279: osmux_xfrm_input_fini (osmux.c:620)
==9753== by 0x80651FC: osmux_disable_endpoint (mgcp_osmux.c:96)
==9753== by 0x805CAFF: mgcp_release_endp (mgcp_protocol.c:1540)
==9753== by 0x805DD35: handle_delete_con (mgcp_protocol.c:1274)
==9753== by 0x805E998: mgcp_handle_message (mgcp_protocol.c:415)
==9753== by 0x804CFF1: mgcp_do_read (bsc_mgcp_utils.c:972)
==9753== by 0x403F96D: osmo_wqueue_bfd_cb (write_queue.c:48)
==9753== by 0x403E724: osmo_select_main (select.c:158)
==9753== by 0x804EBD3: main (bsc_nat.c:1613)
If it is not possible to put the RTP message into the batch in case that:
1) The message is malformed.
2) The message is duplicated.
3) OOM.
4) The batch is already full.
Osmux releases the messages and gets back to the upper layer with an OK
to give another chance to follow up RTP messages.
This patch also fixes a use-after-free that was possible when the batch
was full. The message was released and osmux_batch_add() was returning 0
osmux_xfrm_input(), which resulted in accessing the released message
when updating the statistics.
This is ugly, with many ifdefs, but they don't want any debug message
that can be spamming. I don't want to remove these because there is
no dissector for osmux, and this can be useful for troubleshooting.
Should be enabled only in case you consider that Osmux should
is lagging when reconstructing RTP messages. I haven't seen
any issue regarding this so far. Let's disable it by default.
This patch adds a new field to the struct osmux_in_handle that allows
you to specify the osmux frame size. If not specified, the default
size assumes your nic uses a mtu of 1500 bytes.
Pau Espin
Harald Welte
Pablo Neira Ayuso
Daniel Willmann