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 raw AAC (ADTS) file support #956

Merged
merged 3 commits into from Dec 16, 2023

Conversation

philippe44
Copy link
Contributor

Currently, AAC files format defaults to Movie which is mp4, not raw aac (with ADTS) So files with .aac extension won't play and won't be scanned properly. This PR adds independent .aac support, including the option to use id3. There is a micro-modification of Slim::Formats::MP3 (no functional change, just a separation of mp3 info/tag getter) to maximize code reuse. This addition should not change anything if scanBitrate is called from another class either.

@philippe44
Copy link
Contributor Author

philippe44 commented Dec 16, 2023

argh... I went a bit too fast and did not realize there is partial support for aac in Slim:Format::Movie. It actually fails to play but it works just for the scanner. The problem is that the search for frame boundaries and getting an initialblock are only for mp4 and croaks on aac. The problem is that some people might call their mp4 files with aac inside ".aac", which is a wrong habit, but I guess that happens.

There are 2 options: either incorporae the aac handling inside "Movie" or strip it from partial aac handling, add "AAC" as an independent class but check that it's not an mp4 file and then redirect to "Movie".

I prefer option 2 because it seems more clean and there are cases where #1 will not work properly: for example the "volatileAudioBlock" cannot differentiate between the two. I'd rather be forgiving on naming aac files with .mp4 extension and strict about the other way around. A file named .mp4 is no way a raw ADTS file.

I'll think about it more tomorrow

@philippe44
Copy link
Contributor Author

Finally I opted for the solution 1 as it ended being more simple and less intrusive to the codebase. It might be a bit less logical (aac should not be taken care of by the mp4 handler), but that's likely a matter of opinion.

@mherger mherger merged commit 5e095bf into LMS-Community:public/8.4 Dec 16, 2023
@mherger
Copy link
Contributor

mherger commented Dec 16, 2023

I trust your judgement 😁. Thanks a lot!

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

2 participants