-
Notifications
You must be signed in to change notification settings - Fork 5
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
How to format incoming requests w multiple images? #6
Comments
@tech4GT how did you address this in the initial implementation? Thanks!! |
Hi Jeff, right now I am taking the urls as an array in the request json and the steps are taken as a sequencer string. |
Oh ok and how do we map from the array to the steps? I guess we can just do
it manually for now and figure out a flexible architecture later. Thanks!
…On Tue, Mar 19, 2019, 9:06 AM Varun Gupta ***@***.***> wrote:
Hi Jeff, right now I am taking the urls as an array in the request json
and the steps are taken as a sequencer string.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ_DkcPpobzLEv2T0ehNsnAfNf0M0ks5vYODNgaJpZM4b8CMa>
.
|
Is there a way to compress the images if they exceed a limit. Maybe a size limit like 20MB? |
@jywarren For now I used a common string but I'll update it to this following architecture {
// request body
images: [
{
url: <URL>
steps: <String>
},
{
url: <>
steps: <>
}
]
} |
We can then expand each of the images with as much metadata as we want like coordinates and orientation. |
Oh but the images could be very large, we're only transmitting their URLs
on the initial request. I think they may be over 100mb once downloaded.
…On Tue, Mar 19, 2019, 9:25 AM Varun Gupta ***@***.***> wrote:
We can then expand each of the images with as much metadata as we want
like coordinates and orientation.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJzohcH5eKe-4P_6GuPeXt1lQWBYRks5vYOU7gaJpZM4b8CMa>
.
|
But it's gonna be that anyway, right? I mean if we transfer the image in part initially from the client side, then too the total data remains the same, I think the best option we have is to compress them in mapknitter and host those images on the urls. We can then create a step like decompress which gets the original image back. |
Ah I see. But the image may be remotely stored in aws or Google cloud, so
we should standardize on sending the URLs first. Later I agree we could
think of transmission speed optimization by compressing, but we might be
able to use on-the-fly gzip in nginx configuration, and do it at that level
instead of building it into the architecture. Sound good?
…On Tue, Mar 19, 2019, 9:42 AM Varun Gupta ***@***.***> wrote:
But it's gonna be that anyway, right? I mean if we transfer the image in
part initially from the client side, then too the total data remains the
same, I think the best option we have is to compress them in mapknitter and
host those images on the urls. We can then create a step like decompress
which gets the original image back.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ4XqIeVHqhvimSvt9dTx_9xhw_oGks5vYOkqgaJpZM4b8CMa>
.
|
Yeah this sounds good to me!! 👍 |
Hi @jywarren , I updated the api structure and now on using this url |
I think this completes our part 1 majorly? Should I get started on the stitch module using https://github.com/publiclab/mapknitter-exporter/blob/main/lib/mapknitterExporter.rb#L252 as reference?? |
OK, this is good; however, I wonder if we should give each incoming image
an id, and actually make it a step, in an associative array, so:
/api/v1/process/?steps={1:{"url":"./Images/Mario.png","sequence":"invert"},2:{"url":"./Images/Mario.png","sequence":"colormap"}}
Is this valid JSON?
But then we could add steps that run once other ones are done, but we need
to think how we'd represent this... could it be like:
/api/v1/process/?steps={1:{"url":"./Images/Mario.png","sequence":"invert"},2:{"url":"./Images/Mario.png","sequence":"colormap"}},3:{'after':
1,"sequence":"composite{image:$1},composite{image:$2}"}
What do you think? any suggestions?
…On Fri, Mar 22, 2019 at 4:39 AM Varun Gupta ***@***.***> wrote:
Hi Jeff, I updated the api structure and now on using this url
http://localhost:4000/api/v1/process/?images=[{
"url":"./Images/Mario.png","sequence":"invert"},{"url":"./Images/Mario.png","sequence":"colormap"}]
this is the result:
[image: Screen Shot 2019-03-22 at 2 08 08 PM]
<https://user-images.githubusercontent.com/25617855/54810366-f381cf00-4cab-11e9-9fc9-bcc8364b10be.png>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJxiM8yBMDiyzd2yYW5Mne5QdPqqPks5vZJa4gaJpZM4b8CMa>
.
|
Hmm, this sounds right! Maybe what we can do is have a common sequence that runs once others are done and then every image can have a sequence property which is specific to that image {
images: [
{
url: <>,
sequence: <>
}
],
sequence: <Common sequence>
} |
Once we get this sorted we can move on to part 2 finally 😃 |
I like separating steps from images because we can then use images more
than once, you know? And if we mark some steps as depending on others,
using their unique IDs, we can later optimize which can run in parallel.
What do you think?
…On Sun, Mar 24, 2019, 2:48 AM Varun Gupta ***@***.***> wrote:
Once we get this sorted we can move on to part 2 finally 😃
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ8GpP1NUZgiaMqrXJgWJtPmJTlZuks5vZx-dgaJpZM4b8CMa>
.
|
Hmm, Sure! So should I update it according to the format you suggested earlier? |
I'm thinking,
1. we can have a trigger that runs at the start and whenever a step
completes,
2. then we check each next step with a conditional that checks if it's
prerequisites are complete, and starts them if so
3. when they complete, they also check remaining steps in order to see if
their prerequisites are complete
I'm brainstorming here, so maybe we need to think this through more before
implementing, but what do you think here?
…On Sun, Mar 24, 2019 at 12:23 PM Varun Gupta ***@***.***> wrote:
Hmm, Sure! So should I update it according to the format you suggested
earlier?
One thing I can't wrap my head around though, is that how would we use
these IDs to generate the dependency info?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJzodTdOFtpZdFC7tqXv3yJvbkX8rks5vZ6ZpgaJpZM4b8CMa>
.
|
Hello Jeff, sorry for the late reply! Okay this sounds good!!
|
This makes a lot of sense, I was meaning to ask that if we don't mind using some extra memory, I can implement a stripped down version of the |
So the API request will kind of look like {
images: [
{
<ID>
<pre>
<post>
<sequence>
<requisites>
}
],
common_sequence: <String>
upload: <Boolean>
redirect: <Boolean>
} |
cc @jywarren |
OR
|
One thing is that the frontend will also have to be developed to calculate the no of requests and no of images per request and to distribute those images accordingly. |
Idea 2
|
So one thing re: upload time is that we'll probably start by simply passing
a JSON post request to this app with URLs for each of the images, rather
than passing the full images in the request. Then the app can fetch them
when it wants. We might add ways to POST images up later, but this will be
simpler to start with.
directed graph data structure
Whoa, so would this be able to figure out the optimal pairings of image
merges that could be done in the "tree" diagram I posted here?
publiclab/mapknitter#298
If so, then GREAT! Although we again can start by just working linearly,
but this JSON structure for submitting requests can be easily adapted to
such a sorting system later if we use this "requisites" idea. Great!!!
Do you think it's better to just have a flat set of steps, instead of
pre/post for each image? I'm on the fence, but maybe we could plan to add
pre/post later since it is nice but adds a layer of complexity.
the server can distribute the work itself between different threads and
provide the full output at once
Exactly, this is great Harsh - yes, this app should be able to send Google
Cloud function calls to do some tasks off-app and parallelize!
Thanks all!!!
…On Wed, Mar 27, 2019 at 9:47 AM Harsh Khandeparkar ***@***.***> wrote:
- If the frontend isn't developed for this kind of behaviour, a single
request with all the imgs can also be taken.
- So the server has to get the no of images i and the the number of
requests r. If the number of requests is small, each request should be
allowed a smaller no of images for the asynchronous processing to work
efficiently. If there is a single request, no of imgs per request can be
more as only a single thread will be utilised for it and no performance
will be reduced.
Idea 2
- If the inet speed of the user is fast and the user uploads a lot of
images as once in the same request, the server can distribute the work
itself between different threads and provide the full output at once
(because inet speed is fast, that won't be a bottleneck).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ05ABr0LVZXW2N2rr-n3BDWMkpXPks5va3ZVgaJpZM4b8CMa>
.
|
Where would the URLs link to? Some other site like imgur? If yes, how will the user upload images to that site? Wouldn't that be a bottleneck? |
Also is there a way to "stream" the images? I mean the rgba data values for each pixel can be sent in a stream for a max of a few imgs at once. The output can also be received in a stream. |
My concern is that process time(local processing time) should not be less than the upload time for multiple imgs so parallelizing upload, processing and download I think can be a good way to do this. I'm not sure though. |
OK, removing the parameters, to just use the default
But it did not succeed:
|
OK, also opened publiclab/image-sequencer#1033 and publiclab/image-sequencer#1032 which also need to be solved before this will run. 🎉 |
Once we can confirm this is running, we'll need to make a converter function from the
|
OK, well -- with a small image, this works!!!
Input: Output: Here's a better test image: OK! We had a JSON parse error but it seems to relate to the encoding of the punctuation marks; this URL will bypass it: It's not completing... but it meets a timeout:
anyways, next step would be to try to pull in the output of one sequencer into the other, with a sequence like: Actually, with the function code @tech4GT has written, we should be able to point at the output of the earlier step with a number as input, and then use overlay: |
Trying something that doesn't require Puppeteer, to confirm that we can do this Update: OK! Got an error for the data-url here:
Update: haha, cool, that data-url wasn't an image -- it displayed:
|
@jywarren Is the parsing error specific to google cloud or are we facing it locally too? |
Can you guide me to replicate the error locally? |
I found that I could get it to happen on Google Cloud, but ALSO using the
CLI interface locally.
…On Wed, May 8, 2019 at 12:09 PM Varun Gupta ***@***.***> wrote:
Can you guide me to replicate the error locally?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAF6J6I7AMHSZ7U3WYYK4DPUL3KNANCNFSM4G7QEMNA>
.
|
publiclab/image-sequencer#1033 shows how to replicate it in the CLI mode - it gives the same error, so it seems likely it's the same parsing issue! |
I'm sorry - wait. The error is not the same. But @VibhorCodecianGupta has gone into some detail on the parsing in there, so there is more context available in publiclab/image-sequencer#1033. |
So, i'd go by this comment to replicate it: #6 (comment) And I bet you could get this running locally: https://gist.github.com/jywarren/7721a38725affdbc4fad2598ff1be59e |
@jywarren So to sum this up, the problem is with using |
Okay I have isolated this error to our import string functions. Okay I'll make sure to write some tests for the string parsing code to prevent this in the future. 👍 |
Also @jywarren I found out why the topological sort wasn't working! Actually that one is my fault since I didn't factor in that the graph might have nodes which are entirely disconnected. The library we are using does not support such nodes so all we need to do make that work is add those nodes in a loop. I'll make the necessary changes in a separate PR. 🎉 |
Oh this is great to hear you reproduced it! |
Also this issue with the data url returned -- after these two fixes, would you mind taking a look? Thanks! |
And Varun, if we get this version of multi-sequencer (from cloud functions) running in a Docker container, we can move it to a non Cloud Functions part of Google Cloud and get longer runtimes and more memory. I think @icarito can help us with this. So, let's aim to get all these bugs fixed and we'll ask @icarito for some help getting this booting in a docker container? |
Sure! The project is already set up with docker, so it shouldn't be a problem adding the multi sequencer! 😄 |
Okay finally figured this out! The commas in the input for webgl-distort are messing with the commas separating steps! |
@jywarren the problem is with the sequencer string not being URL safe. So how importString works is the inputs to every step, that is the stuff between |
I did it this way to make sure that complete functions can be passed in as arguments like in the blend step. 😄 |
Okay also closing this up, since we have fixed the API structure and implemented parallel processing! |
Should it be a Json format with an array of steps, and should we start by importing a list of URLs and assigning them short variable names in the Json?
Or should we basically be submitting a Js function that runs using an array of URLs as additional input?
The text was updated successfully, but these errors were encountered: