Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: ResourceStream class for custom resource loading  (Read 15292 times)

0 Members and 1 Guest are viewing this topic.

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« on: June 08, 2011, 05:29:22 am »
For previous discussion on this topic, see this thread:

http://www.sfml-dev.org/forum/viewtopic.php?p=32631#32631

Specifically, I'm interested in custom file/stream I/O support for resource loading in SFML.  Mainly because I have interest in using zip file "packages" for game modules in my projects, and I want to be able to load resources directly from those packages without having to dump the file into memory first.

The current LoadFromFile and LoadFromMemory options available in SFML are unsuitable for this goal, so I suggest the below abstract class as well as "LoadFromStream" functions added to any and all resource loading classes.

As mentioned in the above thread, I am entirely willing to work towards wrapping existing libs (like libsndfile, etc) to use the below class.

Code: [Select]

namespace sf
{

class ResourceStream : public NonCopyable
{
public:
    //=============================================
    // enums and typedefs
    typedef Int64  Offset;

    enum Mode
    {
        SeekOrigin,
        SeekOffset,
        SeekEnd
    };

    //=============================================
    // construction / destruction
    virtual ~ResourceStream() { }

    //=============================================
    // stream I/O
    virtual Offset      Read(void* data, Offset size) = 0;          // returns bytes read
    virtual Offset      Write(const void* data, Offset size) = 0;   // returns bytes written
    virtual Offset      Seek(Offset pos,Mode = SeekOrigin) = 0;     // returns position seeked to (-1 on error)
    virtual Offset      Tell() = 0;                                 // returns -1 on error
    virtual Offset      GetSize() = 0;                              // returns total file size (-1 on error)
};

} // namespace sf


Not very complicated.  KISS.

Design decisions:

1) no const functions.  (precedent:  STL).  You typically don't have const streams anyway.  Additionally, even tasks that seem const may not be. (GetSize for example, may involve seeking, violating const rules)

2) no error reporting.  (precedent:  SFML).  Getting bogged down with error reporting would make the class complicated and the minimal requirements for deriving your own class more difficult.  As it stands now, implementing the above is very straightforward and simple.

And considering this class is designed to be used in SFML resource loading classes, and those classes don't have detailed error reporting anyway, the information would likely be lost.

3) I did my best to mimic SFML naming conventions.  I'm not sure about the enum, though.  Maybe that could be better.

4) typedeffing Offset seemed to add clarity.  It also allows for the possibility of support for future platforms which may not have 64-bit capabilities (maybe some hand-held devices?)

5)  I didn't make Doxygen style comments because I'm lazy.  That wasn't really a design decision

6)  I was torn as to whether or not GetSize should be included.  I also considered giving it a body that did the Seek/Tell/Seek approach instead of making it pure virtual.  However Seek/Tell/Seek leaves room for problems (what if the 2nd seek fails?).

Plus at least one lib needs such a function.  And it shouldn't be hard to implement in derived classes anyway.




Overall I think the simplicity says it all.

Laurent:  I hope you give this your consideration, and I am eager to hear your thoughts / feedback.


EDIT:

so I took a look at stb_image.c to see how much work will need to be put into it.  Looks like it is going to take a pretty decent chunk of effort in some areas, but in other areas, maybe not so much.  He seems to have compartmentalized file I/O (which is good), but the thing that's tripping me up is that there's #ifdef blocks everywhere and seemingly no indication of exactly what the defines are supposed to do.

It's definitely doable, though.

If he had used a callback mechanism from day 1 of this, he would have saved himself lots of duplicate code.  I see 2 sets of functions for pretty much everything -- one for memory loads and one for stdio loads.  He could have just implemented one callback set, and then employed it to implement the memory/stdio sets.

Though I suppose that would have an ever-so-slight performance hit.

But whatever.  I won't get started on this until I hear back from Laurent anyway.  No sense in doing all that work if it will be for naught.

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
ResourceStream class for custom resource loading
« Reply #1 on: June 08, 2011, 08:37:30 am »
That looks good. Like you said, simplicity says it all.

There are a few things that I would change:
- I don't think we need to support Write; and if we do one day, a separate class would be better (so we don't have to mix incompatible functions in a single class)
- Seek can probably be simplified, I don't think that the "mode" argument is really necessary
- I'm not sure if we should enforce non-copyability in base class. I don't see anything wrong if a derived class is copyable
- Int64 must be supported by the target compiler anyway, this is not optional, so we don't need a typedef
- char* may be better than void* -- it doesn't realy make a difference, but it says "this is a sequence of bytes", while void* only means "this could be anything".

So it would be even simpler:
Code: [Select]
class InputStream
{
public:

    virtual ~InputStream() {}

    virtual Int64 Read(char* data, Int64 size) = 0;

    virtual Int64 Seek(Int64 position) = 0;

    virtual Int64 GetPosition() = 0;

    virtual Int64 GetSize() = 0;
};


I think I should first contact the author of stb_image (he's fast to reply, and always accepts my comments on his library).
Laurent Gomila - SFML developer

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
ResourceStream class for custom resource loading
« Reply #2 on: June 08, 2011, 10:47:13 am »
Here is the answer from Sean, the author of stb_image:

Quote
It's certainly something I've considered doing--it wasn't
worth it originally before stb_image was well-known, since
it was a more narrowly-focused library, but at this point it's
reasonable to want to extend it to be useful in more scenarios.

But the reality is there's almost no chance I'll have time to do it,
since I have too many libraries on my hands now.

I'd accept an external patch if it did it cleanly (by mostly
only modifying the existing routines that fetch from memory
or disk), if the code felt reasonably small, and it compiled in C.
(You could also support C++ streams as long as it was all
#ifdef __cpp__'d, although that might bloat a little.)

To make it small in code size, it would probably be a good time
to strip out the deprecated interface where every image format
has its own public entry points for decoding just that image from
various possible sources (each one independently opens the file
and calls "start_file"). If you stripped that out, then you could just
have a single entry point for each decoder that just takes the "stbi"
I/O handle, instead of one for memory, one for file, one for filename.
(Actually you'd still three entry points: test, info, and load. But the
current ones have eight entry points, AND they're public, and the
#ifdef for whether C stdio should be linked in is all through the
code, whereas with them gone the entry points would be private and
the stdio code isolated to the shared open/read functions.) I
deprecated those interfaces in July 2010 explicitly so the codebase
could be simplified, possibly with an eye towards I/O abstraction
(I don't remember), and nobody's complained, so I think it's reasonable
to kill it.


Don't hesitate to contact him directly if you start working on stb_image and have questions about his code.
Laurent Gomila - SFML developer

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
ResourceStream class for custom resource loading
« Reply #3 on: June 08, 2011, 11:01:20 am »
One additional comment: this was initially not planned for SFML 2, and as it doesn't break the existing API it definitely doesn't need to be done for 2.0.

So it would probably be part of a future release, most likely SFML 2.1.
Laurent Gomila - SFML developer

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #4 on: June 08, 2011, 12:26:05 pm »
Quote
- I don't think we need to support Write


I was thinking it would be useful for sf::Image::SaveToStream, but you're probably right.  I personally don't have any use for a SaveToStream, mostly I put it there for the sake of completeness.

OK

Quote
Seek can probably be simplified, I don't think that the "mode" argument is really necessary


Are you sure?  I'm all for simplifying it if it really isn't needed.  I'm just a little paranoid that it will be necessary for one of the libs.

iirc, when I did something like this with libvorbisfile way back when, it used SEEK_CUR for when you seeked around in the audio file.

If it turns out we need the functionality later, adding it might be an API breaker, as all derived classes would need to be updated.

Mainly I'm worried about libsndfile and FreeType here, as I'm not sure what they require.  Perhaps I'll give them a closer look later and report back.

An alternative option might be to do something like this:

Code: [Select]

    virtual Int64 Seek(Int64 position) = 0;

    virtual Int64 SeekFromEnd(Int64 position)
    {
        Int64 end = GetSize();
        if(end < 0)   return -1;
        return Seek(position + end);
    }

    virtual Int64 SeekFromCur(Int64 position)
    {
        Int64 cur = Tell();
        if(cur < 0)   return -1;
        return Seek(position + cur);
    }


Pros:
- deriving classes is simpler
- SeekFromEnd/SeekFromCur are virtual / optionally overridable so derived classes can re-implement them if suitable

Cons:
- possibly a bit more awkward to use
- makes InputStream more complicated


Quote

- I'm not sure if we should enforce non-copyability in base class. I don't see anything wrong if a derived class is copyable
- Int64 must be supported by the target compiler anyway, this is not optional, so we don't need a typedef


Sounds good to me.

Quote
- char* may be better than void* -- it doesn't realy make a difference, but it says "this is a sequence of bytes", while void* only means "this could be anything".


I have mixed feelings about this, myself.  iostream uses char*, but personally I always found that to be annoying as it requires an explicit cast.  On the other hand I suppose it is more typesafe, and maybe an explicit cast is a good thing.

If you like char* more, let's go with it.  I'm fine either way.

OK

Quote
Here is the answer from Sean, the author of stb_image: ...


Thanks for that.  I actually do want to speak with him directly as I do have a few questions about it.  I'll probably shoot him a message this afternoon (my time)

Quote
So it would probably be part of a future release, most likely SFML 2.1.


Works for me.

I'll get started on transitioning stb_image.  If all goes well I might have it done by this weekend, but that's kind of optimistic.  We'll see.


Thanks again Laurent!  It's appreciated.

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
ResourceStream class for custom resource loading
« Reply #5 on: June 08, 2011, 01:09:21 pm »
Quote
Are you sure? I'm all for simplifying it if it really isn't needed. I'm just a little paranoid that it will be necessary for one of the libs.

iirc, when I did something like this with libvorbisfile way back when, it used SEEK_CUR for when you seeked around in the audio file.

If it turns out we need the functionality later, adding it might be an API breaker, as all derived classes would need to be updated.

Seek(current) and Seek(end) can always be implemented in terms of Seek(begin) + GetSize(). So the only possible drawback is that an implementation might be really slow in one of these two functions, whereas it could potentially be much faster in a direct Seek(current). Something with only sequential access, like a linked list.
But I don't really see what kind of stream implementation would behave this way...

Quote
Mainly I'm worried about libsndfile and FreeType here, as I'm not sure what they require. Perhaps I'll give them a closer look later and report back.

Since the two other can be implemented with the first one, we don't have to worry about what libraries require. We'll still be able to give them what they want.

Quote
I have mixed feelings about this, myself. iostream uses char*, but personally I always found that to be annoying as it requires an explicit cast. On the other hand I suppose it is more typesafe, and maybe an explicit cast is a good thing.

Indeed.

Quote
I'll get started on transitioning stb_image. If all goes well I might have it done by this weekend, but that's kind of optimistic. We'll see.

I think it's very optimistic, given that you must run a lot of tests to make sure that it still compiles on all compilers, and runs on all platforms.
Laurent Gomila - SFML developer

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #6 on: June 08, 2011, 04:07:14 pm »
Quote
I think it's very optimistic, given that you must run a lot of tests to make sure that it still compiles on all compilers, and runs on all platforms.


To be honest I hadn't considered all that.  I just meant I would have it working on my machine by then.  XD

That's what I get for posting at 3 AM

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #7 on: June 11, 2011, 09:02:54 am »
I just finished up the changes to stb_image.

Well not finished.. but I think I've changed everything that needs to be changed, and it's compiling OK.  Now I have to actually test it.

but it's already passed midnight, so it'll have to wait until tomorrow.

zzzzzz

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
ResourceStream class for custom resource loading
« Reply #8 on: June 11, 2011, 09:58:14 am »
Very good :)
Laurent Gomila - SFML developer

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #9 on: June 11, 2011, 06:28:45 pm »
OK!

bmp, gif, png, tga, jpg, psd loaders all check out.  At least when you load from file.  Didn't test loading from memory yet but I don't anticipate a problem, as I hardly touched that code.

File loading now uses the callback system, so if loading from file works, then loading from callbacks will also work.  But I'll test that too for due dilligence.

The biggest problem now is I cannot find any .pic or .hdr files to test those loaders.


Does anyone have any example .pic or .hdr files that I can get a copy of?

Once I finish up the testing, writing sf::Image::LoadFromStream should take me all of 5 minutes.

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #10 on: June 12, 2011, 03:43:00 am »
All done with stb_image and the SFML wrapper for it.

Complete with Doxygen comments.

stb_image itself was tested on:
- VC++ 2010 Express on Win7
- Fedora 11 i386 (Linux) g++ (GCC) 4.4.1 20090725 (Red Hat 4.4.1-2)

Given that there's nothing platform specific or unusual about the code, I don't expect problems on other platforms/compilers.

The SFML stuff was only tested on Win7 with VC++ 2010 Express, but the code is especially trivial so I'm confident there won't be any portability issues.


Files:

http://www.filedropper.com/customstream


All modified files included in the archive in the original directory structure.  Unchanged files are not included.


Here's to hoping it gets added to the official release!

As always, I'm looking forward to hearing your comments and feedback, Laurent.


EDIT:  actually I didn't get to test the .pic file loading because I couldn't get a .pic file.  But I doubt it got broken in the update.  Who cares about that format anyway.

EDIT 2:  I submitted the stb_image update to Sean as well via email.  I didn't hear back yet but once I do I'll report back.  Especially if anything needs tweaking.

EDIT 3:  On to music!

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #11 on: June 12, 2011, 07:25:18 am »
Finished up the stream stuff for Music, SoundBuffer, and Font as well.

Updated files (including the image stuff, so you can ignore that previous link) :

http://www.filedropper.com/customstream_1


Also, I spotted a potential memory leak in the Font loaders.  If FT_Select_Charmap fails, then 'face' leaks because the function returns before freeing it.

I fixed that in the files I uploaded.


* starts another project while he awaits judgement *

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
ResourceStream class for custom resource loading
« Reply #12 on: June 12, 2011, 09:42:48 am »
Awesome, thanks :)

Don't forget to test the code in pure standard C; something that compiles/works in C++ may be completely wrong or broken in C.

I don't know when I'll apply your new code, I have a few things to finish before starting on this one. But maybe it will be part of SFML 2.0 finally, because basically everything's already done :)
Laurent Gomila - SFML developer

Nexus

  • SFML Team
  • Hero Member
  • *****
  • Posts: 6287
  • Thor Developer
    • View Profile
    • Bromeon
ResourceStream class for custom resource loading
« Reply #13 on: June 12, 2011, 02:59:29 pm »
Looks great, I like the idea of streams for custom resource loading. :)

Do you plan to write an example implementation of a derived class?
Zloxx II: action platformer
Thor Library: particle systems, animations, dot products, ...
SFML Game Development:

Disch

  • Full Member
  • ***
  • Posts: 220
    • View Profile
ResourceStream class for custom resource loading
« Reply #14 on: June 12, 2011, 04:00:58 pm »
Quote
Don't forget to test the code in pure standard C; something that compiles/works in C++ may be completely wrong or broken in C.


The stb_image stuff all works in C and C++.

Obviously I didn't try compiling the SFML stuff in C.

Quote
Do you plan to write an example implementation of a derived class?


I suppose I could, but the thing is it's more of a feature that would be used in non-trivial cases, so an example class would either be too complicated or kind of useless.

I plan on writing an SFML extension that works with .zip files and the like, which is the main reason I wanted it.

 

anything