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

Deezer flac seeking for IP3K #489

Merged
merged 2 commits into from Dec 27, 2020
Merged

Conversation

philippe44
Copy link
Contributor

@philippe44 philippe44 commented Dec 27, 2020

I don't know if you are interested but now that Deezer allows flac & seeking, we have the same issue as of Tidal and others, which is that flac seeking of HTTP streams does not work on IP3K players due to lack of frame alignement.

I'm not absolutely sure about the need of the DNS resolving, but is it correct to assume that because we'll do a header parsing in case we stream flac, then name will be solved "there" so it's not needed anymore.

It looks to me a 8.1.1 b/c it's not a new feature, but a fix. Still obviously your call

@philippe44 philippe44 changed the title flac seeking for IP3K Deezer flac seeking for IP3K Dec 27, 2020
@mherger mherger merged commit 85ac338 into LMS-Community:public/8.1 Dec 27, 2020
@philippe44
Copy link
Contributor Author

Did it look right? I've been doing a fair bit of mistakes ...

@michaelherger
Copy link
Member

It seems to be working ok. Tried with a couple of tracks on a Classic.

What I was wondering about was that async DNS lookup. I've never done that in recent implementations. But leaving it in shouldn't hurt.

@philippe44
Copy link
Contributor Author

I was not sure why it was here as no other PH that I've seen do that, but I thought that I would leave it only for mp3 when there is no header parsing.

One thing I was wondering is how people feel about proxying. With that, we proxy more often, every time seeking is involved, even when not needed (player does not need to receive aligned framed). There is an easy way around that which is to query, in canSongDirectStream, an additional function in the "processors" that can confirm that the processor really want to be used for that client and this seeking position. Now, I would tend to believe that

1/ most LMS hosts are very powerful nowadays
2/ most users don't care
3/ adding more complexity in code might not be a good idea. This whole transcode thing is complex, at least for me, and I've demonstrated enough difficulties to converge to something stable, so simpler is better, especially when the benefit is questionable - optimized results but only when seeking which likely does not happen often. I would think differently if proxy was always on.

Thoughts?

@michaelherger
Copy link
Member

The purist's discomfort 😀. I believe it's 2 because of 1. Most users really just want to listen to the music. They don't care about bandwidth use, CPU cycles etc., as long as the music plays. Except they're audiophiles who believe that any additional processing hurts sound quality. Except upsampling. But anyway: they're not the majority. 2 because of 1.

And this proxying only happens when seeking. Which again is something many probably never do. Plus https might require proxying in many cases anyway.

I'd therefore say proxying isn't an issue. Thanks for considering even more improvements. But that time probably is better spent somewhere else.

@philippe44
Copy link
Contributor Author

That sounds like the right think to do - thanks :-)

@philippe44 philippe44 deleted the Deezer branch December 27, 2020 19:08
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