Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save videos directly to the filesystem [electron-only] #1358

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

rafaellehmkuhl
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl commented Sep 23, 2024

This PR changes the behavior of the Electron-based application.

Instead of saving the video chunks, video files and telemetry logs to the IndexedDB, we now save them directly to the filesystem.

The main advantage for the user is that it stops risking losing videos on a browser cache cleanup, which was the main cause of video loss. As a side-effect, the user has also the video files directly on their machine, without the need to download them, keeping everything always organized and in sync.

Kapture.2024-12-11.at.05.21.07.mp4

The implementation is done over the renderer/main IPC system, so we don't leave unnecessary filesystem permissions for the client, making everything safe.

Fix #1292.

@rafaellehmkuhl rafaellehmkuhl force-pushed the video-on-filesystem branch 4 times, most recently from 23b0794 to 5ecff0a Compare September 23, 2024 21:30
@rafaellehmkuhl rafaellehmkuhl force-pushed the video-on-filesystem branch 5 times, most recently from f31bfea to 53fa3e4 Compare November 7, 2024 18:30
@rafaellehmkuhl rafaellehmkuhl force-pushed the video-on-filesystem branch 4 times, most recently from c21cbe7 to 461da63 Compare November 29, 2024 22:25
@rafaellehmkuhl rafaellehmkuhl force-pushed the video-on-filesystem branch 8 times, most recently from c22259b to fa42f76 Compare December 10, 2024 20:12
@rafaellehmkuhl rafaellehmkuhl marked this pull request as ready for review December 12, 2024 17:19
@rafaellehmkuhl rafaellehmkuhl changed the title [WIP] Save videos directly to filesystem (on Electron version) Save videos directly to filesystem (on Electron version) Dec 12, 2024
@rafaellehmkuhl rafaellehmkuhl changed the title Save videos directly to filesystem (on Electron version) Save videos directly to the filesystem (electron-only) Dec 12, 2024
@ArturoManzoli
Copy link
Contributor

ArturoManzoli commented Dec 16, 2024

Important to mention that the video feed is coming through the BlueBoat base station.
Next tests will be made on direct LAN connection.

I'm having some issues on the tests using the base station. The first videos I tried were not recorded and a popup was shown to alert about the video error. As I kept trying, only the videos shorter than 5s got sometimes corrupted and couldn't be processed.

Btw.: Where are the videos chunks/files stored when running on linux?

Screenshare.-.2024-12-16.6_46_28.PM.mp4

@ArturoManzoli
Copy link
Contributor

Didn't have any more video issues when using Cockpit on the built AppImage, even when connected via BB BaseStation.
Probably was something related to the electron app running from bun on dev mode

@rafaellehmkuhl
Copy link
Member Author

Important to mention that the video feed is coming through the BlueBoat base station. Next tests will be made on direct LAN connection.

I'm having some issues on the tests using the base station. The first videos I tried were not recorded and a popup was shown to alert about the video error. As I kept trying, only the videos shorter than 5s got sometimes corrupted and couldn't be processed.

Btw.: Where are the videos chunks/files stored when running on linux?

Screenshare.-.2024-12-16.6_46_28.PM.mp4

Strange. This PR does not change the video pipeline at all, besides saving the chunks to a given folder and not the the browser temporary folder (which is used by the IndexedDB).

The files should be under /home/username/Cockpit.

@ArturoManzoli
Copy link
Contributor

Is it possible to add a 'Select video saving folder' option on the video settings?
Or maybe an indication of where the user can find the video files?

@rafaellehmkuhl
Copy link
Member Author

Is it possible to add a 'Select video saving folder' option on the video settings? Or maybe an indication of where the user can find the video files?

Will add a button to open the video folder in the video modal!

When the extension was unknown, the video file was being generated with two dots in the end (e.g.: 'video..webm'). This also created a side-effect of the system not being able to find the correspondent telemetry logs and leaving them behind when clearing the videos.
For that an abstraction layer was created. It coordinates the usage of the IndexedDB for the browser version and the filesystem for the Electron version.
@rafaellehmkuhl
Copy link
Member Author

@ArturoManzoli done.

@ArturoManzoli
Copy link
Contributor

Tested on Linux and Windows. Works very well on both

@ArturoManzoli ArturoManzoli merged commit ab5a1f9 into bluerobotics:master Dec 17, 2024
11 checks passed
@rafaellehmkuhl rafaellehmkuhl changed the title Save videos directly to the filesystem (electron-only) Save videos directly to the filesystem [electron-only] Dec 18, 2024
@rafaellehmkuhl rafaellehmkuhl deleted the video-on-filesystem branch December 18, 2024 16:40
@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support recording videos to the filesystem instead of the browser DB
3 participants