jeffbass / imagezmq

A set of Python classes that transport OpenCV images from one computer to another using PyZMQ messaging.
MIT License
1.01k stars 160 forks source link

Update README and docs/more-details.rst #15

Closed bigdaddymax closed 4 years ago

bigdaddymax commented 4 years ago

Update a README, add info about PUB/SUB capabilities, add a simple use case with example and explanation for PUB/SUP pattern.

jeffbass commented 4 years ago

Thanks for your pull request. I am going to create a new branch "add-pub-sub" to allow testing and refinement of your code. I should have the new branch created in a few days, and will send you instructions on how to move your pull requests to that branch. Thanks for your patience!

jeffbass commented 4 years ago

I've added a feature branch named "add-pub-sub" to the imagezmq repository. Could you please move your pull request to that branch? Instructions here. I believe adding PUB/SUB is a useful feature and I appreciate your contribution. Let's develop your contribution in the "add-pub-sub" branch until it is refined and tested. After development and testing, I will merge it to the master branch. Thanks!

bigdaddymax commented 4 years ago

Done.

I will also add a separate document with a more comprehensive description of differences between REQ/REP and PUB/SUB patterns and use cases.

Thank you.

jeffbass commented 4 years ago

Thanks. That is a GREAT next step.

bigdaddymax commented 4 years ago

Agreed, when I was writing the readme I had the feeling that it touches too many other concepts and ideas.

Let me split that into two (or more?) subchapters and examples.

bigdaddymax commented 4 years ago

Hi Jeff,

I have updated documentation so it reflects changes in the library and tests, also added advanced test files that demonstrate headless non-blocking implementation.

Please check and let me know if anything else should be fixed or changed.

Thanks.

Maksym

jeffbass commented 4 years ago

Hi Maksym,

Great work. I have merged your pull requests into the add-pub-sub feature branch. Next steps for me:

Next steps for you:

Thanks again for your great work. I will add to this thread when I have questions and comments. Jeff

jeffbass commented 4 years ago

Hi Maksym, I apologize that I am taking so long with this testing and merge. I have been dealing with some medical stuff and some travel. I have gotten back to work on this project now and expect to have testing and merge done in next 7 days. Jeff

bigdaddymax commented 4 years ago

No need to apologize - take your time and let me know if you need anything.

By the way, I have found out one potential problem with PUB/SUB scenario, which was not obvious until I started testing different scenarios (and had a bug in publisher that just spammed my subscriber).

When a subscriber is slower then publisher, unprocessed messages are aggregating at the subscriber socket and basically causing a memory leak.

So I am thinking of adding some code to help slow subscriber clean a queue if it doesn't keep up.

If you have any ideas I would gladly listen.

Thanks, Maksym

bigdaddymax commented 4 years ago

And I will do timing tests - somehow I have missed this request, sorry for that.

jeffbass commented 4 years ago

Hi Maksym, I finished my testing and all my existing programs work well with your PUB / SUB code, so I have merged the add-pub-sub branch into master. Thanks for your good work! You now are listed as a contributor in the master branch. You are my first contributor for a significant change to the imagezmq package, and I am going to add a Contributors section to the main README.rst. It could be a simple table: Contribution | Name | Github
Original imagezmq code & docs | Jeff Bass | @jeffbass Added PUB / SUB capability | Maksym | @bigdaddymax Web image stream example | Maksym | @bigdaddymax

Let me know if you are OK with that or if you would prefer some other way of being listed (or if you would rather not be listed at all; that's OK, too.)

bigdaddymax commented 4 years ago

Hi Jeff, thank you so much!

Everything is fine with me.

This is actually my very first contribution to an open-source project and I am very excited :)

I am still going to add those tests that I promised, I will create a pull request for that.

Thanks again!

jeffbass commented 4 years ago

Hi Maksym, I added a "contributors" section to the README.rst and added your name & github link to it. I also did some re-wording and further description of the PUB/SUB option. I will be adding other changes to the README.rst to reflect your other improvements as well. I really like what you did with the "frame numbering" in test_1. It is a significant improvement over the "green / red box" I had in test_1 before. As I make other improvements to the various documentation files, feel free to correct me if I don't describe your PUB/SUB option and your http streaming example the way you would like them described. Thanks again for all your great work! Jeff