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 true streaming #90

Merged
merged 1 commit into from
Feb 9, 2025
Merged

Conversation

rolandlo
Copy link
Collaborator

@rolandlo rolandlo commented Dec 23, 2024

Tests are currently restricted to writing to a file target. The script example/target.lua writes to stdout.
An illegal machine instruction issue with that script using Lua 5.4 has been solved by stopping/restarting garbage collection in Target.new_to_*.

Tests are still missing.

The script example/target.lua works with luajit and Lua 5.1, 5.2 and 5.3, but crashes with Lua 5.4 due to illegal machine instruction.

@rolandlo rolandlo marked this pull request as draft December 23, 2024 21:19
@jcupitt
Copy link
Member

jcupitt commented Dec 24, 2024

This is very cool!

The custom source / target classes would need the g_signal_connect() etc. mechanism to be wrapped and this is significantly harder because of callbacks, GC integration, and marshaling.

Signals are handy for other stuff --- pyvips exposes the progress signals, so you can have on_eval() etc. handlers which tick down during evaluation. They are useful for desktop or CLI programs but probably less so on servers heh.

@rolandlo rolandlo changed the title WIP: Add true streaming (initial commit) WIP: Add true streaming Dec 24, 2024
@rolandlo rolandlo force-pushed the add-true-streaming branch 5 times, most recently from 1efbfce to 5a84eca Compare February 4, 2025 09:23
@rolandlo
Copy link
Collaborator Author

rolandlo commented Feb 4, 2025

Basic true streaming (without custom classes) is working fine now with all Lua versions.

@jcupitt Could you please check my corresponding addition to the README.md 5a84eca. I hope I didn't write anything wrong there, so I'd be glad if you could check.

This commit allows creating image sources from descriptor, file or memory
and image targets to descriptor, file or memory.

Custom classes are not implemented
@rolandlo rolandlo marked this pull request as ready for review February 9, 2025 10:22
@rolandlo rolandlo changed the title WIP: Add true streaming Add true streaming Feb 9, 2025
@rolandlo rolandlo merged commit 25e7aaa into libvips:master Feb 9, 2025
13 checks passed
@rolandlo rolandlo deleted the add-true-streaming branch February 9, 2025 10:26
@jcupitt
Copy link
Member

jcupitt commented Feb 9, 2025

@rolandlo sorry, I'm travelling this week, I've not had a chance to do any work. Your README update looks fine, and nice job!

@rolandlo
Copy link
Collaborator Author

rolandlo commented Feb 9, 2025

@rolandlo sorry, I'm travelling this week, I've not had a chance to do any work. Your README update looks fine, and nice job!

Thanks for the feedback!

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.

2 participants