ECU in UL path: make it optional per vty config

Current osmo-bts-trx includes a provision for invoking ECUs from
libosmocodec in the UL path from the channel decoder to the RTP
output.  This pre-existing implementation is counter to the spirit
of 3GPP specs (a BTS should merely mark BFI conditions in its UL
output, as opposed to actively modifying the frame stream with an ECU),
inconsistent between different osmo-bts models (only in -trx and no
others), and inconsistent even within osmo-bts-trx itself, where
the link quality check in l1sap will sometimes suppress the output
of the ECU - a quirk which the designers of the current mechanism
most certainly did not intend.

The solution decided upon in OsmoDevCall on 2023-06-21 is to make
this ECU optional per vty config, and move it from the trx model
to the common layer to resolve the inconsistencies.  Implement the
first part: make the ECU application optional per vty config.

For backward compatibility with existing deployments, the new
"rtp internal-uplink-ecu" setting is enabled by default on osmo-bts-trx
but not on any other models.

Related: OS#6040
Change-Id: I0acca9c6d7da966a623287563e0789db9e0fae8e

include/osmo-bts/bts.h

@@ -300,6 +300,7 @@ struct gsm_bts {
 	int rtp_priority;
 
 	bool rtp_nogaps_mode;		/* emit RTP stream without any gaps */
+	bool use_ul_ecu;		/* "rtp internal-uplink-ecu" option */
 	bool emit_hr_rfc5993;
 
 	struct {

src/common/vty.c

@@ -418,6 +418,8 @@ static void config_write_bts_single(struct vty *vty, const struct gsm_bts *bts)
 		vty_out(vty, " rtp socket-priority %d%s", bts->rtp_priority, VTY_NEWLINE);
 	if (bts->rtp_nogaps_mode)
 		vty_out(vty, " rtp continuous-streaming%s", VTY_NEWLINE);
+	vty_out(vty, " %srtp internal-uplink-ecu%s",
+		bts->use_ul_ecu ? "" : "no ", VTY_NEWLINE);
 	vty_out(vty, " rtp hr-format %s%s",
 		bts->emit_hr_rfc5993 ? "rfc5993" : "ts101318", VTY_NEWLINE);
 	vty_out(vty, " paging queue-size %u%s", paging_get_queue_max(bts->paging_state),
@@ -799,6 +801,28 @@ DEFUN(cfg_bts_no_rtp_cont_stream,
 	return CMD_SUCCESS;
 }
 
+DEFUN(cfg_bts_rtp_int_ul_ecu,
+      cfg_bts_rtp_int_ul_ecu_cmd,
+      "rtp internal-uplink-ecu",
+      RTP_STR "Apply a BTS-internal ECU to the uplink traffic frame stream\n")
+{
+	struct gsm_bts *bts = vty->index;
+
+	bts->use_ul_ecu = true;
+	return CMD_SUCCESS;
+}
+
+DEFUN(cfg_bts_no_rtp_int_ul_ecu,
+      cfg_bts_no_rtp_int_ul_ecu_cmd,
+      "no rtp internal-uplink-ecu",
+      NO_STR RTP_STR "Apply a BTS-internal ECU to the uplink traffic frame stream\n")
+{
+	struct gsm_bts *bts = vty->index;
+
+	bts->use_ul_ecu = false;
+	return CMD_SUCCESS;
+}
+
 DEFUN_ATTR(cfg_bts_rtp_hr_format,
 	   cfg_bts_rtp_hr_format_cmd,
 	   "rtp hr-format (rfc5993|ts101318)",
@@ -2700,6 +2724,8 @@ int bts_vty_init(void *ctx)
 	install_element(BTS_NODE, &cfg_bts_rtp_priority_cmd);
 	install_element(BTS_NODE, &cfg_bts_rtp_cont_stream_cmd);
 	install_element(BTS_NODE, &cfg_bts_no_rtp_cont_stream_cmd);
+	install_element(BTS_NODE, &cfg_bts_rtp_int_ul_ecu_cmd);
+	install_element(BTS_NODE, &cfg_bts_no_rtp_int_ul_ecu_cmd);
 	install_element(BTS_NODE, &cfg_bts_rtp_hr_format_cmd);
 	install_element(BTS_NODE, &cfg_bts_band_cmd);
 	install_element(BTS_NODE, &cfg_description_cmd);

src/osmo-bts-trx/l1_if.c

@@ -439,8 +439,13 @@ int bts_model_l1sap_down(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap)
 				break;
 			}
 
-			/* attempt to allocate an Error Concealment Unit instance, if available */
-			lchan->ecu_state = osmo_ecu_init(trx, lchan2ecu_codec(lchan));
+			/* Attempt to allocate an Error Concealment Unit
+			 * instance, if available, unless it is disabled
+			 * by the vty config. */
+			if (trx->bts->use_ul_ecu)
+				lchan->ecu_state = osmo_ecu_init(trx, lchan2ecu_codec(lchan));
+			else
+				lchan->ecu_state = NULL;
 
 			/* activate dedicated channel */
 			trx_sched_set_lchan(lchan, chan_nr, LID_DEDIC, true);
@@ -473,7 +478,10 @@ int bts_model_l1sap_down(struct gsm_bts_trx *trx, struct osmo_phsap_prim *l1sap)
 			/* ECU for possibly new codec */
 			if (lchan->ecu_state)
 				osmo_ecu_destroy(lchan->ecu_state);
-			lchan->ecu_state = osmo_ecu_init(trx, lchan2ecu_codec(lchan));
+			if (trx->bts->use_ul_ecu)
+				lchan->ecu_state = osmo_ecu_init(trx, lchan2ecu_codec(lchan));
+			else
+				lchan->ecu_state = NULL;
 			/* change mode */
 			trx_sched_set_mode(lchan->ts, chan_nr,
 					   lchan->rsl_cmode, lchan->tch_mode,

src/osmo-bts-trx/main.c

@@ -164,6 +164,10 @@ int bts_model_init(struct gsm_bts *bts)
 	 * a surprise functional change upon software update. */
 	bts->emit_hr_rfc5993 = true;
 
+	/* For the same reason as the above, rtp internal-uplink-ecu
+	 * needs to be enabled by default on osmo-bts-trx model only. */
+	bts->use_ul_ecu = true;
+
 	return 0;
 }
 

tests/osmo-bts.vty

@@ -235,6 +235,8 @@ OsmoBTS(bts)# list
   rtp socket-priority <0-255>
   rtp continuous-streaming
   no rtp continuous-streaming
+  rtp internal-uplink-ecu
+  no rtp internal-uplink-ecu
   rtp hr-format (rfc5993|ts101318)
   band (450|GSM450|480|GSM480|750|GSM750|810|GSM810|850|GSM850|900|GSM900|1800|DCS1800|1900|PCS1900)
   description .TEXT