search

bakape / thumbnailer

Go media thumbnailer
MIT License
152 stars 36 forks source link

Rewrite thumbnailer in Rust #34

Closed Chiiruno closed 5 years ago

Chiiruno commented 5 years ago

I'll do this, and probably some parts of meguca/hydron too afterwards so we don't have to worry about memory bloat. This'll take a little while, but I guess give me collaborator permissions on this repo and assign me to this issue. In the meantime, could you give me some ideas on how thumbnailer could be improved, as well as your thoughts on whether or not we should have different libraries to use?

bakape commented 5 years ago

I'm already planning to rewrite the thumbnailer in C.

Chiiruno commented 5 years ago

I would do that in C++ at least, for the smart pointers. That said, honestly I feel Rust might be a better option overall. Thoughts?

bakape commented 5 years ago

Not much point in C++ as most of the code will just be shuffling C APIs. Rust is also pretty bad at interacting with C (compared to C++ and Go, anyway). If you r want to, of course you can write the glue, once I have the C rewrite done.

On 23 September 2018 at 01:29, チルノ notifications@github.com wrote:

I would do that in C++ at least, for the smart pointers. That said, honestly I feel Rust might be a better option overall. Thoughts?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bakape/thumbnailer/issues/34#issuecomment-423777870, or mute the thread https://github.com/notifications/unsubscribe-auth/AHfPsPsUd8RGwnhHSmW6SjTcKjUocxauks5udrmtgaJpZM4W1dv6 .

Chiiruno commented 5 years ago

Rust has some crates for this though, so that shouldn't be an issue. See:

Not sure about the graphicsmagick one, thoughts?

bakape commented 5 years ago

I've tried some of them. Don't remember why I did not use them and just went with C instead. See, you will have to use C APIs, export a C API and write some logic code. With Rust, the FFI glue with likely take most of your code and your logic would still be mostly unafe and thus not protected by Rust compile-time checks.

On 23 September 2018 at 01:36, チルノ notifications@github.com wrote:

Rust has some crates for this though, so that shouldn't be an issue. See:

Not sure about the graphicsmagick one, thoughts?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bakape/thumbnailer/issues/34#issuecomment-423778233, or mute the thread https://github.com/notifications/unsubscribe-auth/AHfPsCAd4LfxZrv5oFLLBHnqvc5rzDwfks5udrthgaJpZM4W1dv6 .

Chiiruno commented 5 years ago

I'll definitely look into this further, and if that really is the case, I'll close this issue. I'll also see if there aren't any better rust-specific libraries for this we might be able to use. I'm doing this to git gud at Rust anyway, so I guess once it's done we can compare and go with whatever works better. (Probably yours honestly, you're a better programmer than me.)

Chiiruno commented 5 years ago

Actually, I don't see any particular reason we couldn't just use this library: https://crates.io/crates/image Have thumbnailer streamline it I guess?

bakape commented 5 years ago

I don't see any particular reason to use it either.

Chiiruno commented 5 years ago

Well, we (I) wouldn't have to worry about C FFI as much, although I feel that isn't even so much a problem with the crates that exist, sans graphicsmagick.

bakape commented 5 years ago

Can you reach functional parity with the current thumbnailer only through existing crates?

On 24 September 2018 at 02:16, チルノ notifications@github.com wrote:

Well, we (I) wouldn't have to worry about C FFI as much, although I feel that isn't even so much a problem with the crates that exist, sans graphicsmagick.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/bakape/thumbnailer/issues/34#issuecomment-423855398, or mute the thread https://github.com/notifications/unsubscribe-auth/AHfPsDKSI86rz8Cqp-UQxlFJkU285t1vks5ueBZXgaJpZM4W1dv6 .

Chiiruno commented 5 years ago

I think so, yes, but I can't say for certain right now. Once I begin implementing, I'll know for sure.