SDL Stream for C++

Question

Looking at the SDL library.

Looking to load images from file into an SDL_Surface using the SDL2 Image API to do this I need to implement the SDL_RWops object class in terms of a C++ stream.

The reason I want to use SDL_RWops and use a C++ stream is so that I can do a tiny amount of wrapping and use the standard C++ operator<< to load a file.

ThorSDLStream.h

#ifndef THORSANVIL_UI_THOR_SDL_STREAM_H
#define THORSANVIL_UI_THOR_SDL_STREAM_H
#include <SDL.h>
#include <SDL_image.h>
struct ThorSDLStreamRead: public SDL_RWops
{
 std::istream& stream;
 ThorSDLStreamRead(std::istream& stream);
};
struct ThorSDLStreamWrite: public SDL_RWops
{
 std::ostream& stream;
 ThorSDLStreamWrite(std::ostream& stream);
};
}
#endif

ThorSDLStream.cpp

#include "Surface.h"
#include <SDL_image.h>
extern "C"
{
 Sint64 streamSizeThors(SDL_RWops*);
 Sint64 streamSeekThorRead(SDL_RWops*, Sint64, int);
 Sint64 streamSeekThorWrite(SDL_RWops*, Sint64, int);
 size_t streamReadThor(SDL_RWops*, void*, size_t, size_t);
 size_t streamReadThorBad(SDL_RWops*, void*, size_t, size_t);
 size_t streamWriteThor(SDL_RWops*, const void*, size_t, size_t);
 size_t streamWriteThorBad(SDL_RWops*, const void*, size_t, size_t);
 int streamCloseThor(SDL_RWops*);
}
std::ios_base::seekdir convertSDLDirectionThor(int dir)
{
 switch (dir)
 {
 case RW_SEEK_SET: return std::ios_base::beg;
 case RW_SEEK_CUR: return std::ios_base::cur;
 case RW_SEEK_END: return std::ios_base::end;
 }
 return std::ios_base::beg;
}
/*
 * The SDL_RWops Seek calls this function.
 * This is used to reset the stream to the clean state when things
 * go wrong (such as when trying to discover file format).
 *
 * The result is the offset into the file after the seek is done.
 *
 * The code use TELL => SEEK(CUR, 0)
 */
Sint64 streamSeekThorRead(SDL_RWops* input, Sint64 dist, int dir)
{
 ThorSDLStreamRead* data = reinterpret_cast<ThorSDLStreamRead*>(input);
 data->stream.clear();
 std::ios_base::seekdir direction = convertSDLDirectionThor(dir);
 data->stream.seekg(dist, direction);
 return data->stream ? static_cast<Sint64>(data->stream.tellg()) : -1;
}
/*
 * Like streamSeekThorRead but uses write stream.
 */
Sint64 streamSeekThorWrite(SDL_RWops* input, Sint64 dist, int dir)
{
 ThorSDLStreamWrite* data = reinterpret_cast<ThorSDLStreamWrite*>(input);
 data->stream.clear();
 std::ios_base::seekdir direction = convertSDLDirectionThor(dir);
 data->stream.seekp(dist, direction);
 return data->stream ? static_cast<Sint64>(data->stream.tellp()) : -1;
}
/*
 * Reads `num` objects of size `size` from the stream.
 * Return -1 on failure or the number of objects read.
 */
size_t streamReadThor(SDL_RWops* input, void* dst, size_t size, size_t num)
{
 ThorSDLStreamRead* data = reinterpret_cast<ThorSDLStreamRead*>(input);
 data->stream.read(reinterpret_cast<char*>(dst), num * size);
 return data->stream ? data->stream.gcount() / size : -1;
}
/*
 * Write `num` objects of size `size` to the stream.
 * Return -1 on failure or the number of objects written.
 */
size_t streamWriteThor(SDL_RWops* input, const void* src, size_t size, size_t num)
{
 ThorSDLStreamWrite* data = reinterpret_cast<ThorSDLStreamWrite*>(input);
 auto before = data->stream.tellp();
 data->stream.write(reinterpret_cast<char const*>(src), num * size);
 return data->stream ? (data->stream.tellp() - before) / size : -1;
}
/* Functionality that does not make sense for this implementation */
int streamCloseThor(SDL_RWops*) {return 0;}
Sint64 streamSizeThor(SDL_RWops*) {return -1;}
size_t streamReadThorBad(SDL_RWops*, void*, size_t, size_t) {return -1;}
size_t streamWriteThorBad(SDL_RWops*, const void*, size_t, size_t) {return -1;}
// Set up a read object.
ThorSDLStreamRead::ThorSDLStreamRead(std::istream& stream)
 : stream(stream)
{
 seek = streamSeekThorRead;
 read = streamReadThor;
 type = SDL_RWOPS_UNKNOWN;
 close = streamCloseThor;
 write = streamWriteThorBad;
 size = streamSizeThor;
}
// Set up a write object.
ThorSDLStreamWrite::ThorSDLStreamWrite(std::ostream& stream)
 : stream(stream)
{
 seek = streamSeekThorWrite;
 write = streamWriteThor;
 type = SDL_RWOPS_UNKNOWN;
 close = streamCloseThor;
 read = streamReadThorBad;
 size = streamSizeThor;
}

I may do a code review of the Surface object later but basically its a wrapper around SDL_Surface. But to give context to the above I will show the usage in the Surface class.

// will be called from `operator>>`
std::istream& Surface::loadFromStream(std::istream& stream)
{
 if (stream)
 {
 try
 {
 ThorSDLStreamRead streamWrapper(stream);
 SDL_Surface* newSurface = IMG_Load_RW(&streamWrapper, 0);
 SDL::Surface tmp(newSurface, "Failed to load image from stream");
 std::swap(surface, tmp);
 }
 catch (std::exception const& e)
 {
 stream.setstate(std::ios::failbit);
 }
 }
 return stream;
}
// Will be called from `operator<<`
std::ostream& SurfaceToPNG::saveToStream(std::ostream& stream) const
{
 if (stream)
 {
 ThorSDLStreamWrite streamWrapper(stream);
 int status = IMG_SavePNG_RW(surface.surface, &streamWrapper, 0);
 if (status != 0) {
 stream.setstate(std::ios::failbit);
 }
 }
 return stream;
};
// Will be called from `operator<<`
std::ostream& SurfaceToJPG::saveToStream(std::ostream& stream) const
{
 if (stream)
 {
 ThorSDLStreamWrite streamWrapper(stream);
 int status = IMG_SaveJPG_RW(surface.surface, &streamWrapper, 0, quality);
 if (status != 0) {
 stream.setstate(std::ios::failbit);
 }
 }
 return stream;
}

• rioki: I was about to write about ThorSDLStreamXYZ should not inherit from SDL_RWops; but after looking further into the problem; there are not many alternate options. So this appears to be the least evil solution.

Answer 1

Use static_cast<>() to cast from base to derived

You can use static_cast<>() to cast from a pointer to base class to a pointer to derived class. This is much safer, because it will give an error if you try to cast to an unrelated class, whereas reinterpret_cast<>() will just let that happen.

Make the C functions static

All the functions marked extern "C" only need to be visible inside ThorSDLStream.cpp, so make them static, or put them inside an anonymous namespace. This avoids conflicts in the global namespace. In fact, you don't have to put Thor in their names if you do this.

Naming

It's confusing to see streamWriteThor(SDL_RWops* input, ...). Are you writing to an input? Just name the first parameter output.

No need to query for the number of bytes read/written

If read() and write() on a stream succeed, they will have read or written exactly the number of bytes that you requested, otherwise the conversion of data->stream to bool will result in false.

• Loki Astari: Thinking about the number of bytes we return. It seems like that on the eof we still need to return the number of bytes read (unless eof is already set before the read).
• Loki Astari: Follow up: codereview.stackexchange.com/q/282531/507