Voice Broadcast - Stop recording on app restart #7450
Conversation
Florian14
changed the base branch from
develop
to
feature/fre/voice_broadcast_timeline_improvements
Florian14
changed the title
Florian14
requested review from
a team and
onurays
and removed request for
a team
Florian14
changed the base branch from
feature/fre/voice_broadcast_timeline_improvements
to
feature/fre/voice_broadcast_device_id
|
Moved to draft for the moment, I detected an issue |
Florian14
force-pushed
the
feature/fre/voice_broadcast_device_id
branch
from
1b7928e to
a4eff0c
Compare
Florian14
force-pushed
the
feature/fre/voice_broadcast_stop_on_app_restart
branch
from
a154db9 to
85bc78b
Compare
| private fun stopOngoingVoiceBroadcast() { | ||
| val session = activeSessionHolder.getSafeActiveSession() ?: return | ||
|
|
||
| // FIXME Iterate only on recent rooms for the moment, improve this |
There was a problem hiding this comment.
This will be fixed in a dedicated PR, it should be enough for the moment
| * Stop ongoing voice broadcast if any. | ||
| */ | ||
| private fun stopOngoingVoiceBroadcast() { | ||
| val session = activeSessionHolder.getSafeActiveSession() ?: return |
There was a problem hiding this comment.
Do you think it is possible to encapsulate all this code in a dedicated UseCase such as StopOngoingVoiceBroadcastUseCase? We could add unit tests on it later.
| import javax.inject.Inject | ||
|
|
||
| class GetOngoingVoiceBroadcastsUseCase @Inject constructor( | ||
| private val session: Session, |
There was a problem hiding this comment.
I think we should use ActiveSessionHolder instead. @bmarty what do you think about this suggestion?
|
|
||
| Timber.d("## GetLastVoiceBroadcastUseCase: get last voice broadcast in $roomId") | ||
|
|
||
| return room.stateService().getStateEvents( |
There was a problem hiding this comment.
I am wondering if we should use getStateEvent() instead. I think the issue may only happen for the last emitted state event of the room right? If we use getStateEvents it will go through every saved state events which may be a big list in large room.
There was a problem hiding this comment.
@mnaturel Not sure, in Element, we cannot have more than one ongoing VB in a room. But "advanced users" or other clients are still allowed to send new state events with different state keys and we'll have several ongoing VB from several users...
|
SonarCloud Quality Gate failed. |
| @@ -31,8 +31,12 @@ class GetOngoingVoiceBroadcastsUseCase @Inject constructor( | |||
| ) { | |||
|
|
|||
| fun execute(roomId: String): List<VoiceBroadcastEvent> { | |||
| val session = activeSessionHolder.getSafeActiveSession() ?: return emptyList() | |||
| val room = session.getRoom(roomId) ?: error("Unknown roomId: $roomId") | |||
| println("## GetOngoingVoiceBroadcastsUseCase") | |||
There was a problem hiding this comment.
Should we remove this println? If we want to keep some logs, we should use Timber instead.
There was a problem hiding this comment.
Good catch... will create a new PR
Type of change
Content
Stop the recording when opening the room timeline after an application crash or restart
Also, handled already paused VB before pausing again when the timeline is not shown anymore
Motivation and context
The voice broadcast is stuck in a recording state when the app is not active anymore, until we improve the code to be able to resume a VB from the last sequence, it is better to stop it as soon as we detect the wrong recording state
Screenshots / GIFs
Tests
Tested devices
Checklist
developresilience-rc branch