-
Notifications
You must be signed in to change notification settings - Fork 768
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
Audio files in the timeline now appear with the audio player #5586
Changes from all commits
00bced9
cde10ca
2617632
d54b465
fab78c9
2e716cb
7a7d36d
ff26829
5a819bb
c9946b6
06468a4
b035911
249db18
82cde16
806af47
c8a56d6
313595e
8aaaf80
931c0e9
fa56a5e
cc5e8f3
6adf487
8ad4f20
34dcd70
d0155c9
76b2cfd
dc36301
355db98
4159850
d9f2033
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Adds the ability for audio attachments to be played in the timeline | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ import android.media.AudioAttributes | |
import android.media.MediaPlayer | ||
import androidx.core.content.FileProvider | ||
import im.vector.app.BuildConfig | ||
import im.vector.app.features.home.room.detail.timeline.helper.VoiceMessagePlaybackTracker | ||
import im.vector.app.features.home.room.detail.timeline.helper.AudioMessagePlaybackTracker | ||
import im.vector.app.features.voice.VoiceFailure | ||
import im.vector.app.features.voice.VoiceRecorder | ||
import im.vector.app.features.voice.VoiceRecorderProvider | ||
|
@@ -40,12 +40,13 @@ import javax.inject.Inject | |
/** | ||
* Helper class to record audio for voice messages. | ||
*/ | ||
class VoiceMessageHelper @Inject constructor( | ||
class AudioMessageHelper @Inject constructor( | ||
private val context: Context, | ||
private val playbackTracker: VoiceMessagePlaybackTracker, | ||
private val playbackTracker: AudioMessagePlaybackTracker, | ||
voiceRecorderProvider: VoiceRecorderProvider | ||
) { | ||
private var mediaPlayer: MediaPlayer? = null | ||
private var currentPlayingId: String? = null | ||
private var voiceRecorder: VoiceRecorder = voiceRecorderProvider.provideVoiceRecorder() | ||
|
||
private val amplitudeList = mutableListOf<Int>() | ||
|
@@ -58,7 +59,7 @@ class VoiceMessageHelper @Inject constructor( | |
amplitudeList.clear() | ||
attachmentData.waveform?.let { | ||
amplitudeList.addAll(it) | ||
playbackTracker.updateCurrentRecording(VoiceMessagePlaybackTracker.RECORDING_ID, amplitudeList) | ||
playbackTracker.updateCurrentRecording(AudioMessagePlaybackTracker.RECORDING_ID, amplitudeList) | ||
} | ||
} | ||
|
||
|
@@ -127,7 +128,7 @@ class VoiceMessageHelper @Inject constructor( | |
|
||
fun startOrPauseRecordingPlayback() { | ||
voiceRecorder.getCurrentRecord()?.let { | ||
startOrPausePlayback(VoiceMessagePlaybackTracker.RECORDING_ID, it) | ||
startOrPausePlayback(AudioMessagePlaybackTracker.RECORDING_ID, it) | ||
} | ||
} | ||
|
||
|
@@ -136,7 +137,8 @@ class VoiceMessageHelper @Inject constructor( | |
mediaPlayer?.stop() | ||
stopPlaybackTicker() | ||
stopRecordingAmplitudes() | ||
if (playbackState is VoiceMessagePlaybackTracker.Listener.State.Playing) { | ||
currentPlayingId = null | ||
if (playbackState is AudioMessagePlaybackTracker.Listener.State.Playing) { | ||
playbackTracker.pausePlayback(id) | ||
} else { | ||
startPlayback(id, file) | ||
|
@@ -163,6 +165,7 @@ class VoiceMessageHelper @Inject constructor( | |
seekTo(currentPlaybackTime) | ||
} | ||
} | ||
currentPlayingId = id | ||
} catch (failure: Throwable) { | ||
Timber.e(failure, "Unable to start playback") | ||
throw VoiceFailure.UnableToPlay(failure) | ||
|
@@ -171,17 +174,24 @@ class VoiceMessageHelper @Inject constructor( | |
} | ||
|
||
fun stopPlayback() { | ||
playbackTracker.pausePlayback(VoiceMessagePlaybackTracker.RECORDING_ID) | ||
playbackTracker.pausePlayback(AudioMessagePlaybackTracker.RECORDING_ID) | ||
mediaPlayer?.stop() | ||
stopPlaybackTicker() | ||
currentPlayingId = null | ||
} | ||
|
||
fun movePlaybackTo(id: String, percentage: Float, totalDuration: Int) { | ||
val toMillisecond = (totalDuration * percentage).toInt() | ||
playbackTracker.updateCurrentPlaybackTime(id, toMillisecond, percentage) | ||
playbackTracker.pauseAllPlaybacks() | ||
|
||
stopPlayback() | ||
playbackTracker.pausePlayback(id) | ||
if (currentPlayingId == id) { | ||
mediaPlayer?.seekTo(toMillisecond) | ||
playbackTracker.updatePlayingAtPlaybackTime(id, toMillisecond, percentage) | ||
} else { | ||
mediaPlayer?.pause() | ||
playbackTracker.updatePausedAtPlaybackTime(id, toMillisecond, percentage) | ||
stopPlaybackTicker() | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 thanks! |
||
} | ||
|
||
private fun startRecordingAmplitudes() { | ||
|
@@ -200,7 +210,7 @@ class VoiceMessageHelper @Inject constructor( | |
try { | ||
val maxAmplitude = voiceRecorder.getMaxAmplitude() | ||
amplitudeList.add(maxAmplitude) | ||
playbackTracker.updateCurrentRecording(VoiceMessagePlaybackTracker.RECORDING_ID, amplitudeList) | ||
playbackTracker.updateCurrentRecording(AudioMessagePlaybackTracker.RECORDING_ID, amplitudeList) | ||
} catch (e: IllegalStateException) { | ||
Timber.e(e, "Cannot get max amplitude. Amplitude recording timer will be stopped.") | ||
stopRecordingAmplitudes() | ||
|
@@ -233,7 +243,7 @@ class VoiceMessageHelper @Inject constructor( | |
val currentPosition = mediaPlayer?.currentPosition ?: 0 | ||
val totalDuration = mediaPlayer?.duration ?: 0 | ||
val percentage = currentPosition.toFloat() / totalDuration | ||
playbackTracker.updateCurrentPlaybackTime(id, currentPosition, percentage) | ||
playbackTracker.updatePlayingAtPlaybackTime(id, currentPosition, percentage) | ||
} else { | ||
playbackTracker.stopPlayback(id) | ||
stopPlaybackTicker() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,4 +42,5 @@ sealed class MessageComposerAction : VectorViewModelAction { | |
data class EndAllVoiceActions(val deleteRecord: Boolean = true) : MessageComposerAction() | ||
data class VoiceWaveformTouchedUp(val eventId: String, val duration: Int, val percentage: Float) : MessageComposerAction() | ||
data class VoiceWaveformMovedTo(val eventId: String, val duration: Int, val percentage: Float) : MessageComposerAction() | ||
data class AudioSeekBarMovedTo(val eventId: String, val duration: Int, val percentage: Float) : MessageComposerAction() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering if this is really an action of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You make a strong point! It doesn't sound like this and some of the other voice actinos like the ones you mentioned and ones concerning play/pause functionality should belong here. Perhaps I could work it in another PR to separate these out a little bit |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use the Github issue number instead for the Changelog entry? I know there were some discussions about it, but I don't know what was decided. I think all the team should use use the same pattern: either Github issue number or PR number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I learned about this process, I heard it's very ambiguous and we go either way, but I agree that one way or the other we should choose one and be consistent with it!