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 instructions to bundle alac #1505

Closed
wants to merge 1 commit into from

Conversation

outsharded
Copy link

Added note on alac decoder, and instructions to add the decoder

Added note on alac decoder, and instructions to add the decoder
@mikebrady
Copy link
Owner

mikebrady commented Jul 19, 2022

Thanks for the suggestion. However, there are a couple of comments that I'd like to make about it.

  1. Shairport Sync already has an ALAC decoder built in, so the Apple ALAC decoder is an option. It's not actually needed -- as your text would seem to imply -- for Shairport Sync to be able to decode ALAC-encoded material.

  2. Experimentally, it was worth adding the Apple ALAC decoder as an option:

    1. It was not known whether the Apple decoder would decode material better than the built-in decoder, written by David Hammerton.
    2. The Apple decoder appears to be capable of handling different numbers of channels, bit depths and rates.

    As it turns out the Hammerton decoder has not, to my knowledge, been found to decode incorrectly and those extra formats are not used in AirPlay 2 or AirPlay.

  3. The Apple decoder doesn't seem to be noticeably more or less efficient than the Hammerton decoder.

  4. The Apple ALAC decoder is known to have security issues, and it doesn't seem to be maintained upstream any more. David Hammerton has provided the ALAC decoder in the FFmpeg decoder collection. Since FFmpeg is maintained, there is a better chance that security and other issues will be fixed there over time. In fact, I'd be hoping to add the FFmpeg ALAC decoder as a configuration option.

So it seems to me that we should not be drawing attention to the Apple ALAC decoder at this time. In fact, if anything, we should be de-emphasising or even deprecating it in the near future.

@outsharded outsharded closed this Sep 7, 2022
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.

2 participants