pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29516 )
Change subject: cosmetic: osmux: Fix typo in comment
......................................................................
cosmetic: osmux: Fix typo in comment
Change-Id: I05aa1941f9cc8f3aa5ba873cf134767b56b56b69
---
M src/osmux.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/16/29516/1
diff --git a/src/osmux.c b/src/osmux.c
index 991fd33..de59d2f 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -152,7 +152,7 @@
/* rtp packet with the marker bit is always guaranteed to be the first
* one. We want to notify with marker in 2 scenarios:
* 1- Sender told us through osmux frame rtp_m.
- * 2- Sntermediate osmux frame lost (seq gap), otherwise rtp receiver only sees
+ * 2- Intermediate osmux frame lost (seq gap), otherwise rtp receiver only sees
* steady increase of delay
*/
rtph->marker = first_pkt &&
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29516
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I05aa1941f9cc8f3aa5ba873cf134767b56b56b69
Gerrit-Change-Number: 29516
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29518 )
Change subject: osmux: Fix osmux seqnum incremented globally instead of per circuit
......................................................................
osmux: Fix osmux seqnum incremented globally instead of per circuit
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
---
M src/osmux.c
M tests/osmux/osmux_test.c
M tests/osmux/osmux_test.ok
3 files changed, 588 insertions(+), 587 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/18/29518/1
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29518
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I562de6a871d8a35475c314ca107c2a12b55d6683
Gerrit-Change-Number: 29518
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29517 )
Change subject: osmux: Mark unused osmux_in_handle field as deprecated
......................................................................
osmux: Mark unused osmux_in_handle field as deprecated
It is not used anywhere, and it shouldn't be used since the seqnum state
is per circuit. It cannot be dropped since it would break ABI.
Change-Id: Ia26fcba5d7364a5744b2d64d0542a2b3880eee34
---
M include/osmocom/netif/osmux.h
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/17/29517/1
diff --git a/include/osmocom/netif/osmux.h b/include/osmocom/netif/osmux.h
index c663c1b..373b86d 100644
--- a/include/osmocom/netif/osmux.h
+++ b/include/osmocom/netif/osmux.h
@@ -57,7 +57,7 @@
/* one to handle all existing RTP flows */
struct osmux_in_handle {
- uint8_t osmux_seq;
+ uint8_t osmux_seq; /* Unused, DEPRECATED */
uint8_t batch_factor;
uint16_t batch_size;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29517
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Ia26fcba5d7364a5744b2d64d0542a2b3880eee34
Gerrit-Change-Number: 29517
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29512 )
Change subject: osmux: Proper encoding of osmux frames when when AMR FT changes
......................................................................
osmux: Proper encoding of osmux frames when when AMR FT changes
The AMR FT (and hence payload content and size) may well change over the
lifetime of an RTP stream. Since all AMR payloads in an osmux batch
share the same FT (its joined payload is calculated based on
osmuxh->ctr*amr_bytes((osmuxh->amr_ft)), if a new RTP/AMR with a
different FT arrives we cannot simply append it to the current batch.
Instead, the current batch must be considered done and a new batch with
anew osmuxh header must be prepared for the resulting osmux frame to
send over UDP.
Before this patch, when a packet with a different AMR FT was submitted
for batching, it would be added in the same batch and decoding would
fail since the sizes of the batches would be wrong.
Related: SYS#5987
Change-Id: I25eb6ee54c898f575cc71ccfd6d190fe51d56dbe
---
M src/osmux.c
1 file changed, 52 insertions(+), 29 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/12/29512/1
diff --git a/src/osmux.c b/src/osmux.c
index ac3cd06..07d2b2e 100644
--- a/src/osmux.c
+++ b/src/osmux.c
@@ -398,23 +398,6 @@
return 0;
}
-static int osmux_xfrm_encode_amr(struct osmux_batch *batch,
- struct osmux_input_state *state)
-{
- uint32_t amr_len;
-
- state->amrh = osmo_rtp_get_payload(state->rtph, state->msg, &amr_len);
- if (state->amrh == NULL)
- return -1;
-
- state->amr_payload_len = amr_len - sizeof(struct amr_hdr);
-
- if (osmux_batch_put(batch, state) < 0)
- return -1;
-
- return 0;
-}
-
static void osmux_encode_dummy(struct osmux_batch *batch, uint8_t batch_factor,
struct osmux_input_state *state)
{
@@ -457,6 +440,7 @@
llist_for_each_entry(circuit, &batch->circuit_list, head) {
struct msgb *cur, *tmp;
int ctr = 0;
+ int prev_amr_ft;
if (circuit->dummy) {
struct osmux_input_state state = {
@@ -473,6 +457,7 @@
.out_msg = batch_msg,
.ccid = circuit->ccid,
};
+ uint32_t amr_len;
#ifdef DEBUG_MSG
char buf[4096];
@@ -482,20 +467,31 @@
#endif
state.rtph = osmo_rtp_get_hdr(cur);
- if (state.rtph == NULL)
+ if (!state.rtph)
return NULL;
+ state.amrh = osmo_rtp_get_payload(state.rtph, state.msg, &amr_len);
+ if (!state.amrh)
+ return NULL;
+ state.amr_payload_len = amr_len - sizeof(struct amr_hdr);
if (ctr == 0) {
#ifdef DEBUG_MSG
- LOGP(DLMUX, LOGL_DEBUG, "add osmux header\n");
+ LOGP(DLMUX, LOGL_DEBUG, "Add osmux header (First in batch)\n");
+#endif
+ state.add_osmux_hdr = 1;
+ } else if (prev_amr_ft != state.amrh->ft) {
+ /* If AMR FT changed, we have to generate an extra batch osmux header: */
+#ifdef DEBUG_MSG
+ LOGP(DLMUX, LOGL_DEBUG, "Add osmux header (New AMR FT)\n");
#endif
state.add_osmux_hdr = 1;
}
- osmux_xfrm_encode_amr(batch, &state);
+ osmux_batch_put(batch, &state);
osmux_batch_dequeue(cur, circuit);
- msgb_free(cur);
+ prev_amr_ft = state.amrh->ft;
ctr++;
+ msgb_free(cur);
batch->nmsgs--;
}
}
@@ -536,16 +532,10 @@
osmux_xfrm_input_deliver(h);
}
-static int osmux_rtp_amr_payload_len(struct msgb *msg, struct rtp_hdr *rtph)
+static int osmux_rtp_amr_payload_len(struct amr_hdr *amrh, uint32_t amr_len)
{
- struct amr_hdr *amrh;
- unsigned int amr_len;
int amr_payload_len;
- amrh = osmo_rtp_get_payload(rtph, msg, &amr_len);
- if (amrh == NULL)
- return -1;
-
if (!osmo_amr_ft_valid(amrh->ft))
return -1;
@@ -561,6 +551,28 @@
return amr_payload_len;
}
+/* Last stored AMR FT to be added in the current osmux batch. -1 if none stored yet */
+static int osmux_circuit_get_last_stored_amr_ft(struct osmux_circuit *circuit)
+{
+ struct msgb *last;
+ struct rtp_hdr *rtph;
+ struct amr_hdr *amrh;
+ uint32_t amr_len;
+ /* Have we seen any RTP packet in this batch before? */
+ if (llist_empty(&circuit->msg_list))
+ return -1;
+ OSMO_ASSERT(circuit->nmsgs > 0);
+
+ /* Get last RTP packet seen in this batch */
+ last = llist_entry(circuit->msg_list.prev, struct msgb, list);
+ rtph = osmo_rtp_get_hdr(last);
+ amrh = osmo_rtp_get_payload(rtph, last, &amr_len);
+ if (amrh == NULL)
+ return -1;
+ return amrh->ft;
+
+}
+
/* returns: 1 if batch is full, 0 if batch still not full, negative on error. */
static int osmux_replay_lost_packets(struct osmux_circuit *circuit,
struct rtp_hdr *cur_rtph, int batch_factor)
@@ -683,6 +695,8 @@
struct osmux_circuit *circuit;
struct msgb *cur;
int rc;
+ struct amr_hdr *amrh;
+ uint32_t amr_len;
circuit = osmux_batch_find_circuit(batch, ccid);
if (!circuit)
@@ -693,7 +707,11 @@
circuit->dummy = 0;
batch->ndummy--;
}
- amr_payload_len = osmux_rtp_amr_payload_len(msg, rtph);
+
+ amrh = osmo_rtp_get_payload(rtph, msg, &amr_len);
+ if (amrh == NULL)
+ return -1;
+ amr_payload_len = osmux_rtp_amr_payload_len(amrh, amr_len);
if (amr_payload_len < 0) {
LOGP(DLMUX, LOGL_NOTICE, "AMR payload invalid\n");
return -1;
@@ -723,8 +741,13 @@
}
/* First check if there is room for this message in the batch */
+ /* First in batch comes after the batch header: */
if (circuit->nmsgs == 0)
bytes += sizeof(struct osmux_hdr);
+ /* If AMR FT changes in the middle of current batch a new header is
+ * required to adapt to size change: */
+ else if (osmux_circuit_get_last_stored_amr_ft(circuit) != amrh->ft)
+ bytes += sizeof(struct osmux_hdr);
bytes += amr_payload_len;
/* No room, sorry. You'll have to retry */
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29512
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I25eb6ee54c898f575cc71ccfd6d190fe51d56dbe
Gerrit-Change-Number: 29512
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange