Support for SoundCloud AAC HQ streams (Go+)#3401
Support for SoundCloud AAC HQ streams (Go+)#3401mgranell wants to merge 4 commits intomusic-assistant:devfrom
Conversation
mgranell
force-pushed
the
feat/soundcloud-aac-hq
branch
2 times, most recently
from
ce5bf21 to
762665d
Compare
mgranell
force-pushed
the
feat/soundcloud-aac-hq
branch
from
762665d to
6ee3b26
Compare
| f"FFmpeg version {version} is not supported. " | ||
| f"Minimal version required is {MINIMAL_FFMPEG_VERSION}." | ||
| ) | ||
| if version != "N": # N-versions are development versions, so we assume they are recent enough |
There was a problem hiding this comment.
What is this about? MA already builds with ffmpeg 7. The version check exists for a reason — bypassing it for 'N-versions' with no lower bound is unsafe. If you need a dev build, it should still be a recent ffmpeg 7+ dev build, and the version string parsing should handle that properly rather than skipping the check entirely
| label="Authorization", | ||
| required=True, | ||
| ), | ||
| ConfigEntry( |
There was a problem hiding this comment.
Regarding the two experimental config entries — your own descriptions label these as experimental, which suggests this may not be ready for users yet. Can you explain the reasoning behind exposing both options? Ideally we'd want to settle on the best-performing option as a default and not expose the protocol preference to users at all. Is there a reason that's not possible here?
| if content_type == ContentType.AAC | ||
| else ContentType.MP3 | ||
| if content_type == ContentType.MP3 | ||
| else ContentType.OGG, |
There was a problem hiding this comment.
Is OGG the only remaining format SoundCloud delivers, and if so can you add a comment explaining that? Because without that context it looks like a bug.
| if protocol == "progressive" | ||
| else StreamType.HLS | ||
| if protocol == "hls" | ||
| else StreamType.UNKNOWN |
There was a problem hiding this comment.
Can protocol ever be anything other than progressive or hls? If not, the UNKNOWN branch is dead code and should be removed. If it can, what should MA actually do in that case?
|
Also there are conflicts to be resolved |
2 participants
Not Ready to Merge
This is initial support for high quality streams (up to 256 kb/s), available to Go+ subscribers.
Issue:
Earlier builds of ffmpeg (e.g. ffmpeg 4) are extremely slow to open progressive m4a aac streams for soundcloud, which can be 10 seconds or more. Using a latest source code compile of ffmpeg has lowest latency with HLS (starting in a couple of seconds), and slightly longer for progressive.
I have added an initial configuration option to enable the AAC support, as well as a configuration option to prefer HLS to progressive for testing purposes in different environments.
Welcome any comments or discussion on the PR.
Thanks