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

Merge uiua-nokhwa with upstream? #651

Closed
dilr opened this issue Jan 24, 2025 · 5 comments
Closed

Merge uiua-nokhwa with upstream? #651

dilr opened this issue Jan 24, 2025 · 5 comments

Comments

@dilr
Copy link
Contributor

dilr commented Jan 24, 2025

I noticed that uiua-nokhwa is a fork of upstream nokhwa. I can't find find the reasoning for this in the issue tracker though. What originally caused the fork? Was nokhwa unmaintained at the time? Were dependencies conflicting? Assuming that the reason is resolvable or already resolved, I'd like to merge the two.

My reason is that in testing my latest PR, I noticed that the webcam feature does not work either before or after the PR for me. The issue is that the received image had more raw data than x times y times channels, so I could patch enough for testing by just truncating the image data. I did not include that change in my PR though, as really we should just receive the correct image data.

It looks like upstream nokhwa has completely rewritten the mac backend for version 0.11, and on their issue tracker they believe it will fix a similar issue someone else had. (Right now the main branch doesn't compile, so I can't test that though) Rather than me trying to track down the bug in the forked version of nokhwa, I'd rather spend the effort merging back any improvements from the Uiua to upstream and then getting Uiua to use upstream nokhwa.

Do you think that is a reasonable course of action? If so, could you give me an overview of what major changes the fork makes?

@kaikalii
Copy link
Member

I was under the impression that nokhwa was unmaintained.

@dilr
Copy link
Contributor Author

dilr commented Jan 24, 2025

The last commit was last week. So it seems like it is at least somewhat maintained as of now.

@dilr
Copy link
Contributor Author

dilr commented Jan 24, 2025

I assume that means the reason for the fork is resolved now. Looking at the uiua-nokhwa commit log, it seems like the only change is a name change.

Would you like me to update Uiua to use the latest stable version of nokhwa? If you want to maintain control of the fork, I could merge the latest stable changes from nokhwa into uiua-nokhwa and leave that as Uiua's dependency.

This won't solve my issue, which is probably solved by the next verision of nokhwa, but it will make it easier to transition when that release comes around. The latest stable version of nokhwa requires a later version of image than we use, and I've already sorted out how to get everything to compile on my system.

@kaikalii
Copy link
Member

The main change was updating dependencies to compile properly on some targets. It's possible this has been fixed in the main crate. You can try it.

@dilr
Copy link
Contributor Author

dilr commented Jan 24, 2025

Submitted PR #652. Closing this as further discussion should probably happen on the PR.

@dilr dilr closed this as completed Jan 24, 2025
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

No branches or pull requests

2 participants