MT#59069 merge handling of output-dest w metadata

Handle the `output-destination` flag in the same way as the `metadata`
flag, eliminating the need to do it explicitly everywhere it's relevant.

Add a few alternative spellings of `output-destination`.

Rename the member from flags to `recording_file` to make it more clear
what it is.

Add save/restore capabilities of that field to the Redis code.

Unify printing of monologue LABEL into the same function that handles
the metadata.

Update documentation to better explain this option, and use the new
clearer name.

Change-Id: I4496341013b0ccab5b1dec026cf3a1a0ea879018

daemon/call_interfaces.c

@@ -1699,7 +1699,18 @@ static void call_ng_main_flags(sdp_ng_flags *out, str *key, bencode_item_t *valu
 			call_ng_flags_str_list(out, value, ng_osrtp_option, NULL);
 			break;
 		case CSH_LOOKUP("output-destination"):
-			out->output_dest = s;
+		case CSH_LOOKUP("output-dest"):
+		case CSH_LOOKUP("output-file"):
+		case CSH_LOOKUP("recording-destination"):
+		case CSH_LOOKUP("recording-dest"):
+		case CSH_LOOKUP("recording-file"):
+		case CSH_LOOKUP("output destination"):
+		case CSH_LOOKUP("output dest"):
+		case CSH_LOOKUP("output file"):
+		case CSH_LOOKUP("recording destination"):
+		case CSH_LOOKUP("recording dest"):
+		case CSH_LOOKUP("recording file"):
+			out->recording_file = s;
 			break;
 		case CSH_LOOKUP("passthrough"):
 		case CSH_LOOKUP("passthru"):
@@ -2621,10 +2632,8 @@ static const char *call_recording_common_ng(bencode_item_t *input, bencode_item_
 
 
 static void start_recording_fn(bencode_item_t *input, call_t *call) {
-	str output_dest;
-	bencode_dictionary_get_str(input, "output-destination", &output_dest);
 	CALL_SET(call, RECORDING_ON);
-	recording_start(call, &output_dest);
+	recording_start(call);
 }
 const char *call_start_recording_ng(bencode_item_t *input, bencode_item_t *output) {
 	return call_recording_common_ng(input, output, start_recording_fn);
@@ -2840,7 +2849,7 @@ const char *call_start_forwarding_ng(bencode_item_t *input, bencode_item_t *outp
 	else
 		update_metadata_call(call, &flags);
 
-	recording_start(call, NULL);
+	recording_start(call);
 	return NULL;
 }
 

daemon/recording.c

@@ -274,43 +274,46 @@ static void update_call_field(call_t *call, str *dst_field, const str *src_field
 	if (!call)
 		return;
 
-	if (src_field->len && str_cmp_str(src_field, dst_field)) {
+	if (src_field && src_field->len && str_cmp_str(src_field, dst_field))
 		call_str_cpy(call, dst_field, src_field);
-		if (call->recording) {
-			va_list ap;
-			va_start(ap, meta_fmt);
-			vappend_meta_chunk_str(call->recording, src_field, meta_fmt, ap);
-			va_end(ap);
-		}
+
+	if (call->recording && dst_field->len) {
+		va_list ap;
+		va_start(ap, meta_fmt);
+		vappend_meta_chunk_str(call->recording, dst_field, meta_fmt, ap);
+		va_end(ap);
 	}
 }
 
+// lock must be held
 void update_metadata_call(call_t *call, const sdp_ng_flags *flags) {
-	update_call_field(call, &call->metadata, &flags->metadata, "METADATA");
+	update_call_field(call, &call->metadata, flags ? &flags->metadata : NULL, "METADATA");
+	update_call_field(call, &call->recording_file, flags ? &flags->recording_file : NULL, "RECORDING_FILE");
 }
 
 // lock must be held
-void update_metadata_monologue(struct call_monologue *ml, const sdp_ng_flags *flags) {
+void update_metadata_monologue_only(struct call_monologue *ml, const sdp_ng_flags *flags) {
 	if (!ml)
 		return;
 
-	call_t *call = ml->call;
-
-	update_call_field(call, &ml->metadata, &flags->metadata, "METADATA-TAG %u", ml->unique_id);
-
-	update_metadata_call(call, flags);
+	update_call_field(ml->call, &ml->metadata, flags ? &flags->metadata : NULL,
+			"METADATA-TAG %u", ml->unique_id);
+	update_call_field(ml->call, &ml->label, NULL, "LABEL %u", ml->unique_id);
 }
 
-static void update_output_dest(call_t *call, const str *output_dest) {
-	if (!output_dest || !output_dest->s || !call->recording)
+void update_metadata_monologue(struct call_monologue *ml, const sdp_ng_flags *flags) {
+	if (!ml)
 		return;
-	recording_meta_chunk(call->recording, "OUTPUT_DESTINATION", output_dest);
+
+	update_metadata_monologue_only(ml, flags);
+	update_metadata_call(ml->call, flags);
 }
 
 // lock must be held
 static void update_flags_proc(call_t *call, bool streams) {
 	append_meta_chunk_null(call->recording, "RECORDING %u", CALL_ISSET(call, RECORDING_ON));
 	append_meta_chunk_null(call->recording, "FORWARDING %u", CALL_ISSET(call, REC_FORWARDING));
+	update_metadata_call(call, NULL);
 	if (!streams)
 		return;
 	for (__auto_type l = call->streams.head; l; l = l->next) {
@@ -324,10 +327,9 @@ static void recording_update_flags(call_t *call, bool streams) {
 }
 
 // lock must be held
-void recording_start(call_t *call, const str *output_dest) {
+void recording_start(call_t *call) {
 	if (call->recording) {
 		// already active
-		update_output_dest(call, output_dest);
 		recording_update_flags(call, true);
 		return;
 	}
@@ -353,7 +355,6 @@ void recording_start(call_t *call, const str *output_dest) {
 	// update main call flags (global recording/forwarding on/off) to prevent recording
 	// features from being started when the stream info (through setup_stream) is
 	// propagated if recording is actually off
-	update_output_dest(call, output_dest);
 	recording_update_flags(call, false);
 
 	// if recording has been turned on after initial call setup, we must walk
@@ -429,7 +430,7 @@ void detect_setup_recording(call_t *call, const sdp_ng_flags *flags) {
 
 	if (!str_cmp(recordcall, "yes") || !str_cmp(recordcall, "on") || flags->record_call) {
 		CALL_SET(call, RECORDING_ON);
-		recording_start(call, &flags->output_dest);
+		recording_start(call);
 	}
 	else if (!str_cmp(recordcall, "no") || !str_cmp(recordcall, "off")) {
 		CALL_CLEAR(call, RECORDING_ON);
@@ -934,10 +935,7 @@ static void setup_monologue_proc(struct call_monologue *ml) {
 		return;
 
 	append_meta_chunk_str(recording, &ml->tag, "TAG %u", ml->unique_id);
-	if (ml->label.len)
-		append_meta_chunk_str(recording, &ml->label, "LABEL %u", ml->unique_id);
-	if (ml->metadata.len)
-		append_meta_chunk_str(recording, &ml->metadata, "METADATA-TAG %u", ml->unique_id);
+	update_metadata_monologue_only(ml, NULL);
 }
 
 static void setup_media_proc(struct call_media *media) {

daemon/redis.c

@@ -2079,7 +2079,9 @@ static void json_restore_call(struct redis *r, const str *callid, bool foreign)
 		// coverity[check_return : FALSE]
 		redis_hash_get_str(&s, &call, "recording_metadata");
 		call_str_cpy(c, &c->metadata, &s);
-		recording_start(c, NULL);
+		redis_hash_get_str(&s, &call, "recording_file");
+		call_str_cpy(c, &c->recording_file, &s);
+		recording_start(c);
 	}
 
 	// force-clear foreign flag (could have been set through call_flags), then
@@ -2368,6 +2370,8 @@ char* redis_encode_json(call_t *c) {
 
 			if (c->recording_meta_prefix.len)
 				JSON_SET_SIMPLE_STR("recording_meta_prefix", &c->recording_meta_prefix);
+			if (c->recording_file.len)
+				JSON_SET_SIMPLE_STR("recording_file", &c->recording_file);
 		}
 
 		json_builder_end_object(builder);

docs/ng_control_protocol.md

@@ -1739,8 +1739,9 @@ call legs, therefore all keys other than `call-id` are currently ignored.
 If the chosen recording method doesn't support in-kernel packet forwarding, enabling call recording
 via this messages will force packet forwarding to happen in userspace only.
 
-If the optional `output-destination` key is set, then its value will be used
-as an output file. Note that a filename extension will not be added.
+If the optional `recording-file` key is set, then its value will be used as an
+output file. Note that the value must refer to a complete (absolute) path
+including file name, and a file name extension will not be added.
 
 ## `stop recording` Message
 

include/call.h

@@ -706,6 +706,7 @@ struct call {
 	struct recording 	*recording;
 	str			metadata;
 	str			recording_meta_prefix;
+	str			recording_file;
 
 	struct call_iterator_entry iterator[NUM_CALL_ITERATORS];
 	int			cpu_affinity;

include/call_interfaces.h

@@ -34,7 +34,7 @@ struct sdp_ng_flags {
 	sockfamily_t *address_family;
 	int tos;
 	str record_call_str;
-	str output_dest;
+	str recording_file;
 	str metadata;
 	str label;
 	str set_label;

include/recording.h

@@ -113,7 +113,7 @@ void detect_setup_recording(call_t *call, const sdp_ng_flags *flags);
 void update_metadata_call(call_t *call, const sdp_ng_flags *flags);
 void update_metadata_monologue(struct call_monologue *ml, const sdp_ng_flags *flags);
 
-void recording_start(call_t *call, const str *output_dest);
+void recording_start(call_t *call);
 void recording_pause(call_t *call);
 void recording_stop(call_t *call);
 void recording_discard(call_t *call);

recording-daemon/metafile.c

@@ -216,7 +216,7 @@ static void meta_section(metafile_t *mf, char *section, char *content, unsigned
 		mf->forwarding_on = u ? 1 : 0;
 	else if (sscanf_match(section, "STREAM %lu FORWARDING %u", &lu, &u) == 2)
 		stream_forwarding_on(mf, lu, u);
-	else if (!strcmp(section, "OUTPUT_DESTINATION"))
+	else if (!strcmp(section, "RECORDING_FILE"))
 		mf->output_dest = g_string_chunk_insert(mf->gsc, content);
 }