add codec-set option

Codecs that were implicitly accepted for transcoding had no option to
set their parameters (such as bitrate) for encoding. The codec-set
option adds this.

fixes #681

Change-Id: Ib238eb73a507af168c366579abc80da07250fe88

README.md

@@ -1471,6 +1471,12 @@ Optionally included keys are:
 
 	Contains a dictionary controlling various aspects of codecs (or RTP payload types).
 	Most of these options should only be used in an `offer` message.
+
+	These options can also be put into the `flags` list using a prefix of `codec-`. For example,
+	to set the codec options for two variants of Opus when they're implicitly accepted, (see
+	the example under `set`), one would put the following into the `flags` list:
+	`codec-set-opus/48000/1/16000` `codec-set-opus/48000/2/32000`
+
 	The following keys are understood:
 
 	* `strip`
@@ -1555,6 +1561,23 @@ Optionally included keys are:
 		As with the `strip` option, the special keyword `all` can be used to mask all
 		codecs that have been offered.
 
+	* `set`
+
+		Contains a list of strings. This list makes it possible to set codec options
+		(bitrate in particular) for codecs that are implicitly accepted for transcoding.
+		For example, if `AMR` was offered, `transcode=PCMU` was given, and the remote
+		ended up accepting `PCMU`, then this option can be used to set the bitrate used
+		for the AMR transcoding process.
+
+		Each string must be a full codec specification as per above, including clock rate
+		and number of channels. Using the example above, `set=AMR/8000/1/7400` can be used
+		to transcode to AMR with 7.4 kbit/s.
+
+		Codec options (bitrate) are only applied to codecs that match the given parameters
+		(clock rate, channels), and multiple options can be given for the same coded with
+		different parameters. For example, to specify different bitrates for Opus for both
+		mono and stereo output, one could use `set=[opus/48000/1/16000,opus/48000/2/32000]`.
+
 * `ptime`
 
 	Contains an integer. If set, changes the `a=ptime` attribute's value in the outgoing

daemon/call_interfaces.c

@@ -596,20 +596,29 @@ static void call_ng_flags_supports(struct sdp_ng_flags *out, str *s, void *dummy
 		out->supports_load_limit = 1;
 }
 static void call_ng_flags_codec_list(struct sdp_ng_flags *out, str *s, void *qp) {
-	str *s_copy;
-	s_copy = g_slice_alloc(sizeof(*s_copy));
-	*s_copy = *s;
+	str *s_copy = str_slice_dup(s);
 	g_queue_push_tail((GQueue *) qp, s_copy);
 }
 static void call_ng_flags_str_ht(struct sdp_ng_flags *out, str *s, void *htp) {
-	str *s_copy;
-	s_copy = g_slice_alloc(sizeof(*s_copy));
-	*s_copy = *s;
+	str *s_copy = str_slice_dup(s);
 	GHashTable **ht = htp;
 	if (!*ht)
 		*ht = g_hash_table_new_full(str_hash, str_equal, str_slice_free, NULL);
 	g_hash_table_replace(*ht, s_copy, s_copy);
 }
+static void call_ng_flags_str_ht_split(struct sdp_ng_flags *out, str *s, void *htp) {
+	GHashTable **ht = htp;
+	if (!*ht)
+		*ht = g_hash_table_new_full(str_hash, str_equal, str_slice_free, str_slice_free);
+	str splitter = *s;
+	while (1) {
+		g_hash_table_replace(*ht, str_slice_dup(&splitter), str_slice_dup(s));
+		char *c = memrchr(splitter.s, '/', splitter.len);
+		if (!c)
+			break;
+		splitter.len = c - splitter.s;
+	}
+}
 // helper to alias values from other dictionaries into the "flags" dictionary
 INLINE int call_ng_flags_prefix(struct sdp_ng_flags *out, str *s_ori, const char *prefix,
 		void (*cb)(struct sdp_ng_flags *, str *, void *), void *ptr)
@@ -704,6 +713,9 @@ static void call_ng_flags_flags(struct sdp_ng_flags *out, str *s, void *dummy) {
 				return;
 			if (call_ng_flags_prefix(out, s, "codec-mask-", call_ng_flags_str_ht, &out->codec_mask))
 				return;
+			if (call_ng_flags_prefix(out, s, "codec-set-", call_ng_flags_str_ht_split,
+						&out->codec_set))
+				return;
 #endif
 
 			ilog(LOG_WARN, "Unknown flag encountered: '" STR_FORMAT "'",
@@ -799,6 +811,7 @@ static void call_ng_process_flags(struct sdp_ng_flags *out, bencode_item_t *inpu
 #ifdef WITH_TRANSCODING
 		call_ng_flags_list(out, dict, "transcode", call_ng_flags_codec_list, &out->codec_transcode);
 		call_ng_flags_list(out, dict, "mask", call_ng_flags_str_ht, &out->codec_mask);
+		call_ng_flags_list(out, dict, "set", call_ng_flags_str_ht_split, &out->codec_set);
 #endif
 	}
 }
@@ -807,6 +820,8 @@ static void call_ng_free_flags(struct sdp_ng_flags *flags) {
 		g_hash_table_destroy(flags->codec_strip);
 	if (flags->codec_mask)
 		g_hash_table_destroy(flags->codec_mask);
+	if (flags->codec_set)
+		g_hash_table_destroy(flags->codec_set);
 	if (flags->sdes_no)
 		g_hash_table_destroy(flags->sdes_no);
 	g_queue_clear_full(&flags->codec_offer, str_slice_free);

daemon/codec.c

@@ -806,7 +806,8 @@ struct rtp_payload_type *codec_make_payload_type(const str *codec_str, struct ca
 	str_init(&ret->encoding_with_params, full_encoding);
 	str_init(&ret->encoding_parameters, params);
 
-	__rtp_payload_type_dup(media->call, ret);
+	if (media)
+		__rtp_payload_type_dup(media->call, ret);
 
 	return ret;
 }
@@ -1157,6 +1158,32 @@ static int __revert_codec_strip(GHashTable *removed, const str *codec,
 	g_queue_free(q);
 	return 1;
 }
+static int __codec_options_set1(struct rtp_payload_type *pt, const str *enc, GHashTable *codec_set) {
+	str *pt_str = g_hash_table_lookup(codec_set, enc);
+	if (!pt_str)
+		return 0;
+	struct rtp_payload_type *pt_parsed = codec_make_payload_type(pt_str, NULL);
+	if (!pt_parsed)
+		return 0;
+	// match parameters
+	if (pt->clock_rate != pt_parsed->clock_rate || pt->channels != pt_parsed->channels) {
+		payload_type_free(pt_parsed);
+		return 0;
+	}
+	// match - apply options
+	if (!pt->bitrate)
+		pt->bitrate = pt_parsed->bitrate;
+	payload_type_free(pt_parsed);
+	return 1;
+}
+static void __codec_options_set(struct rtp_payload_type *pt, GHashTable *codec_set) {
+	if (!codec_set)
+		return;
+	if (__codec_options_set1(pt, &pt->encoding_with_params, codec_set))
+		return;
+	if (__codec_options_set1(pt, &pt->encoding, codec_set))
+		return;
+}
 void codec_rtp_payload_types(struct call_media *media, struct call_media *other_media,
 		GQueue *types, const struct sdp_ng_flags *flags)
 {
@@ -1203,6 +1230,7 @@ void codec_rtp_payload_types(struct call_media *media, struct call_media *other_
 				continue;
 			}
 		}
+		__codec_options_set(pt, flags->codec_set);
 		if (!mask_all && (!flags->codec_mask || !g_hash_table_lookup(flags->codec_mask, &pt->encoding))
 				&& (!flags->codec_mask || !g_hash_table_lookup(flags->codec_mask, &pt->encoding_with_params)))
 			__rtp_payload_type_add(media, other_media, pt);

include/call_interfaces.h

@@ -36,6 +36,7 @@ struct sdp_ng_flags {
 	GQueue codec_offer;
 	GQueue codec_transcode;
 	GHashTable *codec_mask;
+	GHashTable *codec_set;
 	int ptime;
 	GHashTable *sdes_no;
 	int asymmetric:1,

lib/str.h

@@ -94,6 +94,8 @@ INLINE str *g_string_free_str(GString *gs);
 guint str_hash(gconstpointer s);
 gboolean str_equal(gconstpointer a, gconstpointer b);
 
+/* returns a new str object, duplicates the pointers but doesn't duplicate the contents */
+INLINE str *str_slice_dup(const str *);
 /* destroy function, frees a slice-alloc'd str */
 void str_slice_free(void *);
 
@@ -217,6 +219,12 @@ INLINE str *str_dup(const str *s) {
 	r->s[s->len] = '\0';
 	return r;
 }
+INLINE str *str_slice_dup(const str *s) {
+	str *r;
+	r = g_slice_alloc(sizeof(*r));
+	*r = *s;
+	return r;
+}
 
 #define STR_MALLOC_PADDING "xxxxxxxxxxxxxxxx"
 INLINE str *__str_vsprintf(const char *fmt, va_list ap) {

t/transcode-test.c

@@ -67,10 +67,27 @@ static void __start(const char *file, int line) {
 	g_queue_init(&rtp_types); // parsed from received SDP
 	flags.codec_strip = g_hash_table_new_full(str_hash, str_equal, str_slice_free, NULL);
 	flags.codec_mask = g_hash_table_new_full(str_hash, str_equal, str_slice_free, NULL);
+	flags.codec_set = g_hash_table_new_full(str_hash, str_equal, str_slice_free, NULL);
 }
 
 #define transcode(codec) g_queue_push_tail(&flags.codec_transcode, sdup(#codec))
 
+static void codec_set(char *c) {
+	// from call_ng_flags_str_ht_split
+	c = strdup(c);
+	str s;
+	str_init(&s, c);
+	str splitter = s;
+
+	while (1) {
+		g_hash_table_replace(flags.codec_set, str_slice_dup(&splitter), str_slice_dup(&s));
+		char *c = memrchr(splitter.s, '/', splitter.len);
+		if (!c)
+			break;
+		splitter.len = c - splitter.s;
+	}
+}
+
 #define sdp_pt_fmt(num, codec, clockrate, fmt) \
 	__sdp_pt_fmt(num, (str) STR_CONST_INIT(#codec), clockrate, (str) STR_CONST_INIT(#codec "/" #clockrate), \
 			(str) STR_CONST_INIT(fmt))
@@ -254,6 +271,7 @@ static void __packet_seq_ts(const char *file, int line, struct call_media *media
 static void end() {
 	g_hash_table_destroy(rtp_ts_ht);
 	g_hash_table_destroy(rtp_seq_ht);
+	printf("\n");
 }
 
 static void dtmf(const char *s) {
@@ -622,6 +640,46 @@ int main() {
 			check_encoder(A, 0, 96, 7400);
 			check_encoder(B, 96, 0, 0);
 			end();
+
+			// specify reverse bitrate
+			start();
+			sdp_pt(96, AMR, 8000);
+			transcode(PCMU);
+			codec_set("AMR/8000/1/6700");
+			offer();
+			expect(A, recv, "");
+			expect(A, send, "96/AMR/8000");
+			expect(B, recv, "96/AMR/8000 0/PCMU/8000");
+			expect(B, send, "");
+			sdp_pt(0, PCMU, 8000);
+			answer();
+			expect(A, recv, "96/AMR/8000");
+			expect(A, send, "96/AMR/8000");
+			expect(B, recv, "0/PCMU/8000");
+			expect(B, send, "0/PCMU/8000");
+			check_encoder(A, 96, 0, 0);
+			check_encoder(B, 0, 96, 6700);
+			end();
+
+			// specify non-default reverse bitrate
+			start();
+			sdp_pt(96, AMR, 8000);
+			transcode(PCMU);
+			codec_set("AMR/8000/1/7400");
+			offer();
+			expect(A, recv, "");
+			expect(A, send, "96/AMR/8000");
+			expect(B, recv, "96/AMR/8000 0/PCMU/8000");
+			expect(B, send, "");
+			sdp_pt(0, PCMU, 8000);
+			answer();
+			expect(A, recv, "96/AMR/8000");
+			expect(A, send, "96/AMR/8000");
+			expect(B, recv, "0/PCMU/8000");
+			expect(B, send, "0/PCMU/8000");
+			check_encoder(A, 96, 0, 0);
+			check_encoder(B, 0, 96, 7400);
+			end();
 		}
 	}
 

utils/rtpengine-ng-client

@@ -51,6 +51,7 @@ GetOptions(
 	'codec-offer=s@'		=> \$options{'codec-offer'},
 	'codec-transcode=s@'		=> \$options{'codec-transcode'},
 	'codec-mask=s@'			=> \$options{'codec-mask'},
+	'codec-set=s@'			=> \$options{'codec-set'},
 	'ptime=i'			=> \$options{'ptime'},
 	'flags=s@'			=> \$options{'flags'},
 	'codec-options-flat'		=> \$options{'codec options flag'},
@@ -91,7 +92,7 @@ if (defined($options{direction})) {
 	$options{direction} =~ /(.*),(.*)/ or die;
 	$packet{direction} = [$1,$2];
 }
-for my $x (qw(strip offer transcode mask)) {
+for my $x (qw(strip offer transcode mask set)) {
 	if ($options{'codec-'.$x} && @{$options{'codec-'.$x}}) {
 		if (!$options{'codec options flag'}) {
 			$packet{codec}{$x} = $options{'codec-'.$x};