processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.61k stars 3.31k forks source link

saveFrame() implementation #326

Closed juansrx closed 10 years ago

juansrx commented 10 years ago

Hello again.

Looking at the current API documentation I noticed that there is not a saveFrame() function, Do you need help with the implementation or this was left outside intentionally.

Thank you again.

lmccart commented 10 years ago

@therewasaguy where are we now with the save() stuff? I think we came to the conclusion that there would be one save method that would save out the right type of file depending on input. do you see a need for saveFrame as well, or how would these two relate?

brysonian commented 10 years ago

saveFrame is useful when making animations/videos of an app since you can put in the #### to have the current frame number inserted into the filename. I'm not sure how it would work on the web though, you wouldn't want to download a png of each frame as the app is running.

therewasaguy commented 10 years ago

@lmccart I haven't made any updates to save methods since last week, but it sounds from this thread like everyone is on the same page that a single save method would be a good idea. The way I was thinking to implement it would be to keep all of the existing methods, and decide which one to use based on the input parameters/file format/options. Should I give this a shot?

My only concern about combining all the save methods is that the documentation page would have a lot to document, because this one method could save anything from an HTML Table to a .wav soundfile to an image...

For now, there is already a `save() method that only saves an image in a pop-up window. I didn't get a chance to work on this method, but would be happy to make it prompt the download like the other save methods.

@juansrx saveFrame() functionality would be great to implement, but, to @brysonian's point, it could be tricky...for now, especially since save() creates a popup window, it's definitely not a good idea to save a frame every run through the draw loop. But maybe there is a better way to handle this... @juansrx do you have any ideas?

shiffman commented 10 years ago

saveFrame() is generally to used to output an image sequence to render into a movie. Maybe for the web it would be simpler to write to a video stream (that could be saved/downloaded) directly?

This might be a horrifying question but is there ever a scenario where there would be "application" features for when you are using p5 in the IDE and you would somehow secretly save files without a "pop-up"? This remind me of the old days when the Processing included an "environment" field in the reference: "web" or "application" etc.

juansrx commented 10 years ago

Actually...I was thinking about the save() function (the one that saves a "screenshot" of your canvas), about the saveFrame function I think you are right: generating many image files may not be the best option.

Generating an animated GIF may require an external library (and dealing with the fact that is a GIF), creating a set of downloads only change the problem into generating a lot of download forms. ¿Maybe creating a set of "slides" into the same generated image?.

@shiffman I'm not big into this subject and I don´t know much about the IDE yet, but as far as I know, to just save a file the server holding the file must send in the response header a content type attribute to specify that the file is an attachment. And even so it might ask if you want to save it.

therewasaguy commented 10 years ago

I started experimenting with a saveFrames method in this branch https://github.com/therewasaguy/p5.js/blob/saveFrames/src/output/image.js

You give it a frameRate and duration, and when it's done, all of the files download at once (rather than downloading each frame as it is saved, which interrupts the animation loop). In Firefox a dialogue appears asking your permission to download each frame, in Chrome they just download, in Safari it only downloads one of them. Even when it works, it is kind of frightening when the files suddenly start to download. So maybe this shouldn't be implemented...

But, if we were to add the WebP format for images (developed by Google, only supported in a few browsers) it looks like it would be possible to take all of the images and convert them into a WebM movie using this library Whammy. I wonder if there is anything else out there like this that could make a movie from png or jpeg.

Alternately, saveFrames could send the image data to a server that creates the video. So it could come in handy for a future version of the IDE, or there could be a hook for users who want to send the data to a server.

brysonian commented 10 years ago

Maybe a zip file of the images using something like http://stuk.github.io/jszip/ ?

Having the function generate a movie may be close to the most common original use, but isn't exactly what is expected. Perhaps in that case removing it in favor something akin to MovieMaker (RIP).

lmccart commented 10 years ago

@therewasaguy the conclusion for save() sounds good, and your plan for implementation makes sense. if you want to go ahead and give it a shot please do. I think the documentation will be ok, we just need to be careful about the formatting and organization of it so people can understand all that save does. happy to take a look at this with you, too.

in terms of the frames... maybe something that is separate from the core like moviemaker is better suited to do this sort of task. then we could build in some more control of the rendering. this kind of thing sort of sounds suited for phantomjs, but then I guess then we'd be talking server side, which we haven't figured out a real protocol for interacting with yet. though we will want to if we want to support kinect and things eventually..

therewasaguy commented 10 years ago

@lmccart cool, I'll get the global save method working, draft the documentation, and I'll let you know when it's ready— I'll appreciate some feedback on that!

therewasaguy commented 10 years ago

here's a pull request with the new save method https://github.com/lmccart/p5.js/pull/334

lmccart commented 10 years ago

also wondering to what extent we should reveal the verbose specific save methods. right now in the reference I'm not showing saveStrings, saveJSON, saveTable, etc to simplify things. does this make sense? should we also remove the .save() method in p5.Image and the .saveSound() method in p5.SoundRecorder?

therewasaguy commented 10 years ago

I think it's important to be able to see that the sounds you record with SoundRecorder can be saved. But this could just be accomplished with the inline example.

...do we need a single load() method - instead of loadJSON, loadTable, loadImage, loadXML - to match save()?

lmccart commented 10 years ago

I think you're right, let's keep object specific save methods in the ref. Are you ok removing saveStrings etc? I could go either way but feel this is a little less overwhelming maybe?

Hmm load(), interesting... I think this makes sense, does anyone see any potential issues?

therewasaguy commented 10 years ago

I like the idea of a pointer to the canvas, it would definitely make this method more consistent. Maybe a string that would be the canvas ID, like save('defaultCanvas', 'myImage.jpg') ?

lmccart commented 10 years ago

I was thinking a pointer to the actual object, like...

var cnv = createCanvas(100, 100);
save(cnv, 'myCanvas.jpg');
brysonian commented 10 years ago

my concern about load is the ambiguity in what is being loaded. How would the function know if it should parse the contents as JSON or CSV or just "Strings"? It could be file extension, but that always leads to hacks like sticking dummy get variables at the end of a url to force it to be read a certain way. I've had that issue with loadImage in processing even, when trying to load from a url that doesn't have an extension. I feel like at the point of loading it helps to be clear in the code what your intentions are, have all that surfaced, whereas with save() it is clear that you want to save the thing, and you know what the thing is, or at least know that you can trust processing to know how to save the thing.

shiffman commented 10 years ago

I would keep the load methods the same. Also re: save() are we keeping the save() methods in the specifics object themselves, i.e. table.save() or img.save() etc. The global save() is great but I wonder if these object ones are actually more clear for teaching/examples etc.?

lmccart commented 10 years ago

good point @brysonian, let's stick with the explicit load methods then.

@shiffman for the save methods, I think we've arrived at keeping the object specific save methods img.save() etc, but removing the top level specific ones saveStrings() etc. though right now they are exposed, just not represented in the documentation.

shiffman commented 10 years ago

perfect!

lmccart commented 10 years ago

so I think this one is mostly set, with the possible exception of changing the signature for saving canvas with specified filename to align more closely with the rest of the two argument signatures from:

save('myFile.jpg');

to:

var cnv = createCanvas(100, 100);
save(cnv, 'myCanvas.jpg');

note: the default save() would remain which saves the canvas and lets the user input the filename in the dialog box.

saveFrame() is effectively removed, though we could have it throw a warning directing the user to look up save() instead.

@therewasaguy does this sound about right? if you're on board with this last change but busy, I'm happy to take a look, too.

therewasaguy commented 10 years ago

@lmccart this sounds good, just sent a pull request.

I think it would still be useful to be able to say

lmccart commented 10 years ago

I think your intuition is right, allowing the string argument aligns with the processing api, I'll comment it back in. thanks for the pr, looks great.