FreeALUT stream proof-of-concept

[Chris Robinson-5]

Here's a patch that can/should be able to be applied to alut in svn to show
off the combined C/C++ proof-of-concept idea, as well as a general API I'd go
for. You can find a sample C++ app using it here:
http://kcat.strangesoft.net/alutstream.cpp

Currently it relies on libvorbisfile, but obviously a proper implementation
would not. If something like this can go forward, though, I have ideas for
implementing an run-time detected Ogg Vorbis loader for the buffer loading
API, and using the internal loading APIs to do the actual reading for the
data stream.

Ideas, comments, and criticism welcome.

[alut-stream.diff]

---

Index: alut/include/AL/alut.h
===================================================================
--- alut/include/AL/alut.h (revision 1484)
+++ alut/include/AL/alut.h (working copy)
@@ -81,6 +81,8 @@
 #define ALUT_LOADER_BUFFER                     0x300
 #define ALUT_LOADER_MEMORY                     0x301
 
+typedef struct ALUTStream ALUTStream;
+
 ALUT_API ALboolean ALUT_APIENTRY alutInit (int *argcp, char **argv);
 ALUT_API ALboolean ALUT_APIENTRY alutInitWithoutContext (int *argcp, char **argv);
 ALUT_API ALboolean ALUT_APIENTRY alutExit (void);
@@ -98,6 +100,13 @@
 ALUT_API ALvoid *ALUT_APIENTRY alutLoadMemoryHelloWorld (ALenum *format, ALsizei *size, ALfloat *frequency);
 ALUT_API ALvoid *ALUT_APIENTRY alutLoadMemoryWaveform (ALenum waveshape, ALfloat frequency, ALfloat phase, ALfloat duration, ALenum *format, ALsizei *size, ALfloat *freq);
 
+ALUT_API ALUTStream* ALUT_APIENTRY alutCreateStreamFromFile (const char *fileName, ALuint length, ALuint bufferCount);
+ALUT_API ALboolean   ALUT_APIENTRY alutStreamIsPlaying (ALUTStream *stream);
+ALUT_API ALboolean   ALUT_APIENTRY alutStreamPoll (ALUTStream *stream);
+ALUT_API ALboolean   ALUT_APIENTRY alutStreamPlayOnSource (ALUTStream *stream, ALuint sourceID);
+ALUT_API ALboolean   ALUT_APIENTRY alutStreamStop (ALUTStream *stream);
+ALUT_API ALboolean   ALUT_APIENTRY alutDestroyStream (ALUTStream *stream);
+
 ALUT_API const char *ALUT_APIENTRY alutGetMIMETypes (ALenum loader);
 
 ALUT_API ALint ALUT_APIENTRY alutGetMajorVersion (void);
@@ -121,6 +130,15 @@
 
 #if defined(__cplusplus)
 }
+
+struct ALUTStream {
+    virtual ALboolean ALUT_APIENTRY IsPlaying () = 0;
+    virtual ALboolean ALUT_APIENTRY Poll () = 0;
+    virtual ALboolean ALUT_APIENTRY PlayOnSource (ALuint sourceID) = 0;
+    virtual ALboolean ALUT_APIENTRY Stop () = 0;
+    virtual ~ALUTStream(){};
+};
+
 #endif
 
 #endif
Index: alut/src/alutInternal.h
===================================================================
--- alut/src/alutInternal.h (revision 1484)
+++ alut/src/alutInternal.h (working copy)
@@ -35,10 +35,14 @@
 #define UNUSED(x) x
 #endif
 
-#include <AL/alut.h>
+#include "AL/alut.h"
 
 #define AU_HEADER_SIZE 24
 
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
 /* see: http://en.wikipedia.org/wiki/Au_file_format, G.72x are missing */
 enum AUEncoding
 {
@@ -128,4 +132,9 @@
                                  ALenum *format);
 extern ALuint _alutPassBufferData (BufferData *bufferData);
 
+#if defined(__cplusplus)
+} // extern "C"
+#endif
+
+
 #endif /* not ALUT_INTERNAL_H */
Index: alut/CMakeLists.txt
===================================================================
--- alut/CMakeLists.txt (revision 1484)
+++ alut/CMakeLists.txt (working copy)
@@ -2,13 +2,13 @@
 CMAKE_MINIMUM_REQUIRED(VERSION 2.0)
 
 SET(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/admin/CMakeModules")
-PROJECT(Alut C)
+PROJECT(Alut)
 SET(PACKAGE "freealut")
 SET(PACKAGE_TARNAME "freealut")
 SET(PACKAGE_NAME "freealut library")
 SET(PACKAGE_MAJOR_VERSION "1")
-SET(PACKAGE_MINOR_VERSION "0")
-SET(PACKAGE_BUILD_VERSION "1")
+SET(PACKAGE_MINOR_VERSION "1")
+SET(PACKAGE_BUILD_VERSION "0")
 SET(PACKAGE_VERSION "${PACKAGE_MAJOR_VERSION}.${PACKAGE_MINOR_VERSION}.${PACKAGE_BUILD_VERSION}")
 SET(MAJOR_VERSION "0")
 SET(MINOR_VERSION "0")
@@ -32,6 +32,7 @@
                  src/alutInternal.h
                  src/alutLoader.c
                  src/alutOutputStream.c
+                 src/alutStream.cpp
                  src/alutUtil.c
                  src/alutVersion.c
                  src/alutWaveform.c)
--- /dev/null 2008-04-18 22:12:15.169035187 -0700
+++ alut/src/alutStream.cpp 2008-06-08 01:47:13.000000000 -0700
@@ -0,0 +1,425 @@
+#include "alutInternal.h"
+
+#include <new>
+
+#include <vorbis/vorbisfile.h>
+
+#include <stdio.h>
+#include <string.h>
+
+/****************************************************************************/
+
+enum eError {
+    BufferGen,
+    BadContext
+};
+
+/****************************************************************************/
+
+struct StreamImpl : public ALUTStream {
+    virtual ALboolean ALUT_APIENTRY IsPlaying ()
+    {
+        ALint state;
+
+        if(!_alutSanityCheck())
+            return AL_FALSE;
+
+        if(!alIsSource(Source))
+        {
+            _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+            return AL_FALSE;
+        }
+
+        alGetSourcei(Source, AL_SOURCE_STATE, &state);
+        if(alGetError() != AL_NO_ERROR)
+        {
+            _alutSetError(ALUT_ERROR_IO_ERROR);
+            return AL_FALSE;
+        }
+        return (state==AL_PLAYING || (state!=AL_PAUSED && VorbisInfo)) ? AL_TRUE : AL_FALSE;
+    }
+
+    virtual ALboolean ALUT_APIENTRY Poll ()
+    {
+        int total = BufferLength*VorbisInfo->channels*2;
+        ALint processed, state;
+        ALuint buffer;
+        int bytes, i;
+
+        if(!_alutSanityCheck())
+            return AL_FALSE;
+
+        if(!alIsSource(Source))
+        {
+            _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+            return AL_FALSE;
+        }
+
+        if(!VorbisInfo)
+            return AL_FALSE;
+
+        alGetSourcei(Source, AL_BUFFERS_PROCESSED, &processed);
+        if(alGetError() != AL_NO_ERROR)
+        {
+            _alutSetError(ALUT_ERROR_IO_ERROR);
+            return AL_FALSE;
+        }
+        if(processed <= 0)
+            return AL_TRUE;
+
+        for(i = 0;i < processed;i++)
+        {
+            alSourceUnqueueBuffers(Source, 1, &buffer);
+            if(alGetError() != AL_NO_ERROR)
+            {
+                _alutSetError(ALUT_ERROR_IO_ERROR);
+                return AL_FALSE;
+            }
+
+            bytes = 0;
+            while(bytes < total)
+            {
+                int ret = ov_read(&OggFile, Data+bytes, total-bytes, Endian, 2, 1,
+                                  &BitStream);
+                if(ret < 0)
+                {
+                    _alutSetError(ALUT_ERROR_IO_ERROR);
+                    return AL_FALSE;
+                }
+                if(ret == 0)
+                    break;
+                bytes += ret;
+            }
+            if(bytes == 0)
+            {
+                if(VorbisInfo)
+                    ov_clear(&OggFile);
+                VorbisInfo = NULL;
+                break;
+            }
+
+            if((alBufferData(buffer, Format, Data, bytes, VorbisInfo->rate),
+                alGetError() != AL_NO_ERROR) ||
+               (alSourceQueueBuffers(Source, 1, &buffer),
+                alGetError() != AL_NO_ERROR))
+            {
+                _alutSetError(ALUT_ERROR_BUFFER_DATA);
+                return AL_FALSE;
+            }
+        }
+        // Return false if no buffers got filled
+        if(i == 0)
+            return AL_FALSE;
+
+        alGetSourcei(Source, AL_SOURCE_STATE, &state);
+        if(alGetError() != AL_NO_ERROR)
+        {
+            _alutSetError(ALUT_ERROR_IO_ERROR);
+            return AL_FALSE;
+        }
+
+        if(state != AL_PLAYING)
+        {
+            alSourcePlay(Source);
+            if(alGetError() != AL_NO_ERROR)
+            {
+                _alutSetError(ALUT_ERROR_IO_ERROR);
+                return AL_FALSE;
+            }
+        }
+
+        return AL_TRUE;
+    }
+
+    virtual ALboolean ALUT_APIENTRY PlayOnSource (ALuint sourceID)
+    {
+        int bytes, cnt, total;
+
+        if(!_alutSanityCheck())
+            return AL_FALSE;
+
+        if(!alIsSource(sourceID))
+        {
+            _alutSetError(ALUT_ERROR_INVALID_VALUE);
+            return AL_FALSE;
+        }
+
+        if(InitStream() == AL_FALSE)
+            return AL_FALSE;
+        total = BufferLength*VorbisInfo->channels*2;
+
+        for(cnt = 0;cnt < NumBuffers;cnt++)
+        {
+            bytes = 0;
+            while(bytes < total)
+            {
+                int ret = ov_read(&OggFile, Data+bytes, total - bytes, Endian, 2, 1,
+                                  &BitStream);
+                if(ret < 0)
+                {
+                    _alutSetError(ALUT_ERROR_IO_ERROR);
+                    return AL_FALSE;
+                }
+                if(ret == 0)
+                    break;
+                bytes += ret;
+            }
+            if(bytes == 0)
+            {
+                if(VorbisInfo)
+                    ov_clear(&OggFile);
+                VorbisInfo = NULL;
+                break;
+            }
+
+            alBufferData(Buffers[cnt], Format, Data, bytes, VorbisInfo->rate);
+            if(alGetError() != AL_NO_ERROR)
+            {
+                _alutSetError(ALUT_ERROR_BUFFER_DATA);
+                return AL_FALSE;
+            }
+        }
+
+        alSourcei(sourceID, AL_BUFFER, 0);
+        alSourceQueueBuffers(sourceID, cnt, Buffers);
+        if(alGetError() != AL_NO_ERROR)
+        {
+            _alutSetError(ALUT_ERROR_BUFFER_DATA);
+            return AL_FALSE;
+        }
+
+        alSourcePlay(sourceID);
+        if(alGetError() != AL_NO_ERROR)
+        {
+            alSourcei(Source, AL_BUFFER, 0);
+            alGetError();
+
+            _alutSetError(ALUT_ERROR_BUFFER_DATA);
+            return AL_FALSE;
+        }
+
+        Source = sourceID;
+        return AL_TRUE;
+    }
+
+    virtual ALboolean ALUT_APIENTRY Stop ()
+    {
+        if(!_alutSanityCheck())
+            return AL_FALSE;
+
+        if(!alIsSource(Source))
+        {
+            _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+            return AL_FALSE;
+        }
+
+        if(VorbisInfo)
+            ov_clear(&OggFile);
+        VorbisInfo = NULL;
+
+        alSourceStop(Source);
+        alSourcei(Source, AL_BUFFER, 0);
+        alGetError();
+
+        Source = 0;
+        return AL_TRUE;
+    }
+
+
+    ALboolean InitStream()
+    {
+        FILE *f;
+
+        if(VorbisInfo)
+            ov_clear(&OggFile);
+        VorbisInfo = NULL;
+
+        f = fopen(Filename, "rb");
+        if(!f)
+            return AL_FALSE;
+
+        ov_open_callbacks(f, &OggFile, NULL, 0, OV_CALLBACKS_DEFAULT);
+        // Get some information about the OGG file
+        VorbisInfo = ov_info(&OggFile, -1);
+
+        delete[] Data;
+        Data = new char[BufferLength*VorbisInfo->channels*2];
+
+        // Check the number of channels... always use 16-bit samples
+        if(VorbisInfo->channels == 1)
+            Format = AL_FORMAT_MONO16;
+        else if(VorbisInfo->channels == 2)
+            Format = AL_FORMAT_STEREO16;
+        else
+        {
+            ov_clear(&OggFile);
+            VorbisInfo = NULL;
+
+            _alutSetError(ALUT_ERROR_UNSUPPORTED_FILE_SUBTYPE);
+            return AL_FALSE;
+        }
+
+        return AL_TRUE;
+    }
+
+
+    StreamImpl(const char *fileName, ALuint bufferLength,
+               ALuint bufferCount)
+        : VorbisInfo(NULL),
+          Endian(0), // FIXME: 0 for Little-Endian, 1 for Big-Endian
+          NumBuffers(bufferCount), Buffers(NULL), BufferLength(bufferLength),
+          Data(NULL), Source(0)
+    {
+        Filename = strdup(fileName);
+        if(!Filename)
+            throw std::bad_alloc();
+
+        if(NumBuffers < 2)
+        {
+            _alutSetError(ALUT_ERROR_GEN_BUFFERS);
+            throw BufferGen;
+        }
+        if(BufferLength == 0)
+        {
+            _alutSetError(ALUT_ERROR_BUFFER_DATA);
+            throw BufferGen;
+        }
+
+        Buffers = new ALuint[NumBuffers];
+        alGenBuffers(NumBuffers, Buffers);
+        if(alGetError() != AL_NO_ERROR)
+        {
+            delete[] Buffers;
+            Buffers = NULL;
+
+            free(Filename);
+            Filename = NULL;
+
+            _alutSetError(ALUT_ERROR_GEN_BUFFERS);
+            throw BufferGen;
+        }
+    }
+
+    virtual ~StreamImpl()
+    {
+        if(VorbisInfo)
+            ov_clear(&OggFile);
+        VorbisInfo = NULL;
+
+        if(Source)
+        {
+            if(!alIsSource(Source))
+            {
+                _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+                throw BadContext;
+            }
+
+            alSourceStop(Source);
+            alSourcei(Source, AL_BUFFER, 0);
+            Source = 0;
+        }
+
+        free(Filename);
+        Filename = NULL;
+
+        if(Buffers)
+            alDeleteBuffers(NumBuffers, Buffers);
+        delete[] Buffers;
+        Buffers = NULL;
+
+        delete[] Data;
+        Data = NULL;
+    }
+
+private:
+    char *Filename;
+
+    vorbis_info *VorbisInfo;
+    OggVorbis_File OggFile;
+    const int Endian;
+    int BitStream;
+
+    const ALuint NumBuffers;
+    ALuint *Buffers;
+    const ALuint BufferLength;
+    char *Data;
+
+    ALenum Format;
+
+    ALuint Source;
+};
+
+/****************************************************************************/
+
+extern "C" {
+
+ALUT_API ALUTStream* ALUT_APIENTRY alutCreateStreamFromFile (const char *fileName, ALuint length, ALuint bufferCount)
+{
+    try {
+        return new StreamImpl(fileName, length, bufferCount);
+    } catch(eError /*err*/) {
+        // the error is already set
+    } catch(std::bad_alloc /*ba*/) {
+        _alutSetError(ALUT_ERROR_OUT_OF_MEMORY);
+    } catch(...) {
+        // Something really bad if we're here
+        throw;
+    }
+    return NULL;
+}
+
+ALUT_API ALboolean ALUT_APIENTRY alutStreamIsPlaying (ALUTStream *stream)
+{
+    if(!stream)
+    {
+        _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+        return AL_FALSE;
+    }
+    return stream->IsPlaying();
+}
+
+ALUT_API ALboolean ALUT_APIENTRY alutStreamPoll (ALUTStream *stream)
+{
+    if(!stream)
+    {
+        _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+        return AL_FALSE;
+    }
+    return stream->Poll();
+}
+
+ALUT_API ALboolean ALUT_APIENTRY alutStreamPlayOnSource (ALUTStream *stream, ALuint sourceID)
+{
+    if(!stream)
+    {
+        _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+        return AL_FALSE;
+    }
+    return stream->PlayOnSource(sourceID);
+}
+
+ALUT_API ALboolean ALUT_APIENTRY alutStreamStop (ALUTStream *stream)
+{
+    if(!stream)
+    {
+        _alutSetError(ALUT_ERROR_INVALID_OPERATION);
+        return AL_FALSE;
+    }
+    return stream->Stop();
+}
+
+ALUT_API ALboolean ALUT_APIENTRY alutDestroyStream (ALUTStream *stream)
+{
+    if(!_alutSanityCheck())
+        return AL_FALSE;
+
+    try {
+        delete stream;
+    } catch(eError /*err*/) {
+        return AL_FALSE;
+    }
+
+    return AL_TRUE;
+}
+
+} // extern "C"

---

_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Chris Robinson-5]

Seems the patch didn't quite come out properly... sorry about that.

Here's the SVN diff:
http://kcat.strangesoft.net/alut-stream.diff
and the diff for the new file:
http://kcat.strangesoft.net/alutStream.cpp.diff

and the example again:
http://kcat.strangesoft.net/alutstream.cpp
_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Sherief N. Farouk]

> Here's a patch that can/should be able to be applied to alut in svn to
> show off the combined C/C++ proof-of-concept idea, as well as a general
> API I'd go for. You can find a sample C++ app using it here:
> http://kcat.strangesoft.net/alutstream.cpp

[Sherief N. Farouk]

Regarding the line:
stream = alutCreateStreamFromFile(argv[1], 8192, 4);

Why not have that functionality in the constructor? The current way seems a
bit too attached to C heritage. How about:

stream = new alut::Stream(argv[1], 8192, 4);

Minor point: I vote for all lower-case class names (alut::stream VS Stream),
ala STL. C++ aimed to make user-created types and native types on a similar
ground, and most C++ de-facto libraries (Boost, for one) use that
convention.

- Sherief

_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Chris Robinson-5]

On Sunday 08 June 2008 03:27:16 pm Sherief N. Farouk wrote:
> Regarding the line:
> stream = alutCreateStreamFromFile(argv[1], 8192, 4);
>
> Why not have that functionality in the constructor? The current way seems a
> bit too attached to C heritage.

Because the actual implementation declaration is never exported from the lib
(and currently, isn't even declared outside of the file it's in). Don't
forget, ALUTStream (which really should be ALUTstream to better match AL
type-naming) is a virtual interface with no implementation. The actual
implementation is a StreamImpl object, but the app uses it through the base
ALUTstream type. An app can't explicitly new an ALUTstream object because it
has pure virtuals (the compiler will throw an error).

> How about:
>
> stream = new alut::Stream(argv[1], 8192, 4);

Hmm. The main issue there would be overriding new just for that type. Is it
possible to make 'new alut::stream(a,b,c)' transparently
call 'alutCreateStreamFromFile(a,b,c)'? I've never played around with
overriding new before.

> Minor point: I vote for all lower-case class names (alut::stream VS
> Stream), ala STL. C++ aimed to make user-created types and native types on
> a similar ground, and most C++ de-facto libraries (Boost, for one) use that
> convention.

For C, I think it should be ALUTstream so it better matches AL's type-naming
(ALuint, ALboolean, etc). Though with a C++ alut namespace, I don't have an
issue with alut::stream being a mirror of ALUTstream (though it must be
possible that an ALUTstream* and a alut::stream* be interchangeable).
_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Sherief N. Farouk]

> Because the actual implementation declaration is never exported from
> the lib
> (and currently, isn't even declared outside of the file it's in). Don't
> forget, ALUTStream (which really should be ALUTstream to better match
> AL
> type-naming) is a virtual interface with no implementation. The actual
> implementation is a StreamImpl object, but the app uses it through the
> base
> ALUTstream type. An app can't explicitly new an ALUTstream object
> because it
> has pure virtuals (the compiler will throw an error).

[Sherief N. Farouk]

Why's that? Why not a base class with overridable, virtual functions? You do
know the concrete type anyway, which is a stream-from-file, so why not give
it a natural C++ syntax rather than a C-like bridge? There's an actual
function doing the work anyway, the only difference here is including it in
the constructor.

> > How about:
> >
> > stream = new alut::Stream(argv[1], 8192, 4);
>
> Hmm. The main issue there would be overriding new just for that type.
> Is it
> possible to make 'new alut::stream(a,b,c)' transparently
> call 'alutCreateStreamFromFile(a,b,c)'? I've never played around with
> overriding new before.

[Sherief N. Farouk]

Why would you need to override the new operator? I think you have this bit
mixed up. You override new to provide your own allocations. You can make
alut::stream transparently call anything you want, by placing that call in
the constructor.

> > Minor point: I vote for all lower-case class names (alut::stream VS
> > Stream), ala STL. C++ aimed to make user-created types and native
> types on
> > a similar ground, and most C++ de-facto libraries (Boost, for one)
> use that
> > convention.
>
> For C, I think it should be ALUTstream so it better matches AL's type-
> naming
> (ALuint, ALboolean, etc). Though with a C++ alut namespace, I don't
> have an
> issue with alut::stream being a mirror of ALUTstream (though it must be
> possible that an ALUTstream* and a alut::stream* be interchangeable).

[Sherief N. Farouk]

Wouldn't mind that at all. I'm talking from a pure C++ perspective here.

- Sherief

_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Chris Robinson-5]

On Sunday 08 June 2008 04:39:20 pm Sherief N. Farouk wrote:

> > Because the actual implementation declaration is never exported from
> > the lib
> > (and currently, isn't even declared outside of the file it's in). Don't
> > forget, ALUTStream (which really should be ALUTstream to better match
> > AL
> > type-naming) is a virtual interface with no implementation. The actual
> > implementation is a StreamImpl object, but the app uses it through the
> > base
> > ALUTstream type. An app can't explicitly new an ALUTstream object
> > because it
> > has pure virtuals (the compiler will throw an error).
>
> [Sherief N. Farouk]
>
> Why's that? Why not a base class with overridable, virtual functions? You
> do know the concrete type anyway, which is a stream-from-file, so why not
> give it a natural C++ syntax rather than a C-like bridge?

Because the implementation may change, and if the implementation is exposed to
the user app it'll break ABI if it ever changes. The object should be
passible between C and C++ (use the C++ interface in C++ code, even if the
object is created in C code, and vice versa).


Unless you're talking about something like this:

struct ALUTstream {
    virtual ALboolean ALUT_APIENTRY IsPlaying ()
    { return This->IsPlaying(); }
    virtual ALboolean ALUT_APIENTRY Poll ()
    { return This->Poll(); }
    virtual ALboolean ALUT_APIENTRY PlayOnSource (ALuint sourceID)
    { return This->PlayOnSource(sourceID); }
    virtual ALboolean ALUT_APIENTRY Stop ()
    { return This->Stop(); }

    ALUTstream(a,b,c) : This(alutCreateStreamFromFile(a,b,c))
    { }
    virtual ~ALUTstream()
    { delete This; };

private:
    ALUTstream *This;
};

But that's pretty ugly, with more levels of indirection. Plus, there's a
problem when you do something like this:

{
    alut::stream stream(a,b,c);
    stream.PlayOnSource(sourceID);
    alcMakeContextCurrent(NULL);
}

The stream will attempt to destroy itself when it goes out of scope, but
because there's a NULL context, it can't stop the source, and detach/delete
the (internal) AL buffers. As it is right now, alutDestroyStream will return
AL_FALSE and delete will throw an exception (what exactly it'll throw I've
not determined yet). This lets you check the error and retry destroying it.

However, in the case above, an exception would be thrown but the object is
invariably lost, so the AL buffers are leaked because they couldn't be
deleted and there's no way to get them.
_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Sherief N. Farouk]

> Because the implementation may change, and if the implementation is
> exposed to
> the user app it'll break ABI if it ever changes. The object should be
> passible between C and C++ (use the C++ interface in C++ code, even if
> the
> object is created in C code, and vice versa).

[Sherief N. Farouk]
A part has to be exposed to the user. Exposing stream::stream() is no
different than exposing CreateStreamFromFile for this case.

> Unless you're talking about something like this:
>
> struct ALUTstream {
>     virtual ALboolean ALUT_APIENTRY IsPlaying ()
>     { return This->IsPlaying(); }
>     virtual ALboolean ALUT_APIENTRY Poll ()
>     { return This->Poll(); }
>     virtual ALboolean ALUT_APIENTRY PlayOnSource (ALuint sourceID)
>     { return This->PlayOnSource(sourceID); }
>     virtual ALboolean ALUT_APIENTRY Stop ()
>     { return This->Stop(); }
>
>     ALUTstream(a,b,c) : This(alutCreateStreamFromFile(a,b,c))
>     { }
>     virtual ~ALUTstream()
>     { delete This; };
>
> private:
>     ALUTstream *This;
> };

[Sherief N. Farouk]
No. And there's something fundamentally wrong with that sample. If
ALUTstream is the C object, how can a call like This->PlayOnSource(sourceID)
work? There's no "this" parameter in C, so the only case this is
syntactically correct C is when ALUTstream has a member that's a pointer to
function called PlayOnSource. But then that function doesn't have sufficient
information: only parameter is a source ID. You can have the C-function be
merely thunks that call the C++ code in the end. Same pointer can be used
for both objects in this case.

> But that's pretty ugly, with more levels of indirection. Plus, there's
> a
> problem when you do something like this:
>
> {
>     alut::stream stream(a,b,c);
>     stream.PlayOnSource(sourceID);
>     alcMakeContextCurrent(NULL);
> }
>
> The stream will attempt to destroy itself when it goes out of scope,
> but
> because there's a NULL context, it can't stop the source, and
> detach/delete
> the (internal) AL buffers. As it is right now, alutDestroyStream will
> return
> AL_FALSE and delete will throw an exception (what exactly it'll throw
> I've
> not determined yet). This lets you check the error and retry destroying
> it.
>
> However, in the case above, an exception would be thrown but the object
> is
> invariably lost, so the AL buffers are leaked because they couldn't be
> deleted and there's no way to get them.

[Sherief N. Farouk]
If you have live objects after you call alcMakeContextCurrent(NULL), You're
beyond help anyway, and you totally and absolutely deserve what's coming to
you. This is the C++ equivalent of someone calling AL functions after a
MakeContextCurrent(NULL) call - they had it coming! :)

- Sherief

_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Chris Robinson-5]

On Sunday 08 June 2008 06:11:02 pm Sherief N. Farouk wrote:
> A part has to be exposed to the user. Exposing stream::stream() is no
> different than exposing CreateStreamFromFile for this case.

True enough, but whatever is exposed needs to remain exposed (and in the same
order). Adding stuff is fine, but it has to be added to the end. I'd also say
that anything exposed is fair game to the app. If the struct has a variable,
the variable needs to continue to be used the same way by the implementation
or else it can break apps. If something isn't for the app to touch, it
shouldn't be exposed for the app to touch.

> > Unless you're talking about something like this:
> >
> > struct ALUTstream {
> >     virtual ALboolean ALUT_APIENTRY IsPlaying ()
> >     { return This->IsPlaying(); }
> >     virtual ALboolean ALUT_APIENTRY Poll ()
> >     { return This->Poll(); }
> >     virtual ALboolean ALUT_APIENTRY PlayOnSource (ALuint sourceID)
> >     { return This->PlayOnSource(sourceID); }
> >     virtual ALboolean ALUT_APIENTRY Stop ()
> >     { return This->Stop(); }
> >
> >     ALUTstream(a,b,c) : This(alutCreateStreamFromFile(a,b,c))
> >     { }
> >     virtual ~ALUTstream()
> >     { delete This; };
> >
> > private:
> >     ALUTstream *This;
> > };
>
> [Sherief N. Farouk]
> No. And there's something fundamentally wrong with that sample. If
> ALUTstream is the C object, how can a call like
> This->PlayOnSource(sourceID) work?

Because in C you call alutStreamPlayOnSource, which is implemented using C++
(but with C linkage), which can make calls like that just fine. In C, the
struct isn't exposed at all (it's completely opaque). It's evil, though,
because essentially this is what happens for new'd/stack objects:
alutStreamPlayOnSource(stream, x); // if used like C
  stream->PlayOnSource(x);
    stream->This->PlayOnSource(x);
      ...implementation here...
Plus the aforementioned leak potential for stack objects.

> There's no "this" parameter in C

There's no virtual or member functions in C, either (at most there's function
pointers, but as you say the 'this' pointer isn't implicit). Which is why in
C, the struct is opaque and used via the alutStream* functions, while in C++,
the struct is defined to have a set of pure virtual functions and can be used
either way.

The only "ugliness" is having to call an alutCreateStreamFrom* function to get
an object in C++ instead of new'ing it. A fair trade-off, IMO, considering it
can be used C++-ish otherwise (derive from it and make your own
implementation, use auto_ptr or boost's shared pointer, whatever).

> If you have live objects after you call alcMakeContextCurrent(NULL), You're
> beyond help anyway, and you totally and absolutely deserve what's coming to
> you. This is the C++ equivalent of someone calling AL functions after a
> MakeContextCurrent(NULL) call - they had it coming! :)

Perhaps, but even calling AL functions with no context doesn't irreversably
leak objects. You get an error and a chance to correct it. (and I used NULL
as a simple example; It could just as easilly happen with another real
context since sources are bound to the context they're created in).

I agree we don't need to be hand-holding (if they do something dumb, bad
things will happen), but leaks caused by the lib, even if by proxy, are bad
and should be avoided whenever possible. If the user wants to then ignore the
error, there's nothing we can do about that. They at least had the
opportunity to see "hey, something's wrong here..".

Of course it doesn't help that contexts aren't per thread, otherwise I'd be
able to store the context the source uses, and set it when deleting (and
reset it back after). But because another thread may be using the context,
there's no way to syncronize for the temporary switch..
_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel

[Jason Daly]

Chris Robinson wrote:

Minor point: I vote for all lower-case class names (alut::stream VS
Stream), ala STL. C++ aimed to make user-created types and native types on
a similar ground, and most C++ de-facto libraries (Boost, for one) use that
convention.
    

For C, I think it should be ALUTstream so it better matches AL's type-naming
(ALuint, ALboolean, etc). Though with a C++ alut namespace, I don't have an
issue with alut::stream being a mirror of ALUTstream (though it must be
possible that an ALUTstream* and a alut::stream* be interchangeable).
  

FWIW, if there's a C++ API, I think it should stick with the OpenAL style and not adopt another style just because it's there.  I never liked the STL structure and style personally, but more to the point, I don't think it's a good idea to change style between two API's of the same library.

But, that's my opinion  :-)

--"J"


_______________________________________________
Openal-devel mailing list
Openal-devel@...
http://opensource.creative.com/mailman/listinfo/openal-devel