-
As I mentioned in 4. here: https://en.sfml-dev.org/forums/index.php?topic=24998.msg166535#msg166535
There's no way to save image to a memory buffer.
There's also no way to save image to different extension than the one given in the filename (so you can't do a custom extension since no matching extension = fail).
There's also no warning anywhere about this funny thing: https://en.sfml-dev.org/forums/index.php?topic=24996.msg166513#msg166513
-
There's no way to save image to a memory buffer.
Yes, it should definitely be added. I think it's not hat hard to do since stb_image supports saving with custom write callbacks.
There's also no way to save image to different extension than the one given in the filename (so you can't do a custom extension since no matching extension = fail).
Could be done as well, yes. But then we'd have to support custom extensions for everything that can be saved (audio, for example).
There's also no warning anywhere about this funny thing: https://en.sfml-dev.org/forums/index.php?topic=24996.msg166513#msg166513
True. But I wonder if someone ever knew about it :P
-
Yes, it should definitely be added. I think it's not hat hard to do since stb_image supports saving with custom write callbacks.
Yes, it does, but the API is gonna be a problem as always. :-X
Could be done as well, yes. But then we'd have to support custom extensions for everything that can be saved (audio, for example).
The format specifier of some kind is needed for the save-file-to-memory anyway.
True. But I wonder if someone ever knew about it :P
;D >:(
-
Yes, it does, but the API is gonna be a problem as always.
What do you mean?
The format specifier of some kind is needed for the save-file-to-memory anyway.
Good point.
-
What do you mean?
API is always so contentious.
Do we return a vector, take a vector to fill, let user handle the callback themselves (thus having a void * in API - yay) or let them pass a memory buffer and size and fail/ignore stbi writes mid way if the space runs out?
I'd honestly not mind one of the vector ones since it's not like this would be a common operation.
-
Or define sf::OutputStream and its File/Memory derived classes, just like sf::InputStream, and provide saveToStream instead.
-
Sure, sure. And I ask for this to close this asymmetry of "we can load from user stream or file already memory but save only to a file". This will support stuff like saving image via PhysicsFS, to SQLite or some archive, optimizing or converting the file on your own in memory before saving, sending it somewhere, etc.
-
Actually, since MemoryInputStream doesn't copy the data (why isn't that an option honestly?) it can't work the same as MemoryOutputStream because they'd have same 'what if we run out of space' problem.
-
I would obviously offer two ways to construct an OutputMemoryStream: from external memory (with max size) or allocate its own memory dynamically. But I'm afraid lower level libraries such as stb_image won't be able to handle dynamic memory output, nor predicting how much space will be required in advance, so we'd certainly have to stick to fixed size memory stream anyway. It definitely requires more investigation.
-
No? STBI write has functions to write data to a callback. I'd not ask before checking (and it's only sane way to do this in C anyway, other than calculating memory size but that's problematic and STB had a write up about that somewhere).
typedef void stbi_write_func(void *context, void *data, int size);
STBIWDEF int stbi_write_png_to_func(stbi_write_func *func, void *context, int w, int h, int comp, const void *data, int stride_in_bytes);
STBIWDEF int stbi_write_bmp_to_func(stbi_write_func *func, void *context, int w, int h, int comp, const void *data);
STBIWDEF int stbi_write_tga_to_func(stbi_write_func *func, void *context, int w, int h, int comp, const void *data);
STBIWDEF int stbi_write_hdr_to_func(stbi_write_func *func, void *context, int w, int h, int comp, const float *data);
STBIWDEF int stbi_write_jpg_to_func(stbi_write_func *func, void *context, int x, int y, int comp, const void *data, int quality);
-
?
What I meant is that these functions can't be used with dynamic memory, as they won't allow you to allocate memory when needed, nor tell you how much memory you'll need in advance. You'll need to allocate a (probably very) big chunk of memory to be safe, and adjust afterwards.
-
You can pass a pointer to std::vector or something in the context pointer (that's your 'user data' to cast back to whatever and use in the callback that gets called with bunches of data from stbi repeatedly) and keep appending your bytes in the callback to that.
Should I write an example..?
-
Sorry, I had something else in mind. Of course you're right.
-
Time to design an output stream then?
-
Yes. My plan for SFML 3 was to replace our custom base classes with standard streams (std::istream and std::ostream), but I feel really bad now when I tell someone to wait for SFML 3 ;D
-
You could start now, because SFML 3 comes after SFML 2.6 and we don't really need to wait to get the first things started for SFML 3 IMHO. ;)
-
SFML 3 comes after SFML 2.6
Really? Who's working on it? :P
we don't really need to wait to get the first things started for SFML 3
? Nothing is ready for working properly on SFML 3.
-
See https://github.com/SFML/SFML/issues/988. It links to a corresponding PR, and a discussion about sf::OutputStream.
-
An advantage of adding sf::OutputStream before SFML 3 would be that we might get feedback to improve. If the API is not ideal initially, we're not stuck with it forever.
-
If you want to get anything in before SFML 3, then start working on it, as SFML 2.6 will be the last SFML 2.x release.
I feel like some of you may have missed the planned road map: https://en.sfml-dev.org/forums/index.php?topic=24372.0 :D
-
An advantage of adding sf::OutputStream before SFML 3 would be that we might get feedback to improve. If the API is not ideal initially, we're not stuck with it forever.
I would really love to see that in 2.x (if 2.6 shall be last 2.x release, then in 2.6 ;-) ). At the moment I need to write a tmp file and the read it again for further usage. Obviously that is not ideal.
I don't feel experienced enough in C++ development and SFML in specific to get enganged in neither the discussion regarding implementation and nor in contributing code. However I'd be more than happy to give feedback!
-
Today I solved my issue by using stb_image_write.h in my code in order to save images into memory without use of temporary files. Therefore my urgent need is gone, although I sill think it would be a great addition to SFML if it was possible directly using SFML API.
Again, many thanks for your great work.
-
And you're linking dynamically, right? Because there is a small issue with using your own STB image and image write along statically linked SFML: https://en.sfml-dev.org/forums/index.php?topic=25011.msg166598
-
And you're linking dynamically, right? Because there is a small issue with using your own STB image and image write along statically linked SFML: https://en.sfml-dev.org/forums/index.php?topic=25011.msg166598
Thank you for the hint. Yes, currently I am linking SFML dynamically and will probably stick to it.
-
Before anything, sorry for reviving an old thread like this.
I've implemented a saveToMemory function and it has worked quite well for me.
I've been thinking of making a PR regarding this change, but first I need to know two things:
- Would this be a good idea?
- What syntax should be used for this?
In regards to the syntax, I've written something like:
bool sf::Image::saveToMemory(const std::string& format, std::vector<sf::Uint8>& buffer);
To be used as:
sf::Image image;
//fill the image with data
...
std::vector<sf::Uint8> buffer;
image.saveToMemory("png", buffer);
While this is perfectly functional, I feel like this API is very inconsistent with the rest of SFML. What is the general opinion about passing the buffer as a parameter? Also, should the format be determined using an enum? I'd usually go for that, but I wanted to be at least a bit consistent with the saveToFile API.
-
Of your idea of the method proposal, I would say the buffer would go first as the format could have a default.
-
Of your idea of the method proposal, I would say the buffer would go first as the format could have a default.
So for now, this is the current syntax:bool sf::Image::saveToMemory(std::vector<sf::Uint8>& buffer, const std::string& format = "png");
-
One of the issues with any design is, that it would need to be adapted for audio and font as well.
With the current design, one needs to be aware that the buffer can be resized by the save function. It should also be defined what happens with an already allocated buffer and with an over-sized buffer.
Also one thing to consider whether sf::Unit8 is the best binary data representation.
Otherwise, I think it's the most straight forward approach.
-
Audios yes but why fonts? There's a ton of ways to make an image and a few ways (stitch or mix two buffers, record, create samples programmatically) to make audio but there isn't any font editing/mixing/making in SFML. If you want to 'save' a font to a file in memory just take the file you're using and load it all into memory somehow and that's it there.
-
I thought we had a saveToFile on the font, but we don't, so it's okay to also not have a saveToMemory. :)
-
One of the issues with any design is, that it would need to be adapted for audio and font as well.
While I'm not particularly familiar with the audio internals, I'll definitely take a look.
With the current design, one needs to be aware that the buffer can be resized by the save function. It should also be defined what happens with an already allocated buffer and with an over-sized buffer.
I can think of two approaches: clear the buffer before inserting data, or append the data to the end. Since it's a vector, it can be easily resized to either data.size() or buffer.size() + data.size().
Also one thing to consider whether sf::Unit8 is the best binary data representation.
If SFML's spec was set to C++11, then using signed or unsigned wouldn't make a lot of difference, execution-wise. However, since the idea is to target C++03 compilers as well, we need to remember that char can have some padding added to it by the compiler. While unsigned char has always been guaranteed by the standard to have no padding added to it iirc.
There's also the convention that char represents text while unsigned char represents binary data, although signed char could be used instead.
These are the points I can think of for unsigned, but I'd love to hear what everyone else thinks about it.
-
What is that padding in char that you speak of? Char is often signed and that's the 'problem' but char, signed char and unsigned char are three same sized and distinct types (even though char is 100% like one of the other two).
-
[C++11: 3.9.1/1]: [..] A char, a signed char, and an unsigned char occupy the same amount of storage and have the same alignment requirements (3.11); that is, they have the same object representation. For character types, all bits of the object representation participate in the value representation. [..]
From my understanding, this only became part of the standard in C++11, but I could be wrong.
-
That part is there in C++98 and I've never heard of any 'padding' in chars (that'd then not be there for signed/unsigned chars). char, unsigned char and void ptrs get used to mean 'binary data' all the time in APIs.
-
That part is there in C++98 and I've never heard of any 'padding' in chars (that'd then not be there for signed/unsigned chars). char, unsigned char and void ptrs get used to mean 'binary data' all the time in APIs.
I see, good point. Well, then I personally have no preference for one or the other. What are your thoughts?
-
Well, SFML uses void now because everywhere the buffer size is known and 'big enough'.
This can't happen with std::vector so a different API would have to be made to allow user to alloc a buffer and handle failure nicely, something like:
bool sf::Image::saveToMemory(void * buff, std::size_t size, std::size_t& outsize);
Where return bool would say if there was enough space to write it all out or not and outsize would always contain how much bytes would be written if there was enough space. Some C, POSIX and WinAPI functions do it that or similar way (like some signed size type as return and -1 there as error), even allowing calling with NULL ptr and 0 size to get just the output size but that's starting to get too un-SFML-like (and kind of cumbersome to use, I always wrap these myself to just do right thing with dynamic allocation in C and C++ when I need them or make sure the buffer is way bigger than required so they can't fail).
With std::vector (which I don't mind since it solves the size problem and SFML already uses STL a lot and isn't some tight no-lib-side-mem-alloc-ever low-level hardcore library) either char or uchar are fine. Either choice will have someone preferring the other one, since both have decent reasons for being used (char is 'default' byte type to use, and uchar has the nice property of being unsigned so no sign and sign extensions shenanigans take place).
Alternatively a callback function or class could be used and use a const void ptr there but that still leaves a sort of gap in API where reading can be done from 3 source (stream, file, memory) and writing to 2 (file, stream) with memory missing/part of stream (while for reading there is memory input stream but also outright loadFromMemory call that takes just const void ptr and size).
-
While I don't think it's a particularly good idea, nothing really stops us from having two overloads, one with char and one with unsigned char.
Particularly, I think using std::vector fits SFML a lot more, especially since getting an array from it is trivial. I suppose it just makes things a lot more simple.
-
While I don't think it's a particularly good idea, nothing really stops us from having two overloads, one with char and one with unsigned char.
And one with signed char? Those are 3 distinct types, even if char is treated the same as one of the other two.
Alternatively a callback function or class could be used and use a const void ptr there but that still leaves a sort of gap in API where reading can be done from 3 source (stream, file, memory) and writing to 2 (file, stream) with memory missing/part of stream (while for reading there is memory input stream but also outright loadFromMemory call that takes just const void ptr and size).
True, but you need to choose one of the following:
- saveToMemory(void* buff, std::size_t size, std::size_t& outSize);
This would be the direct counterpart of readFromMemory(), and as such the only way to have no "gap".
However, this is very low-level, and user needs to take care of reallocations/retries. Quite user-unfriendly, even the C API of STB is nicer with the callback.
- saveToMemory(std::vector<sf::Uint8>& outMemory);
Easy to use, quite limited though. Expects that the user needs exactly a std::vector, and that the elements are exactly sf::Uint8 (aka unsigned char). As soon as a different format is needed, the user needs to copy or reinterpret pointers.
- saveToStream(sf::OutputStream& stream);
This is effectively a C++ translation of the C callback API (which is great, by the way).
Since saving to memory is not the typical game use case, it is probably occurring mostly when interfacing with a different rendering engine or image processing systems. Those usually have a rather low-level API based on char*/size_t pairs. Even if std::vector supports this format, for any scenario where ownership is transferred to a third-party system, the std::vector leads to a useless temporary copy.
In my opinion, sf::OutputStream is the cleanest and most flexible solution.
std::vector is convenient, but I would add it only once we gather some knowledge about common use cases (e.g. which element type).
-
Since saving to memory is not the typical game use case, it is probably occurring mostly when interfacing with a different rendering engine or image processing systems. Those usually have a rather low-level API based on char*/size_t pairs. Even if std::vector supports this format, for any scenario where ownership is transferred to a third-party system, the std::vector leads to a useless temporary copy.
Usually, these other systems have methods to load RAW RGBA data, which makes getPixelsPtr and width * height * 4 perfect for this use case. The whole idea of saveToMemory is to encode this data in a specified format. Otherwise we could just copy the internal pixel data.
std::vector is convenient, but I would add it only once we gather some knowledge about common use cases (e.g. which element type).
Two particular use cases I've ran into were the following:
1-) Saving a project file in json. One of the data types stored were base64 encoded images. Encoding directly from getPixelsPtr was particularly easy, however the final size transformed a 78KB JPEG into 5,6MB RAW RGBA. That's where saveToMemory comes in, encoding the image without accessing the filesystem (yet). By using std::vector, there is a minimal code difference here.
2-) A server application which modifies files and returns them through the network. Without adding additional libraries, saveToMemory could remove the need to write to the filesystem as a middleman, reducing SSD wear and API response times.
That said, a saveToStream really does sound like the better option, so we could propose something like:bool sf::Image::saveToStream(sf::OutputStream& stream, const std::string& format = "png");
-
Thanks for listing the use cases, they sound reasonable indeed. The web didn't come to my mind, as other languages are often more prominent than C++.
In JSON, the binary representation of an image (whether you represent it as number array or Base-64 string) could also be constructed in a stream-based manner, not requiring to copy the whole image data to an intermediate buffer. This would be a perfect use case for sf::OutputStream.
bool sf::Image::saveToStream(sf::OutputStream& stream, const std::string& format = "png");
looks good, except for the default parameter. I think the user should explicitly specify the format.
Whether enum or string -- I'm inclined to enum for type safety and to immediately see the available options. However, in C++ it's quite a pain to convert it to/from strings (such as in file extensions), and as such, strings might prove easier to use due to fewer conversions.
-
Unfortunately, this ultimately points us to asking even bigger questions, such as "what would sf::OutputStream look like?". And with the talk of switching to standard streams further on, it looks out of reach for the moment. Unless we could define an sf::OutputStream now.
For now, without implementing an output stream, we're looking at something like this:bool sf::Image::saveToMemory(const std::string& format, std::vector<sf::Uint8>& buffer);
Orbool sf::Image::saveToMemory(const sf::Image::Format& format, std::vector<sf::Uint8>& buffer);
With sf::Image::Format being an enum of png, jpg, bmp, etc. Although I personally feel that adding an enum is unecessary.
Discussion in the SFML Discord server pointed to a large preference towards sf::Uint8, so for the moment I plan on going with that.
-
sf::InputStream looks like this:
class InputStream
{
public:
virtual ~InputStream() {}
virtual Int64 read(void* data, Int64 size) = 0;
virtual Int64 seek(Int64 position) = 0;
virtual Int64 tell() = 0;
virtual Int64 getSize() = 0;
};
So the 1:1 counterpart would be:
class OutputStream
{
public:
virtual ~OutputStream() {}
virtual Int64 write(const void* data, Int64 size) = 0;
virtual Int64 seek(Int64 position) = 0;
virtual Int64 tell() = 0; // put pointer (bytes written to here)
virtual Int64 getSize() = 0; // total bytes written (or available?)
};
I'm not sure if this is the best design however -- random access seems like a big constraint. This is already the case for input streams; requiring random access makes it impossible to continuously and incrementally process incoming data (i.e. streaming). For output streams, often you just want to have a sink where you put data in a fire&forget manner.
As such, there could be a more minimal version, even if this creates a slight asymmetry:
class OutputStream
{
public:
virtual ~InputStream() {}
virtual Int64 write(const void* data, Int64 size) = 0;
};
In Java for example, OutputStream (https://docs.oracle.com/javase/8/docs/api/java/io/OutputStream.html) is very minimal too.
We need to check why sf::InputStream provides seek/tell random access. I guess it's to allow partial loading of resources (e.g. audio).
-
If you want discuss OutputStream, I suggest to create a new topic on this subject, as I'm sure it will be more than just a simple API design that needs discussion for it. ;)
Personally, I think both saveToMemory and saveToStream are things that could be added, but as it stands saveToMemory doesn't really require yet another feature to be written first and it's something Ventus seems to be willing to implement, so maybe lets first focus on that here, while in parallel we can discuss implementations for sf::OutputStream in another thread.
-
The Pull Request is now open: https://github.com/SFML/SFML/pull/1597
Thank you for contributing in the discussion!