DTX: further AMR SID cache fixes (lc15, sysmo)

* consolidate AMR CMR and CMI handling in common/amr.c
* use it in save_last_sid()
* remove dead code
* properly compute RTP payload length for AMR
* use save_last_sid() for FR & HR as well
* invalidate cached SID if SPEECH frame is received

Fixes: OS #1800, #1801
Change-Id: I5a1c1ad0b0a295a50e67775a4db85f1d331755ed

6 files changed, 133 insertions, 173 deletions

diff --git a/include/osmo-bts/amr.h b/include/osmo-bts/amr.h
index 6bdc41fc..f3132874 100644
--- a/include/osmo-bts/amr.h
+++ b/include/osmo-bts/amr.h
@@ -11,7 +11,8 @@ void amr_log_mr_conf(int ss, int logl, const char *pfx,
 
 int amr_parse_mr_conf(struct amr_multirate_conf *amr_mrc,
 		      const uint8_t *mr_conf, unsigned int len);
-int get_amr_mode_idx(const struct amr_multirate_conf *amr_mrc, uint8_t cmi);
+void amr_set_mode_pref(uint8_t *data, const struct amr_multirate_conf *amr_mrc,
+		       uint8_t cmi, uint8_t cmr);
 unsigned int amr_get_initial_mode(struct gsm_lchan *lchan);
 
 #endif /* _OSMO_BTS_AMR_H */
diff --git a/include/osmo-bts/msg_utils.h b/include/osmo-bts/msg_utils.h
index 14c23205..f07623d5 100644
--- a/include/osmo-bts/msg_utils.h
+++ b/include/osmo-bts/msg_utils.h
@@ -26,8 +26,9 @@ enum {
 };
 
 void lchan_set_marker(bool t, struct gsm_lchan *lchan);
-void save_last_sid(struct gsm_lchan *lchan, uint8_t *l1_payload, size_t length,
-		   uint32_t fn, bool update);
+void save_last_sid(struct gsm_lchan *lchan, const uint8_t *l1_payload,
+		   size_t length, uint32_t fn, int update, uint8_t cmr,
+		   int8_t cmi);
 uint8_t repeat_last_sid(struct gsm_lchan *lchan, uint8_t *dst, uint32_t fn);
 int msg_verify_ipa_structure(struct msgb *msg);
 int msg_verify_oml_structure(struct msgb *msg);
diff --git a/src/common/amr.c b/src/common/amr.c
index 56ed4302..b5d2c371 100644
--- a/src/common/amr.c
+++ b/src/common/amr.c
@@ -22,7 +22,8 @@ void amr_log_mr_conf(int ss, int logl, const char *pfx,
 	LOGPC(ss, logl, "\n");
 }
 
-int get_amr_mode_idx(const struct amr_multirate_conf *amr_mrc, uint8_t cmi)
+static inline int get_amr_mode_idx(const struct amr_multirate_conf *amr_mrc,
+				   uint8_t cmi)
 {
 	unsigned int i;
 	for (i = 0; i < amr_mrc->num_modes; i++) {
@@ -32,6 +33,46 @@ int get_amr_mode_idx(const struct amr_multirate_conf *amr_mrc, uint8_t cmi)
 	return -EINVAL;
 }
 
+static inline uint8_t set_cmr_mode_idx(const struct amr_multirate_conf *amr_mrc,
+				       uint8_t cmr)
+{
+	int rc;
+
+	/* Codec Mode Request is in upper 4 bits of RTP payload header,
+	 * and we simply copy the CMR into the CMC */
+	if (cmr == 0xF) {
+		/* FIXME: we need some state about the last codec mode */
+		return 0;
+	}
+
+	rc = get_amr_mode_idx(amr_mrc, cmr);
+	if (rc < 0) {
+		/* FIXME: we need some state about the last codec mode */
+		LOGP(DRTP, LOGL_INFO, "RTP->L1: overriding CMR %u\n", cmr);
+		return 0;
+	}
+	return rc;
+}
+
+static inline uint8_t set_cmi_mode_idx(const struct amr_multirate_conf *amr_mrc,
+				       uint8_t cmi)
+{
+	int rc = get_amr_mode_idx(amr_mrc, cmi);
+	if (rc < 0) {
+		LOGP(DRTP, LOGL_ERROR, "AMR CMI %u not part of AMR MR set\n",
+		     cmi);
+		return 0;
+	}
+	return rc;
+}
+
+void amr_set_mode_pref(uint8_t *data, const struct amr_multirate_conf *amr_mrc,
+		      uint8_t cmi, uint8_t cmr)
+{
+	data[0] = set_cmi_mode_idx(amr_mrc, cmi);
+	data[1] = set_cmr_mode_idx(amr_mrc, cmr);
+}
+
 /* parse a GSM 04.08 MultiRate Config IE (10.5.2.21aa) in a more
  * comfortable internal data structure */
 int amr_parse_mr_conf(struct amr_multirate_conf *amr_mrc,
diff --git a/src/common/msg_utils.c b/src/common/msg_utils.c
index f5a48a3d..5c6bbbea 100644
--- a/src/common/msg_utils.c
+++ b/src/common/msg_utils.c
@@ -20,6 +20,7 @@
 #include <osmo-bts/msg_utils.h>
 #include <osmo-bts/logging.h>
 #include <osmo-bts/oml.h>
+#include <osmo-bts/amr.h>
 
 #include <osmocom/gsm/protocol/ipaccess.h>
 #include <osmocom/gsm/protocol/gsm_12_21.h>
@@ -99,17 +100,30 @@ void lchan_set_marker(bool t, struct gsm_lchan *lchan)
 }
 
 /* store the last SID frame in lchan context */
-void save_last_sid(struct gsm_lchan *lchan, uint8_t *l1_payload, size_t length,
-		   uint32_t fn, bool update)
+/*! \brief Store the last SID frame in lchan context
+ *  \param[in] lchan Logical channel on which we check scheduling
+ *  \param[in] l1_payload buffer with SID data
+ *  \param[in] length length of l1_payload
+ *  \param[in] fn Frame Number for which we check scheduling
+ *  \param[in] update 0 if SID_FIRST, 1 if SID_UPDATE, -1 if not AMR SID
+ *  \param[in] cmr Codec Mode Request (AMR-specific, ignored otherwise)
+ *  \param[in] cmi Codec Mode Indication (AMR-specific, ignored otherwise)
+ */
+void save_last_sid(struct gsm_lchan *lchan, const uint8_t *l1_payload,
+		   size_t length, uint32_t fn, int update, uint8_t cmr,
+		   int8_t cmi)
 {
-	size_t copy_len = OSMO_MIN(length + 1,
-				   ARRAY_SIZE(lchan->tch.last_sid.buf));
+	size_t amr = (update < 0) ? 0 : 2,
+	    copy_len = OSMO_MIN(length + 1, ARRAY_SIZE(lchan->tch.last_sid.buf));
 
-	lchan->tch.last_sid.len = copy_len;
+	lchan->tch.last_sid.len = copy_len + amr;
 	lchan->tch.last_sid.fn = fn;
 	lchan->tch.last_sid.is_update = update;
 
-	memcpy(lchan->tch.last_sid.buf, l1_payload, copy_len);
+	if (amr)
+		amr_set_mode_pref(lchan->tch.last_sid.buf, &lchan->tch.amr_mr,
+				  cmi, cmr);
+	memcpy(lchan->tch.last_sid.buf + amr, l1_payload, copy_len);
 }
 
 /*! \brief Check if enough time has passed since last SID (if any) to repeat it
diff --git a/src/osmo-bts-litecell15/tch.c b/src/osmo-bts-litecell15/tch.c
index 5ec1ee74..5b5a375d 100644
--- a/src/osmo-bts-litecell15/tch.c
+++ b/src/osmo-bts-litecell15/tch.c
@@ -199,85 +199,13 @@ static struct msgb *l1_to_rtppayload_amr(uint8_t *l1_payload, uint8_t payload_le
  */
 static int rtppayload_to_l1_amr(uint8_t *l1_payload, const uint8_t *rtp_payload,
 				uint8_t payload_len,
-				struct gsm_lchan *lchan, uint32_t fn)
+				struct gsm_lchan *lchan, uint8_t cmr, int8_t cmi,
+				uint8_t ft)
 {
-	struct amr_multirate_conf *amr_mrc = &lchan->tch.amr_mr;
-	enum osmo_amr_type ft;
-	enum osmo_amr_quality bfi;
-	uint8_t cmr;
-	int8_t sti, cmi;
-	uint8_t *l1_cmi_idx = l1_payload;
-	uint8_t *l1_cmr_idx = l1_payload+1;
-	int rc;
-
-	osmo_amr_rtp_dec(rtp_payload, payload_len, &cmr, &cmi, &ft, &bfi, &sti);
 	memcpy(l1_payload+2, rtp_payload, payload_len);
+	amr_set_mode_pref(l1_payload, &lchan->tch.amr_mr, cmi, cmr);
 
-	/* CMI in downlink tells the L1 encoder which encoding function
-	 * it will use, so we have to use the frame type */
-	switch (ft) {
-	case 0: case 1: case 2: case 3:
-	case 4: case 5: case 6: case 7:
-		cmi = ft;
-		LOGP(DRTP, LOGL_DEBUG, "SPEECH frame with CMI %u\n", cmi);
-		break;
-	case AMR_NO_DATA:
-		LOGP(DRTP, LOGL_DEBUG, "SPEECH frame AMR NO_DATA\n");
-		break;
-	case AMR_SID:
-		LOGP(DRTP, LOGL_DEBUG, "SID %s frame with CMI %u\n",
-		     sti ? "UPDATE" : "FIRST", cmi);
-		break;
-	default:
-		LOGP(DRTP, LOGL_ERROR, "unsupported AMR FT 0x%02x\n", ft);
-		return -EINVAL;
-		break;
-	}
-
-	rc = get_amr_mode_idx(amr_mrc, cmi);
-	if (rc < 0) {
-		LOGP(DRTP, LOGL_ERROR, "AMR CMI %u not part of AMR MR set\n",
-			cmi);
-		*l1_cmi_idx = 0;
-	} else
-		*l1_cmi_idx = rc;
-
-	/* Codec Mode Request is in upper 4 bits of RTP payload header,
-	 * and we simply copy the CMR into the CMC */
-	if (cmr == 0xF) {
-		/* FIXME: we need some state about the last codec mode */
-		*l1_cmr_idx = 0;
-	} else {
-		rc = get_amr_mode_idx(amr_mrc, cmr);
-		if (rc < 0) {
-			/* FIXME: we need some state about the last codec mode */
-			LOGP(DRTP, LOGL_INFO, "RTP->L1: overriding CMR %u\n", cmr);
-			*l1_cmr_idx = 0;
-		} else
-			*l1_cmr_idx = rc;
-	}
-#if 0
-	/* check for bad quality indication */
-	if (bfi == AMR_GOOD) {
-		/* obtain frame type from AMR FT */
-		l1_payload[2] = ft;
-	} else {
-		/* bad quality, we should indicate that... */
-		if (ft == AMR_SID) {
-			/* FIXME: Should we do GsmL1_TchPlType_Amr_SidBad? */
-			l1_payload[2] = ft;
-		} else {
-			l1_payload[2] = ft;
-		}
-	}
-#endif
-
-	if (ft == AMR_SID) {
-		save_last_sid(lchan, l1_payload, payload_len, fn, sti);
-		return -EALREADY;
-	}
-
-	return payload_len+1;
+	return payload_len + 2;
 }
 
 #define RTP_MSGB_ALLOC_SIZE	512
@@ -304,6 +232,7 @@ bool l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len,
 	enum osmo_amr_quality bfi;
 	int8_t sti, cmi;
 	int rc;
+	bool is_sid = false;
 
 	DEBUGP(DRTP, "%s RTP IN: %s\n", gsm_lchan_name(lchan),
 		osmo_hexdump(rtp_pl, rtp_pl_len));
@@ -317,32 +246,55 @@ bool l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len,
 			*payload_type = GsmL1_TchPlType_Fr;
 			rc = rtppayload_to_l1_fr(l1_payload,
 						 rtp_pl, rtp_pl_len);
+			if (rc)
+				is_sid = osmo_fr_check_sid(rtp_pl, rtp_pl_len);
 		} else{
 			*payload_type = GsmL1_TchPlType_Hr;
 			rc = rtppayload_to_l1_hr(l1_payload,
 						 rtp_pl, rtp_pl_len);
+			if (rc)
+				is_sid = osmo_hr_check_sid(rtp_pl, rtp_pl_len);
 		}
+		if (is_sid)
+			save_last_sid(lchan, rtp_pl, rtp_pl_len, fn, -1, 0, 0);
 		break;
 	case GSM48_CMODE_SPEECH_EFR:
 		*payload_type = GsmL1_TchPlType_Efr;
 		rc = rtppayload_to_l1_efr(l1_payload, rtp_pl,
 					  rtp_pl_len);
+		/* FIXME: detect and save EFR SID */
 		break;
 	case GSM48_CMODE_SPEECH_AMR:
+		osmo_amr_rtp_dec(rtp_pl, rtp_pl_len, &cmr, &cmi, &ft, &bfi,
+				 &sti);
+		if (ft == AMR_SID) {
+			save_last_sid(lchan, rtp_pl, rtp_pl_len, fn, sti, cmr,
+				      cmi);
+			return false;
+		}
+		if (ft != AMR_NO_DATA && !osmo_amr_is_speech(ft)) {
+			LOGP(DRTP, LOGL_ERROR, "unsupported AMR FT 0x%02x\n",
+			     ft);
+			return false;
+		}
+		if (osmo_amr_is_speech(ft)) {
+			if (lchan->tch.last_sid.len) { /* FIXME: force ONSET */
+				marker = true;
+			}
+			/* We received AMR SPEECH frame - invalidate saved SID */
+			lchan->tch.last_sid.len = 0;
+		}
 		if (marker) {
 			*payload_type = GsmL1_TchPlType_Amr_Onset;
 			rc = 0;
-			osmo_amr_rtp_dec(rtp_pl, rtp_pl_len, &cmr, &cmi, &ft,
-					 &bfi, &sti);
 			LOGP(DRTP, LOGL_ERROR, "Marker SPEECH frame AMR %s\n",
 			     get_value_string(osmo_amr_type_names, ft));
 		}
 		else {
 			*payload_type = GsmL1_TchPlType_Amr;
 			rc = rtppayload_to_l1_amr(l1_payload, rtp_pl,
-						  rtp_pl_len, lchan, fn);
-			if (-EALREADY == rc)
-				return false;
+						  rtp_pl_len, lchan, cmr, cmi,
+						  ft);
 		}
 		break;
 	default:
diff --git a/src/osmo-bts-sysmo/tch.c b/src/osmo-bts-sysmo/tch.c
index 0e4b2dab..655884d2 100644
--- a/src/osmo-bts-sysmo/tch.c
+++ b/src/osmo-bts-sysmo/tch.c
@@ -282,19 +282,9 @@ static struct msgb *l1_to_rtppayload_amr(uint8_t *l1_payload, uint8_t payload_le
  */
 static int rtppayload_to_l1_amr(uint8_t *l1_payload, const uint8_t *rtp_payload,
 				uint8_t payload_len,
-				struct gsm_lchan *lchan, uint32_t fn)
+				struct gsm_lchan *lchan, uint8_t cmr, int8_t cmi,
+				uint8_t ft)
 {
-	struct amr_multirate_conf *amr_mrc = &lchan->tch.amr_mr;
-	enum osmo_amr_type ft;
-	enum osmo_amr_quality bfi;
-	uint8_t cmr;
-	int8_t sti, cmi;
-	uint8_t *l1_cmi_idx = l1_payload;
-	uint8_t *l1_cmr_idx = l1_payload+1;
-	int rc;
-
-	osmo_amr_rtp_dec(rtp_payload, payload_len, &cmr, &cmi, &ft, &bfi, &sti);
-
 #ifdef USE_L1_RTP_MODE
 	memcpy(l1_payload+2, rtp_payload, payload_len);
 #else
@@ -309,72 +299,9 @@ static int rtppayload_to_l1_amr(uint8_t *l1_payload, const uint8_t *rtp_payload,
 	/* lower 4 bit of first FR2 byte contains FT */
 	l1_payload[2] |= ft;
 #endif /* USE_L1_RTP_MODE */
+	amr_set_mode_pref(l1_payload, &lchan->tch.amr_mr, cmi, cmr);
 
-	/* CMI in downlink tells the L1 encoder which encoding function
-	 * it will use, so we have to use the frame type */
-	switch (ft) {
-	case 0: case 1: case 2: case 3:
-	case 4: case 5: case 6: case 7:
-		cmi = ft;
-		LOGP(DRTP, LOGL_DEBUG, "SPEECH frame with CMI %u\n", cmi);
-		break;
-	case AMR_NO_DATA:
-		LOGP(DRTP, LOGL_DEBUG, "SPEECH frame AMR NO_DATA\n");
-		break;
-	case AMR_SID:
-		LOGP(DRTP, LOGL_DEBUG, "SID %s frame with CMI %u\n",
-		     sti ? "UPDATE" : "FIRST", cmi);
-		break;
-	default:
-		LOGP(DRTP, LOGL_ERROR, "unsupported AMR FT 0x%02x\n", ft);
-		return -EINVAL;
-		break;
-	}
-
-	rc = get_amr_mode_idx(amr_mrc, cmi);
-	if (rc < 0) {
-		LOGP(DRTP, LOGL_ERROR, "AMR CMI %u not part of AMR MR set\n",
-			cmi);
-		*l1_cmi_idx = 0;
-	} else
-		*l1_cmi_idx = rc;
-
-	/* Codec Mode Request is in upper 4 bits of RTP payload header,
-	 * and we simply copy the CMR into the CMC */
-	if (cmr == 0xF) {
-		/* FIXME: we need some state about the last codec mode */
-		*l1_cmr_idx = 0;
-	} else {
-		rc = get_amr_mode_idx(amr_mrc, cmr);
-		if (rc < 0) {
-			/* FIXME: we need some state about the last codec mode */
-			LOGP(DRTP, LOGL_INFO, "RTP->L1: overriding CMR %u\n", cmr);
-			*l1_cmr_idx = 0;
-		} else
-			*l1_cmr_idx = rc;
-	}
-#if 0
-	/* check for bad quality indication */
-	if (bfi == AMR_GOOD) {
-		/* obtain frame type from AMR FT */
-		l1_payload[2] = ft;
-	} else {
-		/* bad quality, we should indicate that... */
-		if (ft == AMR_SID) {
-			/* FIXME: Should we do GsmL1_TchPlType_Amr_SidBad? */
-			l1_payload[2] = ft;
-		} else {
-			l1_payload[2] = ft;
-		}
-	}
-#endif
-
-	if (ft == AMR_SID) {
-		save_last_sid(lchan, l1_payload, payload_len, fn, sti);
-		return -EALREADY;
-	}
-
-	return payload_len+1;
+	return payload_len + 2;
 }
 
 #define RTP_MSGB_ALLOC_SIZE	512
@@ -401,6 +328,7 @@ bool l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len,
 	enum osmo_amr_quality bfi;
 	int8_t sti, cmi;
 	int rc;
+	bool is_sid = false;
 
 	DEBUGP(DRTP, "%s RTP IN: %s\n", gsm_lchan_name(lchan),
 		osmo_hexdump(rtp_pl, rtp_pl_len));
@@ -414,34 +342,57 @@ bool l1if_tch_encode(struct gsm_lchan *lchan, uint8_t *data, uint8_t *len,
 			*payload_type = GsmL1_TchPlType_Fr;
 			rc = rtppayload_to_l1_fr(l1_payload,
 						 rtp_pl, rtp_pl_len);
+			if (rc)
+				is_sid = osmo_fr_check_sid(rtp_pl, rtp_pl_len);
 		} else{
 			*payload_type = GsmL1_TchPlType_Hr;
 			rc = rtppayload_to_l1_hr(l1_payload,
 						 rtp_pl, rtp_pl_len);
+			if (rc)
+				is_sid = osmo_hr_check_sid(rtp_pl, rtp_pl_len);
 		}
+		if (is_sid)
+			save_last_sid(lchan, rtp_pl, rtp_pl_len, fn, -1, 0, 0);
 		break;
 #if defined(L1_HAS_EFR) && defined(USE_L1_RTP_MODE)
 	case GSM48_CMODE_SPEECH_EFR:
 		*payload_type = GsmL1_TchPlType_Efr;
 		rc = rtppayload_to_l1_efr(l1_payload, rtp_pl,
 					  rtp_pl_len);
+		/* FIXME: detect and save EFR SID */
 		break;
 #endif
 	case GSM48_CMODE_SPEECH_AMR:
+		osmo_amr_rtp_dec(rtp_pl, rtp_pl_len, &cmr, &cmi, &ft, &bfi,
+				 &sti);
+		if (ft == AMR_SID) {
+			save_last_sid(lchan, rtp_pl, rtp_pl_len, fn, sti, cmr,
+				      cmi);
+			return false;
+		}
+		if (ft != AMR_NO_DATA && !osmo_amr_is_speech(ft)) {
+			LOGP(DRTP, LOGL_ERROR, "unsupported AMR FT 0x%02x\n",
+			     ft);
+			return false;
+		}
+		if (osmo_amr_is_speech(ft)) {
+			if (lchan->tch.last_sid.len) { /* FIXME: force ONSET */
+				marker = true;
+			}
+			/* We received AMR SPEECH frame - invalidate saved SID */
+			lchan->tch.last_sid.len = 0;
+		}
 		if (marker) {
 			*payload_type = GsmL1_TchPlType_Amr_Onset;
 			rc = 0;
-			osmo_amr_rtp_dec(rtp_pl, rtp_pl_len, &cmr, &cmi, &ft,
-					 &bfi, &sti);
 			LOGP(DRTP, LOGL_ERROR, "Marker SPEECH frame AMR %s\n",
 			     get_value_string(osmo_amr_type_names, ft));
 		}
 		else {
 			*payload_type = GsmL1_TchPlType_Amr;
 			rc = rtppayload_to_l1_amr(l1_payload, rtp_pl,
-						  rtp_pl_len, lchan, fn);
-			if (-EALREADY == rc)
-				return false;
+						  rtp_pl_len, lchan, cmr, cmi,
+						  ft);
 		}
 		break;
 	default: