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

Select transcoding segment container, default to fmp4 #915

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

Chaphasilor
Copy link
Collaborator

This should hopefully fix #600, and maybe some other issues too.

Seems to work on Android (14), but it would be good if someone could give this a try on iOS and test with a Vorbis file to make sure this is indeed a fix.

Either way, it probably doesn't hurt to include this setting, even if it doesn't fix the issue.

- this should hopefully fix #600, and maybe some other issues too
@Chaphasilor Chaphasilor added enhancement Improves an existing feature playback Issues related to playback redesign-beta Issues related to the beta/redsigned version of Finamp labels Oct 8, 2024
@Chaphasilor Chaphasilor requested a review from jmshrv October 8, 2024 15:37
@Chaphasilor Chaphasilor linked an issue Oct 8, 2024 that may be closed by this pull request
@felix920506
Copy link

If I remember correctly there should be extra checks to prevent selecting MP3 codec with fMP4 container because that won't work on most devices.

Opus can also be streamed on newer iOS versions using fMP4 containers

@Chaphasilor
Copy link
Collaborator Author

@felix920506 thanks for the heads up! Right now Finamp only ever uses AAC for transcoding anyway, except for transcoded downloads (but those aren't streamed anyway). So this shouldn't be a problem.
Opus streaming on iOS sounds very interesting though! If there aren't any major issues with the fmp4 containers we could add more codec options for streaming in a future release.

@jmshrv did you manage to give this a quick test yet?

@jmshrv
Copy link
Owner

jmshrv commented Oct 22, 2024

This got lost in my emails sorry, will test :)

@felix920506
Copy link

Is there any update on this PR? Or is there anything I can do to help?

@Chaphasilor Chaphasilor added the hacktoberfest-accepted Issues accepted to count as a Hacktoberfest contribution label Oct 31, 2024
@Chaphasilor
Copy link
Collaborator Author

@felix920506 I still haven't heard from anyone who tested this on iOS, so if you could manage to do that, it would definitely help!

@felix920506

This comment was marked as outdated.

@Chaphasilor
Copy link
Collaborator Author

@felix920506 have you looked through CONTRIBUTING.md? Maybe there's some additional setup required that you haven't done yet?
Any error messages would also help. Are you using VS Code for development? The application logs should appear in the debug console when launching the app through the debugger

@felix920506

This comment was marked as outdated.

@felix920506

This comment was marked as outdated.

@felix920506
Copy link

Update: I was able to get into the main screen and settings after deleting the app from the emulator and rerunning it. The settings show up as expected. I will test playback later.
截圖 2024-10-31 下午2 32 03

@felix920506
Copy link

@Chaphasilor I tested playback on iOS in the Xcode emulator, iOS 18.0. Here are the results:

  • Playback starts normally and plays without issues.
  • Stuttering in ogg vorbis files no longer occurs.
  • Server logs show transcodes are correctly using .mp4 segments

@Chaphasilor
Copy link
Collaborator Author

Thanks for testing! I guess we can merge this then ^^

Now that DirectStreaming dropped with 10.10, do you think it might be worthwhile to add support for that, so we could use TS and fmp4 transports even when not transcoding?

@felix920506
Copy link

@Chaphasilor we could just move to using the UniversalAudio endpoint and the server will auto decide whether to send the source file or do a transcode.

@Chaphasilor
Copy link
Collaborator Author

How does it decide? Is there documentation about that? 🙃

@felix920506
Copy link

felix920506 commented Nov 1, 2024

It would probably be faster to ask on matrix, but I think that can be its own separate PR

@felix920506
Copy link

Is there anything holding this from getting merged?

@Chaphasilor
Copy link
Collaborator Author

Nope, just forgot about it while waiting for the automatic tests to finish ^^
Thanks for the reminder :D

@Chaphasilor Chaphasilor merged commit 88d611e into redesign Nov 6, 2024
8 checks passed
@Chaphasilor Chaphasilor deleted the enable-fmp4-transcoding-container branch November 6, 2024 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves an existing feature hacktoberfest-accepted Issues accepted to count as a Hacktoberfest contribution playback Issues related to playback redesign-beta Issues related to the beta/redsigned version of Finamp
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Audio stuttering when transcode playing ogg vorbis on iOS
3 participants