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

Recordings are not as long as they should be #81

Closed
7 tasks done
RustoMCSpit opened this issue Jan 15, 2025 · 26 comments · Fixed by #86
Closed
7 tasks done

Recordings are not as long as they should be #81

RustoMCSpit opened this issue Jan 15, 2025 · 26 comments · Fixed by #86
Assignees
Labels
ASAP Issue needs to be fixed as soon as possible. bug Something is not working

Comments

@RustoMCSpit
Copy link

Checklist

  • I can reproduce the bug with the latest version given here.
  • I made sure that there are no existing issues - open or closed - to which I could contribute my information.
  • I made sure that there are no existing discussions - open or closed - to which I could contribute my information.
  • I have read the FAQs inside the app (Menu -> About -> FAQs) and my problem isn't listed.
  • I have taken the time to fill in all the required details. I understand that the bug report will be dismissed otherwise.
  • This issue contains only one bug.
  • I have read and understood the contribution guidelines.

Affected app version

1.1.0

Affected Android/Custom ROM version

GrapheneOS

Affected device model

Pixel 6a

How did you install the app?

GitHub releases

Steps to reproduce the bug

  1. Press record
  2. Save recording (the UI looks fine, I may have made a 4 minute recording and it'll look like that when I press save)
  3. Find recording in files
  4. See that the recording cuts off at about six seconds

This bug went unnotived because I couldn't see playback due to the other bug. This app does not save data correctly and there needs to be a warning put out.

Expected behavior

For the app to properly save data

Actual behavior

Files saved at six seconds

Screenshots/Screen recordings

No response

Additional information

No response

@RustoMCSpit RustoMCSpit added bug Something is not working needs triage Issue is not yet ready for PR authors to take up labels Jan 15, 2025
@RustoMCSpit
Copy link
Author

@naveensingh urgent attention required. this bug is very bad.

@RustoMCSpit
Copy link
Author

I figured it out, if the recording is ever paused it saves. Everything after the pause isn't recorded

@RustoMCSpit
Copy link
Author

I figured it out, if the recording is ever paused it saves. Everything after the pause isn't recorded

Incorrect, sorry I assumed this was it.

@Aga-C
Copy link
Member

Aga-C commented Jan 15, 2025

  1. What file format are you using?
  2. Have you closed the app or switched to another app after six seconds from starting recording?

@Aga-C Aga-C added question Further information is requested waiting for author If the author does not respond, the issue will be closed. Otherwise, the label will be removed. labels Jan 15, 2025
@naveensingh
Copy link
Member

What file format are you using?

20 bucks says it's MP3.

@RustoMCSpit
Copy link
Author

yes mp3

@RustoMCSpit
Copy link
Author

  1. What file format are you using?
  2. Have you closed the app or switched to another app after six seconds from starting recording?

mp3 and no

@naveensingh naveensingh removed question Further information is requested waiting for author If the author does not respond, the issue will be closed. Otherwise, the label will be removed. labels Jan 15, 2025
@Aga-C Aga-C added ASAP Issue needs to be fixed as soon as possible. and removed needs triage Issue is not yet ready for PR authors to take up labels Jan 15, 2025
@RustoMCSpit
Copy link
Author

RustoMCSpit commented Jan 15, 2025

in my opinion, you should release an update immediately which is just version 1.0.0 until this is fixed. people have this app attached to obtainium and it will auto update. i know this is an embarrassing thing to do but this is a de facto silent permanent file destroyer and a lot of people are losing extremely important recordings every moment this isnt patched and i can easily see them abandoning this project permanently because of it in upset. it also gives time to work on #80

@naveensingh
Copy link
Member

naveensingh commented Jan 15, 2025

It happens on version 1.0.0 as well, the bug has always been there. Here's how to reproduce:

  • Change recordings folder
  • Change format to MP3
  • Record something
  • Do anything that would refresh the recordings list

@RustoMCSpit
Copy link
Author

then thats it because thats what i did

@RustoMCSpit
Copy link
Author

RustoMCSpit commented Jan 15, 2025

what format should i use and should i go back to the previous version

@naveensingh
Copy link
Member

naveensingh commented Jan 15, 2025

M4A should work fine. MP3 might work properly on the previous version if you don't change the recordings folder.

This bug is there in the last simple version as well.

@RustoMCSpit
Copy link
Author

can i get mp3 working fine on this version then or no?

@RustoMCSpit
Copy link
Author

does opus work on this version?

@naveensingh
Copy link
Member

naveensingh commented Jan 15, 2025

can i get mp3 working fine on this version then or no?

I'm working on it. If you absolutely need MP3 working now, use the "Keep screen on" option and keep the app in foreground.

does opus work on this version?

Yes.

@RustoMCSpit
Copy link
Author

can i get mp3 working fine on this version then or no?

I'm working on it. If you absolutely need MP3 working now, use the "Keep screen on" option and keep the app in foreground.

does opus work on this version?

Yes.

does opus work in the last version, i am migrating back to it if so due to #83 FossifyOrg/Commons#52 (comment)

@naveensingh
Copy link
Member

Last I checked, it was working.

@RustoMCSpit
Copy link
Author

RustoMCSpit commented Jan 15, 2025

i mean version 1.0.0, i'll have to migrate back until the data format thing is changed

@naveensingh
Copy link
Member

naveensingh commented Jan 15, 2025

Yeah, I checked again on 1.0.0, only MP3 is broken.

I have marked MP3 format as experimental until it is fixed properly:

https://github.com/FossifyOrg/Voice-Recorder/releases/tag/1.1.1
https://github.com/FossifyOrg/Voice-Recorder/releases/tag/1.1.2

@RustoMCSpit
Copy link
Author

RustoMCSpit commented Jan 15, 2025

for 1.1.1, you should make a notification in the app telling people that mp3 is experimental, not just changing the string of text

@RustoMCSpit
Copy link
Author

Yeah, I checked again on 1.0.0, only MP3 is broken.

I have marked MP3 format as experimental until it is fixed properly:

https://github.com/FossifyOrg/Voice-Recorder/releases/tag/1.1.1

could you classify 1.1.0 as a pre-release and mention in its release notes that mp3 is broken

@naveensingh
Copy link
Member

could you classify 1.1.0 as a pre-release

How would that help if this bug (among others) is also there in 1.0.0?

@naveensingh naveensingh self-assigned this Jan 16, 2025
@RustoMCSpit
Copy link
Author

i guess in an ideal world we'd classify both as pre releases but theyre the only releases besides the string patch

@naveensingh
Copy link
Member

naveensingh commented Jan 16, 2025

It turns out, the recording file's descriptor lifecycle was not handled properly so it was being invalidated prematurely. This is only reproducible on Android 11 and above because the code assumes direct file access on Android 8, 9 and 10.

This was less noticeable on version 1.0.0 because storage access framework wasn't used until you change the recordings folder. Still, I just don't know how this was never reported because the faulty code has been there since the (real) MP3 support was added. Anyway, here's a test APK:

voicerecorder-4-foss-release.apk.zip (remove the .zip, do not extract).

This APK basically has the following change:

val fileDescriptor = ParcelFileDescriptor.dup(originalFileDescriptor)
originalFileDescriptor.close()
// continue using fileDescriptor

@naveensingh
Copy link
Member

i guess in an ideal world we'd classify both as pre releases but theyre the only releases besides the string patch

In an ideal world, you wouldn't need Fossify ;)

@naveensingh
Copy link
Member

I just don't know how this was never reported because the faulty code has been there since the (real) MP3 support was added.

Maybe it was reported: SimpleMobileTools/Simple-Voice-Recorder#185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASAP Issue needs to be fixed as soon as possible. bug Something is not working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants