From 366448e44b730602d08b52ea4239ae5f0d947830 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 2 Jan 2024 14:08:49 -0500 Subject: [PATCH] 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 | 19 +++++++++++---- daemon/recording.c | 48 ++++++++++++++++++------------------- daemon/redis.c | 6 ++++- docs/ng_control_protocol.md | 5 ++-- include/call.h | 1 + include/call_interfaces.h | 2 +- include/recording.h | 2 +- recording-daemon/metafile.c | 2 +- 8 files changed, 49 insertions(+), 36 deletions(-) diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 7582575e4..22ce08a2c 100644 --- a/daemon/call_interfaces.c +++ b/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; } diff --git a/daemon/recording.c b/daemon/recording.c index 2af3677f5..6cc9e5729 100644 --- a/daemon/recording.c +++ b/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) { diff --git a/daemon/redis.c b/daemon/redis.c index 356faf5f6..ac393a8ee 100644 --- a/daemon/redis.c +++ b/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); diff --git a/docs/ng_control_protocol.md b/docs/ng_control_protocol.md index 72086dc22..e3058ce8a 100644 --- a/docs/ng_control_protocol.md +++ b/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 diff --git a/include/call.h b/include/call.h index acecdaa47..f14620428 100644 --- a/include/call.h +++ b/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; diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 01e047326..a563d5531 100644 --- a/include/call_interfaces.h +++ b/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; diff --git a/include/recording.h b/include/recording.h index 1e623f2f9..f50f78d28 100644 --- a/include/recording.h +++ b/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); diff --git a/recording-daemon/metafile.c b/recording-daemon/metafile.c index 96cde2a1f..6a061513b 100644 --- a/recording-daemon/metafile.c +++ b/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); }