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

Fix scoped storage issue #7

Closed
wants to merge 7 commits into from
Closed

Fix scoped storage issue #7

wants to merge 7 commits into from

Conversation

abkoradiya
Copy link
Contributor

@robertlevonyan @squti I have solved issue #6 using file descriptor and Uri. review it.

inputStream = File(filePath!!).inputStream()
} else {
val descriptor =
context.contentResolver.openFileDescriptor(fileUri!!, "rw")?.fileDescriptor

Choose a reason for hiding this comment

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

fileUri!! still with assertion

randomAccessFile.close()

if (filePath == null) {
val outputStream = context.contentResolver.openOutputStream(fileUri!!, "rw")

Choose a reason for hiding this comment

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

same on this line

outputStream?.write(header)
outputStream?.close()
} else {
val randomAccessFile = RandomAccessFile(File(filePath!!), "rw")

Choose a reason for hiding this comment

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

same on this line

val outputStream: OutputStream
// var length: Long
if (filePath != null) {
val file = File(filePath!!)

Choose a reason for hiding this comment

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

You can use filePath?.let { path -> ... } ?: run { ... } instead of if else to avoid assertion

@@ -170,7 +202,11 @@ class WaveRecorder(private var filePath: String) {
audioRecorder.stop()
audioRecorder.release()
audioSessionId = -1
WaveHeaderWriter(filePath, waveConfig).writeHeader()
if (filePath != null) {
WaveHeaderWriter(waveConfig, context).with(filePath!!).writeHeader()

Choose a reason for hiding this comment

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

same here

@@ -173,6 +186,17 @@ class MainActivity : AppCompatActivity() {
private const val TAG = "MainActivity"
}

override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
Copy link

@robertlevonyan robertlevonyan Nov 13, 2020

Choose a reason for hiding this comment

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

This is not something to change right now. But in newer versions of Android onActivityResult is deprecated, and in future versions it may be deleted at all. There is a new API for that ActivityResultCallback .

@squti
Copy link
Owner

squti commented Nov 18, 2020

Unfortunately I can't accept this PR. The reasons are below:
There is no explanation about different parts of added and changed codes
You didn't use kotlin features in some parts like checking null, it seems your code is not clean
I see irrelevant commits in your PR that aren't relate to issue.
I will appreciate that if you fix this problem and make a new PR.
Thanks for your precious time.

@squti squti closed this Nov 18, 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.

None yet

3 participants