sensors-logging: Refactor to reduce memory usage and allow individual video subtitles

[rafaellehmkuhl]

The old pipeline was saving the entire log in memory, which was affecting the application's performance.

With this new approach, there's no more logs in memory. All log points are saved in the local database, and retrieved when the request to generate the log file is made.

I took the opportunity of this refactor to also address the problem of multiple log requesters interfering with each other. Now the system will not stop logging while there's an active log requester.

Fix #1446

[ES-Alexander]

Aware it's potentially out of scope, but would it make sense to include some of the #776 changes here?

That PR never got merged, and I think would likely need some rework to apply to the current codebase, but basic stuff like using loops for the largely repeated content of the subtitle lines seems like a good idea to reduce future maintenance load...

[rafaellehmkuhl]

Aware it's potentially out of scope, but would it make sense to include some of the #776 changes here?

That PR never got merged, and I think would likely need some rework to apply to the current codebase, but basic stuff like using loops for the largely repeated content of the subtitle lines seems like a good idea to reduce future maintenance load...

I think it's out of scope. There were changes on that part of the code after the mentioned PR and they are unrelated to this patch.

Current patch fixes a open bug, while the other is a code improvement (that was actually adding bugs, as commented there).

[ArturoManzoli]

Does the branch name, in this case, means something to the PR?

[ArturoManzoli]

Nice!

[rafaellehmkuhl]

Does the branch name, in this case, means something to the PR?

Yes. The way that it is right now, if you have to records running, and stop one of them, all of the sensor logs stop, and you won't have sensors logs for the rest of the second recording.

With this patch, the logs stay running until there are no recordings left.

Can you check if it's working as it should for you as well? This is an important feature, so I don't want to risk breaking it. Same @ES-Alexander.

[ES-Alexander]

Can you check if it's working as it should for you as well? This is an important feature, so I don't want to risk breaking it. Same @ES-Alexander.

I was trying to test, but BlueOS doesn't want to play ball with either of my cameras, so I can't set up two video stream sources to do recordings with (just one with the fake source) :(

I've spent the past hour+ on this, and need to get some sleep, so I think I'll have to leave it for next week unfortunately to try to test it properly :-\

[ES-Alexander]

1. It does seem to ensure the subtitles continue for other recordings even when a different recording ends 👍
2. The subtitles seem to be constrained completely inside the video recording, instead of starting right at the start and going right until the end
   • This could be considered an existing bug (so may be out of scope), but seems to make sense to fix if this PR is trying to fix the subtitle timings
   • The desired behaviour would be for the first log sample before the video to be clipped to the start of the video, and the last sample should be the one that goes to the end of the video (or slightly past it, since the player doesn't care if the subtitles are too long)
3. There seems to be a new bug where the recorder mini-widget thinks it can't load a selected stream (it disables the record button in the dialog, and creates an error popup), even if it then works fine to actually record with afterwards:
   

[rafaellehmkuhl]

2. The subtitles seem to be constrained completely inside the video recording, instead of starting right at the start and going right until the end

   • This could be considered an existing bug (so may be out of scope), but seems to make sense to fix if this PR is trying to fix the subtitle timings
   • The desired behaviour would be for the first log sample before the video to be clipped to the start of the video, and the last sample should be the one that goes to the end of the video (or slightly past it, since the player doesn't care if the subtitles are too long)

Agree. Will address #815 with this as well, but will put on a new PR to add info on the changelog.

3. There seems to be a new bug where the recorder mini-widget thinks it can't load a selected stream (it disables the record button in the dialog, and creates an error popup), even if it then works fine to actually record with afterwards:

Will fix that. Nice catch.

[ArturoManzoli]

1. It does seem to ensure the subtitles continue for other recordings even when a different recording ends 👍
2. The subtitles seem to be constrained completely inside the video recording, instead of starting right at the start and going right until the end

Had the same results here. Also noticed the bug when selecting the stream on the mini-video-recorder. It complains about not being possible to load media stream, but it loads normally, anyway. Also the circular progress on the record button keeps active

[rafaellehmkuhl]

1. It does seem to ensure the subtitles continue for other recordings even when a different recording ends 👍
2. The subtitles seem to be constrained completely inside the video recording, instead of starting right at the start and going right until the end

Had the same results here. Also noticed the bug when selecting the stream on the mini-video-recorder. It complains about not being possible to load media stream, but it loads normally, anyway. Also the circular progress on the record button keeps active

I actually just noticed the bug is in master. Can you confirm that?

If so, we can merge this PR and fix that separately.

Opened #1659 to track that.

[ArturoManzoli]

@rafaellehmkuhl

For my side, this pull request is approved since the stream problem will be addressed on a separate issue.
The other aspects of the PR are all good to merge!

Will open the fix for the missing logs in the start of the video on another PR, and the identified bug was already on master and a issue was opened for it.