Include timestamp for start and stop of transmission in milliseconds. #1045
Conversation
Using sample rate and channels to derive the length of the file. 2038 proofed ms timestamps. change from long long to int64
|
Clever! This makes sense to me. Let me go check how things look on the OpenMHz side. I am almost 100% sure it doesn't use the timestamp in the filename for anything, so the change in resolution will not have an impact. |
Call_Data_t Call_Concluder::create_call_data should now be more efficient and have more accurate lengths for ms and s both. add get_start_time_ms to call_impl so we can get the start_time_ms in call_concluder.cc filename is now seconds.ms
|
Added some more work to this. Now we derive seconds from the new ms timestamp on a full call. filename is now call metadata (json) call_length is rounded, but call_length_ms 30.506 is a fully accurate playable length of the file. I figure with these changes, we have some time precision, but shouldn't effect anything down the pipeline too painfully as far as OpenMHZ or users custom scripts work. |
|
I like this. I'll have to change some of my upload logic to the server, but I'm ok with that as it adds some extra details. |
| BOOST_LOG_TRIVIAL(info) << "Broadcastify API Key: " << system->get_bcfy_api_key(); | ||
| system->set_bcfy_system_id(element.value("broadcastifySystemId", 0)); | ||
| BOOST_LOG_TRIVIAL(info) << "Broadcastify Calls System ID: " << system->get_bcfy_system_id(); | ||
| system->set_upload_script(element.value("uploadScript", "")); |
There was a problem hiding this comment.
Why is the upload script dropped here?
|
I read some of the changes, and this is a massive patch. Some files delta like 6k lines of code. It also looks like it drops support for upload script for some reason? There is so much code in here, mostly because it updates a streamer plugin at the same time and calls into protobuf that I wouldn't feel comfortable reviewing. It feels like there are 3 or more patches in here than just adding millisecond timestamps. Is the protobuf streamer plugin updates required for the timestamp updates for some reasons or was this 3 patches squished into one PR? |
Since the upload script process is entirely fire-and-forget with no retry or followup, they've been working on a more extensible plugin solution: https://github.com/TheGreatCodeholio/tr_plugin_external_script It's likely that some additional things have been inadvertently added to the PR branch since the initial push. The |
|
I like the millisecond changes, and I like that it's an extra field so current scripts still work. If that can be cherry picked out as the feature that is implemented in this PR that would be great. Removing something like Upload script probably requires more discussion for the moment. But, really, really great work @TheGreatCodeholio. |
|
I am going to create a new pull request with just the ms_time feature added since I inadvertently added my work in progress here for modifying plugins and moving external scripts to a plugin. We will have to discuss further as a community what might be best for that, but in the end a plugin is the way to go. Maybe even a way to run them in a specific order and get the data from whatever ran last. This way transcript/storage/mqtt could be done locally on the fly and have additional keys added to JSON. |
4 participants
I have added millisecond timestamps to for the start and the end of a transmission. Somewhat quick and dirty some optimizations could be added.
"start_time": 1759627129,
"stop_time": 1759627132,
"start_time_ms": 1759627129115,
"stop_time_ms": 1759627137005,
Future changes would include deriving the start and stop time from the millisecond time, and changing the filenames instead of using a semi-hacky +1 second to avoid collisions.
Doing this would be a step towards more accurate and precise timestamps. Which help in post processing scripts outside of trunk-recorder for detecting duplicate/simulcast calls for analog, or a setup where there are multiple nodes in different locations for the same radio system.