Supporting rotation during voice recordings #4556
Conversation
…rotation - this means we need to be extra careful about releasing any listeners
| @@ -158,22 +162,23 @@ class VoiceMessageRecorderView @JvmOverloads constructor( | |||
| dragState = newDragState | |||
| } | |||
|
|
|||
| private fun startRecordingTicker() { | |||
| private fun startRecordingTicker(startFromLocked: Boolean, startAt: Long) { | |||
| val startMs = ((System.currentTimeMillis() - startAt)).coerceAtLeast(0) | |||
There was a problem hiding this comment.
ideally we'd have a Clock abstraction to avoid directly calling into the system time, would help out massively for testing
what do people think?
| @@ -158,22 +162,23 @@ class VoiceMessageRecorderView @JvmOverloads constructor( | |||
| dragState = newDragState | |||
| } | |||
|
|
|||
| private fun startRecordingTicker() { | |||
| private fun startRecordingTicker(startFromLocked: Boolean, startAt: Long) { | |||
| val startMs = ((System.currentTimeMillis() - startAt)).coerceAtLeast(0) | |||
- had to keep track of the recording start time in order to maintain the current length counter
ouchadam
force-pushed
the
feature/adm/voice-rotation
branch
from
7d2c8e6
to
5d18120
Compare
|
|
||
| class DefaultClock @Inject constructor() : Clock { | ||
| override fun epochMillis(): Long { | ||
| return System.currentTimeMillis() |
There was a problem hiding this comment.
A bit out of scope of this PR, but we will probably want to replace all System.currentTimeMillis() usages by this new Clock?
There was a problem hiding this comment.
💯 that would be the dream! I'll create an issue
| import javax.inject.Inject | ||
|
|
||
| interface Clock { | ||
| fun epochMillis(): Long |
There was a problem hiding this comment.
It's worth noting that the current impl rely on the system date, which can be set by the user (and at any time).
This can lead to bugs...
Also when the clock change automatically (Winter?Summer time) we can have bugs (my experience is talking here).
There was a problem hiding this comment.
unless we use a server time is there a way to avoid this? analytics SDKs will rely on a mix of server received at timestamps and client sent at
the currentTimeMillis epoch is UTC, unless a user sets a custom time using their local time (eg UTC+11) but with UTC+0 timezone 😅
it's becoming rarer for users to misconfigure their time due to automatic network times and a lot apps/websites start complaining about SSL errors if the current time/date is wildly wrong
EDIT: I'll add some java doc to explain this time could be incorrect
|
I have this issue on Element v1.4.32 (2022-08-10) from F-Droid. |
Fixes #4067 Allows the device to rotate whilst recording a voice message
VoiceMessagePlaybackTrackera singleton to avoid losing any state on activity destruction (means we need to be extra careful with releasing the listeners)