Skip to content

Improve video recording management#655

Merged
rafaellehmkuhl merged 3 commits intobluerobotics:masterfrom
rafaellehmkuhl:improve-video-recording
Dec 23, 2023
Merged

Improve video recording management#655
rafaellehmkuhl merged 3 commits intobluerobotics:masterfrom
rafaellehmkuhl:improve-video-recording

Conversation

@rafaellehmkuhl
Copy link
Member

@rafaellehmkuhl rafaellehmkuhl commented Dec 22, 2023

Put the video store in charge of managing the video recordings, so recording information gets stored with the stream data. There are two main benefits on this:

  • The lifecycle of a widget (e.g.: a view being changed and a recording widget being unmounted) does not affect the state of a recording
  • Multiple recordings, from different streams, can be done simultaneously

Fix #624
Fix #633

To be merged after #654.

@rafaellehmkuhl
Copy link
Member Author

@joaoantoniocardoso @patrickelectric open for review.

@rafaellehmkuhl rafaellehmkuhl marked this pull request as ready for review December 22, 2023 15:46
Copy link
Member

@joaoantoniocardoso joaoantoniocardoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not critical, but confusing at least:

Because the widget shares a global state, and the source selection only happens when the user clicks the record/stop button, the user can't select a different source when there is a recording happening for the first source of the list of streams available, requiring the user to stop the recording before configuring the latter added widget:

cannot_change_recording_source_because_widgets_share_global_state.mp4

note that it is also impossible to record two videos from the same source (let's say, a quick clip and the entire dive)

@rafaellehmkuhl
Copy link
Member Author

Not critical, but confusing at least:

Because the widget shares a global state, and the source selection only happens when the user clicks the record/stop button, the user can't select a different source when there is a recording happening for the first source of the list of streams available, requiring the user to stop the recording before configuring the latter added widget:

cannot_change_recording_source_because_widgets_share_global_state.mp4
note that it is also impossible to record two videos from the same source (let's say, a quick clip and the entire dive)

Nice catch.
I think the multiple records for the same stream is something we can delay for now. Will ask our beta testers what they think about it. But the problem of not being able to record multiple sources I will fix already.

@rafaellehmkuhl
Copy link
Member Author

@joaoantoniocardoso fixed.

@rafaellehmkuhl rafaellehmkuhl merged commit a2d7886 into bluerobotics:master Dec 23, 2023
@rafaellehmkuhl rafaellehmkuhl deleted the improve-video-recording branch December 23, 2023 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Video Recorder] Fail to record multiple video streams Video recording stops when changing views

3 participants

Comments