diff mbox

libosmo-abis[master]: Extend osmo_rtp_send_frame API

Message ID gerrit.1463579680231.I23e6dccfad5643e662391a0a2abebbb45597ffd9@gerrit.osmocom.org
State New
Headers show

Commit Message

gerrit-no-reply@lists.osmocom.org May 18, 2016, 1:54 p.m. UTC 
Review at  https://gerrit.osmocom.org/82

Extend osmo_rtp_send_frame API

Add boolean parameter to osmo_rtp_send_frame() to explicitly set marker
bit in RTP header. Previously it was always unset which resulted in
degradation of speech quality for codecs with explicit talkspurt
events (was tested with AMR's ONSET).

Related: OS#1562
Change-Id: I23e6dccfad5643e662391a0a2abebbb45597ffd9
---
M TODO-RELEASE
M include/osmocom/trau/osmo_ortp.h
M src/trau/osmo_ortp.c
3 files changed, 8 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/82/82/1

Comments

gerrit-no-reply@lists.osmocom.org May 31, 2016, 12:25 p.m. UTC | #1 
Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/#/c/82/1/include/osmocom/trau/osmo_ortp.h
File include/osmocom/trau/osmo_ortp.h:

PS1, Line 69: smo_rtp_send_frame(struct osmo_rtp_socket *rs, const uint8_t *payload,
            : 			unsigned int payload_len, unsigned int duration,
            : 			bool marker);
The question is, do we want to break API+ABI deliberately to add this argument, or do we rather add a new osmo_rtp_send_frame2() function that has the new argument?  This way we can support both old and new openbsc/osmo-bts from one given (new) libosmo-abis installation.
gerrit-no-reply@lists.osmocom.org May 31, 2016, 12:48 p.m. UTC | #2 
Patch Set 1:

Probably new function is better - only AMR needs this change and I'm not sure if it's absolutely necessary or we'll find a way to make it work without in the end.
diff mbox

Patch

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 43b1e8e..2ba3b29 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -1 +1,2 @@ 
 #library	what		description / commit summary line
+libosmo-abis	API		change signature of osmo_rtp_send_frame
diff --git a/include/osmocom/trau/osmo_ortp.h b/include/osmocom/trau/osmo_ortp.h
index 9512759..58d2860 100644
--- a/include/osmocom/trau/osmo_ortp.h
+++ b/include/osmocom/trau/osmo_ortp.h
@@ -2,6 +2,7 @@ 
 #define _OSMO_ORTP_H
 
 #include <stdint.h>
+#include <stdbool.h>
 
 #include <osmocom/core/linuxlist.h>
 #include <osmocom/core/select.h>
@@ -66,7 +67,8 @@ 
 int osmo_rtp_socket_set_pt(struct osmo_rtp_socket *rs, int payload_type);
 int osmo_rtp_socket_free(struct osmo_rtp_socket *rs);
 int osmo_rtp_send_frame(struct osmo_rtp_socket *rs, const uint8_t *payload,
-			unsigned int payload_len, unsigned int duration);
+			unsigned int payload_len, unsigned int duration,
+			bool marker);
 int osmo_rtp_socket_poll(struct osmo_rtp_socket *rs);
 
 int osmo_rtp_get_bound_ip_port(struct osmo_rtp_socket *rs,
diff --git a/src/trau/osmo_ortp.c b/src/trau/osmo_ortp.c
index 3313798..cb167d7 100644
--- a/src/trau/osmo_ortp.c
+++ b/src/trau/osmo_ortp.c
@@ -23,6 +23,7 @@ 
  */
 
 #include <stdint.h>
+#include <stdbool.h>
 #include <inttypes.h>
 #include <netdb.h>
 
@@ -415,7 +416,8 @@ 
  *  \returns 0 on success, <0 in case of error.
  */
 int osmo_rtp_send_frame(struct osmo_rtp_socket *rs, const uint8_t *payload,
-			unsigned int payload_len, unsigned int duration)
+			unsigned int payload_len, unsigned int duration,
+			bool marker)
 {
 	mblk_t *mblk;
 	int rc;
@@ -428,6 +430,7 @@ 
 	if (!mblk)
 		return -ENOMEM;
 
+	rtp_set_markbit(mblk, marker);
 	rc = rtp_session_sendm_with_ts(rs->sess, mblk,
 				       rs->tx_timestamp);
 	rs->tx_timestamp += duration;