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

Show recording duration and waveform #4241

Merged
merged 17 commits into from Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
Expand Up @@ -57,6 +57,9 @@ jobs:
- run:
name: Run async unit tests
command: ./gradlew -PdisablePreDex --no-daemon --max-workers=2 async:testDebug
- run:
Copy link
Member Author

Choose a reason for hiding this comment

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

Now that we have a test file in strings we should really be running it on CI.

name: Run strings unit tests
command: ./gradlew -PdisablePreDex --no-daemon --max-workers=2 strings:testDebug
- run:
name: Run material unit tests
command: ./gradlew -PdisablePreDex --no-daemon --max-workers=2 material:testDebug
Expand Down
Expand Up @@ -8,36 +8,14 @@ import androidx.work.OneTimeWorkRequest
import androidx.work.PeriodicWorkRequest
import androidx.work.WorkInfo
import androidx.work.WorkManager
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.Runnable
import kotlinx.coroutines.cancel
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import java.util.concurrent.TimeUnit
import java.util.function.Consumer
import java.util.function.Supplier
import kotlin.coroutines.CoroutineContext

class CoroutineAndWorkManagerScheduler(private val foregroundContext: CoroutineContext, private val backgroundContext: CoroutineContext, private val workManager: WorkManager) : Scheduler {
class CoroutineAndWorkManagerScheduler(foregroundContext: CoroutineContext, backgroundContext: CoroutineContext, private val workManager: WorkManager) : CoroutineScheduler(foregroundContext, backgroundContext) {

constructor(workManager: WorkManager) : this(Dispatchers.Main, Dispatchers.IO, workManager) // Needed for Java construction

override fun <T> immediate(background: Supplier<T>, foreground: Consumer<T>) {
CoroutineScope(foregroundContext).launch {
val result = withContext(backgroundContext) { background.get() }
foreground.accept(result)
}
}

override fun immediate(foreground: java.lang.Runnable) {
CoroutineScope(foregroundContext).launch {
foreground.run()
}
}

override fun networkDeferred(tag: String, spec: TaskSpec) {
val constraints = Constraints.Builder()
.setRequiredNetworkType(NetworkType.CONNECTED)
Expand Down Expand Up @@ -65,19 +43,6 @@ class CoroutineAndWorkManagerScheduler(private val foregroundContext: CoroutineC
workManager.enqueueUniquePeriodicWork(tag, ExistingPeriodicWorkPolicy.REPLACE, workRequest)
}

override fun repeat(foreground: Runnable, repeatPeriod: Long): Cancellable {
val repeatScope = CoroutineScope(foregroundContext)

repeatScope.launch {
while (isActive) {
foreground.run()
delay(repeatPeriod)
}
}

return ScopeCancellable(repeatScope)
}

override fun cancelDeferred(tag: String) {
workManager.cancelUniqueWork(tag)
}
Expand All @@ -97,11 +62,3 @@ class CoroutineAndWorkManagerScheduler(private val foregroundContext: CoroutineC
return false
}
}

private class ScopeCancellable(private val scope: CoroutineScope) : Cancellable {

override fun cancel(): Boolean {
scope.cancel()
return true
}
}
55 changes: 55 additions & 0 deletions async/src/main/java/org/odk/collect/async/CoroutineScheduler.kt
@@ -0,0 +1,55 @@
package org.odk.collect.async

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.isActive
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import java.util.function.Consumer
import java.util.function.Supplier
import kotlin.coroutines.CoroutineContext

open class CoroutineScheduler(private val foregroundContext: CoroutineContext, private val backgroundContext: CoroutineContext) : Scheduler {
Copy link
Member Author

Choose a reason for hiding this comment

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

We won't always need deferred tasks so thought I'd split the implementations. I think long term it might make sense to split the interfaces but lets see how it evolves.


override fun <T> immediate(background: Supplier<T>, foreground: Consumer<T>) {
CoroutineScope(foregroundContext).launch {
val result = withContext(backgroundContext) { background.get() }
foreground.accept(result)
}
}

override fun immediate(foreground: Runnable) {
CoroutineScope(foregroundContext).launch {
foreground.run()
}
}

override fun repeat(foreground: Runnable, repeatPeriod: Long): Cancellable {
val repeatScope = CoroutineScope(foregroundContext)

repeatScope.launch {
while (isActive) {
foreground.run()
delay(repeatPeriod)
}
}

return ScopeCancellable(repeatScope)
}

override fun networkDeferred(tag: String, spec: TaskSpec) {
throw UnsupportedOperationException()
}

override fun networkDeferred(tag: String, spec: TaskSpec, repeatPeriod: Long) {
throw UnsupportedOperationException()
}

override fun cancelDeferred(tag: String) {
throw UnsupportedOperationException()
}

override fun isRunning(tag: String): Boolean {
throw UnsupportedOperationException()
}
}
12 changes: 12 additions & 0 deletions async/src/main/java/org/odk/collect/async/ScopeCancellable.kt
@@ -0,0 +1,12 @@
package org.odk.collect.async

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.cancel

internal class ScopeCancellable(private val scope: CoroutineScope) : Cancellable {

override fun cancel(): Boolean {
scope.cancel()
return true
}
}
Expand Up @@ -68,10 +68,10 @@ class AudioClipViewModelTest {
@Test
fun playMultipleClips_updatesProgress_forAllClips() {
viewModel.play(Clip("clip1", "file://audio.mp3"))
assertThat(fakeScheduler.checkRepeatRunning(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(true))
viewModel.onCleared()
viewModel.play(Clip("clip1", "file://audio.mp3"))
assertThat(fakeScheduler.checkRepeatRunning(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(true))
}

@Test
Expand All @@ -88,14 +88,14 @@ class AudioClipViewModelTest {
val onCompletionListener = captor.value
verify(mediaPlayer).setDataSource("file://audio1.mp3")
verify(mediaPlayer, times(1)).start()
assertThat(fakeScheduler.checkRepeatRunning(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(true))
onCompletionListener.onCompletion(mediaPlayer)
verify(mediaPlayer).setDataSource("file://audio2.mp3")
verify(mediaPlayer, times(2)).start()
assertThat(fakeScheduler.checkRepeatRunning(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(true))
onCompletionListener.onCompletion(mediaPlayer)
verify(mediaPlayer, times(2)).start()
assertThat(fakeScheduler.checkRepeatRunning(), equalTo(false))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(false))
}

@Test
Expand Down Expand Up @@ -204,7 +204,7 @@ class AudioClipViewModelTest {
fun background_cancelsScheduler() {
viewModel.play(Clip("clip1", "file://audio.mp3"))
viewModel.background()
assertThat(fakeScheduler.hasBeenCancelled(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(false))
}

@Test
Expand Down Expand Up @@ -317,7 +317,7 @@ class AudioClipViewModelTest {
fun onCleared_cancelsScheduler() {
viewModel.play(Clip("clip1", "file://audio.mp3"))
viewModel.onCleared()
assertThat(fakeScheduler.hasBeenCancelled(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(false))
}

@Test
Expand All @@ -326,7 +326,7 @@ class AudioClipViewModelTest {
val captor = ArgumentCaptor.forClass(MediaPlayer.OnCompletionListener::class.java)
verify(mediaPlayer).setOnCompletionListener(captor.capture())
captor.value.onCompletion(mediaPlayer)
assertThat(fakeScheduler.hasBeenCancelled(), equalTo(true))
assertThat(fakeScheduler.isRepeatRunning(), equalTo(false))
}

@Test
Expand Down
1 change: 1 addition & 0 deletions audiorecorder/build.gradle
Expand Up @@ -47,6 +47,7 @@ dependencies {
implementation 'com.google.android.material:material:1.2.1'
implementation project(':strings')
implementation "com.google.dagger:dagger:${rootProject.daggerVersion}"
implementation project(path: ':async')
testImplementation project(path: ':testshared')
kapt "com.google.dagger:dagger-compiler:${rootProject.daggerVersion}"

Expand Down
Expand Up @@ -7,6 +7,9 @@ import dagger.BindsInstance
import dagger.Component
import dagger.Module
import dagger.Provides
import kotlinx.coroutines.Dispatchers
import org.odk.collect.async.CoroutineScheduler
import org.odk.collect.async.Scheduler
import org.odk.collect.audiorecorder.recorder.MediaRecorderRecorder
import org.odk.collect.audiorecorder.recorder.RealMediaRecorderWrapper
import org.odk.collect.audiorecorder.recorder.Recorder
Expand Down Expand Up @@ -70,6 +73,11 @@ internal open class AudioRecorderDependencyModule {
open fun providesRecordingRepository(): RecordingRepository {
return RecordingRepository()
}

@Provides
open fun providesScheduler(application: Application): Scheduler {
return CoroutineScheduler(Dispatchers.Main, Dispatchers.IO)
}
}

internal fun RobolectricApplication.clearDependencies() {
Expand Down
Expand Up @@ -51,6 +51,9 @@ internal class MediaRecorderRecorder(private val cacheDir: File, private val med
file?.delete()
}

override val amplitude: Int
get() = mediaRecorder?.getMaxAmplitude() ?: 0

override fun isRecording(): Boolean {
return mediaRecorder != null
}
Expand Down
Expand Up @@ -18,6 +18,7 @@ internal interface MediaRecorderWrapper {
fun start()
fun stop()
fun release()
fun getMaxAmplitude(): Int
}

class RealMediaRecorderWrapper(private val mediaRecorder: MediaRecorder) : MediaRecorderWrapper {
Expand Down Expand Up @@ -61,4 +62,8 @@ class RealMediaRecorderWrapper(private val mediaRecorder: MediaRecorder) : Media
override fun release() {
mediaRecorder.release()
}

override fun getMaxAmplitude(): Int {
return mediaRecorder.maxAmplitude
}
}
Expand Up @@ -7,6 +7,7 @@ internal interface Recorder {
fun stop(): File
fun cancel()

val amplitude: Int
fun isRecording(): Boolean
}

Expand Down
Expand Up @@ -7,12 +7,12 @@ import java.io.File

/**
* Interface for a ViewModel that records audio. Can only record once session
Copy link
Member Author

Choose a reason for hiding this comment

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

I think we're now in a better place with this abstraction. Adding time/amplitude helped it make more sense as it forced the key data exposed to be the current "session" details.

* at a time but supports cases where multiple views can start/playback different
* recordings through a `sessionsId` passed to `start` and `getRecording`.
* at a time.
*/
abstract class AudioRecorderViewModel : ViewModel() {
abstract fun isRecording(): LiveData<Boolean>
abstract fun getRecording(sessionId: String): LiveData<File?>
abstract fun isRecording(): Boolean
abstract fun getCurrentSession(): LiveData<RecordingSession?>

abstract fun start(sessionId: String, output: Output)
abstract fun stop()

Expand All @@ -22,3 +22,5 @@ abstract class AudioRecorderViewModel : ViewModel() {
*/
abstract fun cleanUp()
}

data class RecordingSession(val id: String, val file: File?, val duration: Long, val amplitude: Int)