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

Napster support #506

Merged
merged 1 commit into from Jan 8, 2021
Merged

Napster support #506

merged 1 commit into from Jan 8, 2021

Conversation

philippe44
Copy link
Contributor

The format should stay "mp4"

The format should stay "mp4"
@mherger
Copy link
Contributor

mherger commented Jan 8, 2021

Is this a new behaviour, thanks to your recent changes? I wouldn't have added the override hadn't it been required in the past...

@mherger mherger merged commit 268019c into LMS-Community:public/8.1 Jan 8, 2021
@philippe44
Copy link
Contributor Author

philippe44 commented Jan 8, 2021

Yes it was required previously.

With the override, aac was the format passed to the transcoder rule searcher, as if it was the source format of the stream. Rules searched were "aac xxx" and it worked because most "mp4 xxx" have an "aac xxx" version as well (except that the mp4 ones allow in addition START & END that aac can't). It worked, but it was incorrect in the sense that mp4 was the real format but mp4/aac transcoders usually accept both.

With the changes I've made and for consistency, the format given to the transcoder searcher is always the true source format. When there is a processor, the rule searcher is called with the original format first and then with all the formats processors might spit out. Then the ultimately selected input format is recorded (potentially after processor) in the $song object and the STRM_S message sender, when it sees that it's about to send mp4 or aac looks into that to verify what is the real format.

That's why Tidal does not have any more formatOverride but I missed the Napster one. I've now searched through the whole code and don't think there is any left, but we might have to tell plugin developers (although no user has reported anything yet) that if their ProtocolHandler sends mp4, please don't name it aac using formatOverride. Hope it will not happen or we might have version compatibility issue.

Long explanation, but also helps me reviewing the changes :-)

@michaelherger
Copy link
Member

Thanks - much appreciated!

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

3 participants