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

Attempting to Load Image Just Acquired Using saveFrame() Does Not Work #4218

Closed
ghost opened this Issue Jan 1, 2016 · 11 comments

Comments

Projects
None yet
3 participants
@ghost

ghost commented Jan 1, 2016

In Processing 3.0.1, attempting to load an image that was just saved using saveFrame() does not work, and I get the following error.

The file "C:/fakepath/image.png" is missing or inaccessible, make sure the URL is valid or that the file has been added to your sketch and is readable.

Here's a sample sketch that will reproduce the problem:

File image = new File("C:/fakepath/image.png");
String imageString = "C:/fakepath/image.png";
PImage pimageV;

void setup()
{
size(700, 600);
saveFrame(imageString);
pimageV = requestImage(imageString);
}

In this example sketch, I have attempted to load the image using requestImage(), although the problem is the same if you try to use loadImage().

Prior to Processing 3.0.1, I used Processing 2.2.1 and did not experience any issues with this. I do not know if this issue was present in the initial release of Processing 3.

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar Jan 1, 2016

Contributor

Hi @DavidB-TPW, please use hint(DISABLE_ASYNC_SAVEFRAME); in your setup() after the size()call. Saving now happens asynchronously on a background thread and the image is not finished saving when you try to load it.

@benfry Do you think it would make sense to make async save a separate function (like loadImage() vs. requestImage())? I've already seen several people struggling with this. Then again, many people would probably never know about it, and I think majority would want to use it by default even if they wouldn't understand the difference.

Contributor

JakubValtar commented Jan 1, 2016

Hi @DavidB-TPW, please use hint(DISABLE_ASYNC_SAVEFRAME); in your setup() after the size()call. Saving now happens asynchronously on a background thread and the image is not finished saving when you try to load it.

@benfry Do you think it would make sense to make async save a separate function (like loadImage() vs. requestImage())? I've already seen several people struggling with this. Then again, many people would probably never know about it, and I think majority would want to use it by default even if they wouldn't understand the difference.

@JakubValtar JakubValtar added the core label Jan 1, 2016

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 1, 2016

@JakubValtar Thanks! Problem solved! I was wondering if the function was somehow modified to be asynchronous, but was not sure how I could resolve the problem.

ghost commented Jan 1, 2016

@JakubValtar Thanks! Problem solved! I was wondering if the function was somehow modified to be asynchronous, but was not sure how I could resolve the problem.

@garciadelcastillo

This comment has been minimized.

Show comment
Hide comment
@garciadelcastillo

garciadelcastillo Jan 18, 2016

@JakubValtar @benfry I just ran into this issue. I agree with Ben, it is probably more convenient to leave saveFrame() async behavior by default, which will benefit 98% of the cases... But then it creates a weird asymmetry with loadImage() default sync behavior. What would such a synced saveFrame() function be called? snapFrame()?

loadImage();
requestImage();
saveFrame();
snapFrame();

Node.js solves this issue by adding a suffix to the synced version of the function (most functions are async by default). This way, the four functions would look like:

loadImage();
loadImageAsync();
saveFrame();
saveFrameSync();

In Processing's syntactical spirit, this is probably better resolved with constants and overloads, which could potentially be propagated to other functions:

loadImage("foo.png");
loadImage("foo.png", ASYNC);
saveFrame("bar.png");
saveFrame("bar.png", SYNC);

I would probably opt for the latter ;)

garciadelcastillo commented Jan 18, 2016

@JakubValtar @benfry I just ran into this issue. I agree with Ben, it is probably more convenient to leave saveFrame() async behavior by default, which will benefit 98% of the cases... But then it creates a weird asymmetry with loadImage() default sync behavior. What would such a synced saveFrame() function be called? snapFrame()?

loadImage();
requestImage();
saveFrame();
snapFrame();

Node.js solves this issue by adding a suffix to the synced version of the function (most functions are async by default). This way, the four functions would look like:

loadImage();
loadImageAsync();
saveFrame();
saveFrameSync();

In Processing's syntactical spirit, this is probably better resolved with constants and overloads, which could potentially be propagated to other functions:

loadImage("foo.png");
loadImage("foo.png", ASYNC);
saveFrame("bar.png");
saveFrame("bar.png", SYNC);

I would probably opt for the latter ;)

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 20, 2016

@garciadelcastillo What would be the advantage of using the latter option?

ghost commented Jan 20, 2016

@garciadelcastillo What would be the advantage of using the latter option?

@garciadelcastillo

This comment has been minimized.

Show comment
Hide comment
@garciadelcastillo

garciadelcastillo Jan 20, 2016

@DavidB-TPW I find it more coherent with the way the Processing syntax is designed. Plus it establishes a SYNC/ASYNC logic that could be easily implemented into other functions (loadShape(), loadFont(), etc.)

garciadelcastillo commented Jan 20, 2016

@DavidB-TPW I find it more coherent with the way the Processing syntax is designed. Plus it establishes a SYNC/ASYNC logic that could be easily implemented into other functions (loadShape(), loadFont(), etc.)

@benfry benfry added the high label Feb 13, 2016

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry May 9, 2016

Member

@JakubValtar How about we block until completion of all saveFrame() calls as soon as loadImage() is called?

Member

benfry commented May 9, 2016

@JakubValtar How about we block until completion of all saveFrame() calls as soon as loadImage() is called?

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar May 9, 2016

Contributor

@benfry Hm, this could work. With a small change in async saver I can have a Map<String, Future> from filenames to running tasks and we can directly block on those. Sometimes you want to process huge batch of images and blocking on any save would lower the throughput a lot.

Contributor

JakubValtar commented May 9, 2016

@benfry Hm, this could work. With a small change in async saver I can have a Map<String, Future> from filenames to running tasks and we can directly block on those. Sometimes you want to process huge batch of images and blocking on any save would lower the throughput a lot.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry May 10, 2016

Member

Yeah, but I'm thinking that if you're trying to do a loadImage() inside of that, you're a tiny, tiny fraction, and probably deserve what you get. That is, if you're doing something that intensive with images, requestImage() or the hint() call are more appropriate.

Member

benfry commented May 10, 2016

Yeah, but I'm thinking that if you're trying to do a loadImage() inside of that, you're a tiny, tiny fraction, and probably deserve what you get. That is, if you're doing something that intensive with images, requestImage() or the hint() call are more appropriate.

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar May 10, 2016

Contributor

I mean, we want the filename check because we don't want to block loadImage for no reason. It's like four lines of code.

Contributor

JakubValtar commented May 10, 2016

I mean, we want the filename check because we don't want to block loadImage for no reason. It's like four lines of code.

@benfry

This comment has been minimized.

Show comment
Hide comment
@benfry

benfry May 10, 2016

Member

Eh, but why even bother? I don't think we need to add the complexity, but if you're up for it, just make sure you're using ConcurrentHashMap or something similar so that we're not introducing another race condition.

Member

benfry commented May 10, 2016

Eh, but why even bother? I don't think we need to add the complexity, but if you're up for it, just make sure you're using ConcurrentHashMap or something similar so that we're not introducing another race condition.

@JakubValtar

This comment has been minimized.

Show comment
Hide comment
@JakubValtar

JakubValtar May 11, 2016

Contributor

Because this would disable async image saver when processing batch of images (any loadImage would block until previous image completes saving). This can hurt performance a lot. Why have the async image saver if we are not going to use it?

Contributor

JakubValtar commented May 11, 2016

Because this would disable async image saver when processing batch of images (any loadImage would block until previous image completes saving). This can hurt performance a lot. Why have the async image saver if we are not going to use it?

@benfry benfry closed this in #4465 May 11, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment