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

Add pause/unpause to recording #4256

Merged
merged 8 commits into from Dec 3, 2020
Merged

Add pause/unpause to recording #4256

merged 8 commits into from Dec 3, 2020

Conversation

seadowg
Copy link
Member

@seadowg seadowg commented Dec 2, 2020

Closes #4234
Blocked by #4241

The interface isn't the greatest right now (switches between Pause/Resume with same icon) but will iterate on this when we do design polish.

What has been done to verify that this works as intended?

New tests and verified manually.

Why is this the best possible solution? Were any other approaches considered?

Just uses the Android MediaRecorder to pause so no big changes here. Annoyingly this doesn't work below API 24. Additional comments inline.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Definitely think we should leave QA on this until we have the "finished" (ready for release) feature set.

Before submitting this PR, please make sure you have:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@seadowg seadowg added the blocked label Dec 2, 2020
@@ -65,7 +65,7 @@ internal open class AudioRecorderDependencyModule {

@Provides
open fun providesRecorder(application: Application): Recorder {
return MediaRecorderRecorder(application.cacheDir) { RealMediaRecorderWrapper(MediaRecorder()) }
return RecordingResourceRecorder(application.cacheDir) { MediaRecorderRecordingResource(MediaRecorder()) }
Copy link
Member Author

Choose a reason for hiding this comment

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

Naming is hard. I'm attempting to make it clear that this could be implemented using something other than MediaRecorder. It's the direction I think we'd need to take if we look at a lossless codec.

@seadowg seadowg removed the blocked label Dec 2, 2020
@seadowg seadowg marked this pull request as ready for review December 2, 2020 18:29
@lognaturel lognaturel merged commit 9e1ae28 into getodk:master Dec 3, 2020
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.

Can pause and unpause recording
2 participants