plietar / librespot

Open Source Spotify client library
MIT License
1.14k stars 185 forks source link

Switch to lewton #195

Closed est31 closed 7 years ago

est31 commented 7 years ago

Hi, just have published the 0.6.0 release of lewton. It has the seeking support we spoke about and supports stable Rust (1.18). Also, there is an async mode, which doesn't support seeking though. Maybe its time to switch to lewton? If you see any blockers, please say so!

wfraser commented 7 years ago

Doesn't support soft float ARM yet :-/

est31 commented 7 years ago

@wfraser yeah, there is no support for fixed point math, but for that one you seem to keep tremor around. You can still replace the vorbis dep with lewton, no?

plietar commented 7 years ago

Yes, I'm definitely planning on switching to lewton in the near future, I'm just too busy with other stuff (aka graduating) for the moment. Probably sometime within the next month.

@wfraser I'm planning on keeping tremor support around for this.

I might also support libogg at least for sometime, since it's basically no extra maintenance once I support tremor. lewton will be the default however.

I'm not too sure what to do about async yet. I'd like to use it, but it might be make it complicated to support both async lewton and non-async tremor. At first I'll probably stick with synchronous, and later see if and how I could support async.

@est31 What is missing to lewton to have seeking in asynchronous mode ? Is there any chance lewton might support a fixed point mode eventually ? This aren't high priority requirements for switching to lewton, but would definitely be nice

est31 commented 7 years ago

What is missing to lewton to have seeking in asynchronous mode?

Two things:

It took me quite a bit of time to refactor the ogg crate to support async the tokio way (not the hackish way I had it before), seeking is involving far more back and forth of system calls, so it might be even harder to port.

IDK, given your statement async seek support will be mostly useless to you anway until there is fixed point support? I guess I'll give that one a try first...

plietar commented 7 years ago

how can I use tokio-io and futures traits to express such an API

This is maybe something that should be brought up to tokio authors. They might have ideas on what an AsyncSeek trait would look like.

IDK, given your statement async seek support will be mostly useless to you anway until there is fixed point support? I guess I'll give that one a try first...

When using tremor, I can probably offload the ogg decoding to a separate thread, with a channel sending decoded packets back to the main thread.

In any case, even ignoring tremor, async requires some bigger changes in librespot so it will probably come later.

plietar commented 7 years ago

Actually thinking about it I'll probably keep libogg the default for some time. I prefer not to require a too recent stable version.

I'll probably switch to lewton as the default in one rust release cycle (6 weeks). After this I'll encourage people with old compilers to use libogg/tremor, which requires me to support at least one of them even if lewton were to get fixed point support and asynchronous seeking. I'll drop support in two or three cycles.

plietar commented 7 years ago

Lewton support was added in d1447d2bfb88db6fb9b9293b22c1a96d8d4dccbe. Build with cargo build --features "with-lewton" to enable.

I'll enable it as the default when Rust 1.20 comes out at the end of the month.

Thanks for your great work @est31