Ogg Player

I'm confused to why the 2nd section of code works just fine, but 1st section does not.
(the player does look to support a FILE descriptor, forget about WiiFile it's just a simple wrapper)
REF: \devkitPro\examples\wii\audio\oggplayer

	string FullFileName = Util::GetGamePath() + "Indidginus.ogg";
	FILE* pOggFile( WiiFile::FileOpenForRead( FullFileName.c_str() ) );

	u32 OggSize = WiiFile::GetFileSize(pOggFile);

	PlayOgg(pOggFile, OggSize, 0, OGG_ONE_TIME);

	string FullFileName = Util::GetGamePath() + "Indidginus.ogg";

	FILE* pOggFile( WiiFile::FileOpenForRead( FullFileName.c_str() ) );
	u32 OggSize = WiiFile::GetFileSize(pOggFile);

	u8* pOggData = (u8*) malloc(OggSize);
	fread( pOggData, OggSize, 1, pOggFile);

	PlayOgg(pOggData, OggSize, 0, OGG_ONE_TIME);

Wow thats nasty looking.

I think its has to do with the position of the file read head/counter/thing. I think you may need a "rewind()" or something.



Edited 1 time(s). Last edit at 09/14/2011 01:06AM by owen.

Thanks for the idea Owen - but the 3rd parameter lets you set the time position, I'm using 0 so it should play from the start. Once run the interface provides a "void SetTimeOgg(s32 time_pos)" that sets seek, but the setup also sets seek from the 3rd pram, so no luck looking there.

Anyone made this work directly from file?

All this is not really a problem as I'm loading oggs into memory anyway; it would just be nice to know why it does not play directly from file.
The oggplayer code looks well written, but I'm going to break the code into two parts since I don't require the FILE support. If the File stuff does not work then I might as well just delete those parts from my version and benefit from the tiny optimisations.



Edited 1 time(s). Last edit at 09/14/2011 02:14PM by Titmouse.

The first code blob doesn't have a chance of working. PlayOgg() expects you to pass it a void* that points to an ogg bitstream and you're passing it a FILE*.

Thanks tueidj, but a void pointer is polymorphic.
The first thing PlayOgg does is cast the void* to something usefull.

int PlayOgg(const void *buffer, s32 len, int time_pos, int mode)
{
	StopOgg();

	private_ogg.fd = mem_open((char *)buffer, len);

               ...

No, that's not how polymorphism works. You can't just cast from one type to a completely different one. Regardless, oggplayer is C code and has no idea about such things.
That's a plain old cast to a char* which is completely incompatible with a FILE*.

lol, polymorphic, those things only apply in YOUR classes or code that you can see, most system code avoids syntax magic.

tueidj, when exactly did I say Polymorphism?
I said void* is polymorphic and it is (look it up), you can use the word polymorphic without encroaching on the object orientated world.

Your second post is just not true since in C you can pretty much cast anything to anything.

	
char* ptr = (char*) pOggFile; // here pOggFile  is a FILE*
FILE* ptr2= (FILE*)ptr;

But this is all silly and going off subject. (If you read you first post again I’m sure that you don’t mean “expects you to pass it a void*” that makes no sense as you can pass anything using void*)

...Anyway, since the OggPlayer uses the unistd library, I guessing maybe I’m using the wrong flavor of file descriptor.

Think something like this should work, but I’ve just not had a chance to test it.

int fd = open (FullFileName.c_str(), O_WRONLY);

I guessing you would pass in either a buffer pointer (say u8*) or &fd (address of), depending if you used a allocated block of memory or direct from file.

(Owen - my code example uses a namespace, it would have to be some kind of nasty static to look like that, so sorry nothing todo with a classes)

You're not supposed to pass in a file descriptor at all, you're meant to pass in a pointer to some memory that contains the bitstream. Not a pointer to an fd or a FILE struct. Think about it - there's no way that oggplayer could magically know it should do file I/O (or which type of I/O it should use) when you're only passing it a void pointer.

BTW, a void pointer is only polymorphic if you have a method of knowing what it really points to. Otherwise you're just blindly casting and it doesn't work (as this example shows).



Edited 1 time(s). Last edit at 09/15/2011 10:00PM by tueidj.

My first post is using allocate memory it works fine.

Now because it uses a void* that suggests it can take many flavors, not just a raw memory pointer as you suggest, think about it.Void* is just for situations like this when you either need to pass a file descriptor or a pointer to some memory.

The code has a magic 0x666 logic around file or memory access, the code knows if its incoming from file or memory.

However I've just discovered this, it looks like the OggVorbis_File part is not setup before it's fed to ov_open_callbacks otherwise passing both a pointer to memory or a File descriptor should work.

if (ov_open_callbacks((void *) &private_ogg.fd, &private_ogg.vf, NULL, 0, callbacks) < 0)

I'm going to remove all file access stuff from OggPlayer since I don't need it.

Bullshit. If you pass a void* to a function it has no idea what the memory really points to, unless it points to a struct or union that includes a description of the full type.
That 0x666 stuff in oggplayer is just some ugly kludge that someone has hacked in to implement their own memory-based file I/O functions for libvorbis. Pretty silly considering they're static and there's no chance of an actual fd reaching those callbacks.

Look the function takes void*, again this is why it makes no sense to use void*, it suggests big time the interface takes more than one flavor. Why else would anyone use a void*?
At the time of my first post I was thinking, the void* was for that reason, now I'm sure it was for fd or pointer.

Off the top here are a few ways to work out what a void* is;
- Looking at the incoming fd (i.e 1) or a dereference pointer (i.e 1638400) so each can be identified and used appropriately. (the unistd library uses int for fd so its numbers are small and start small unlike a memory pointer)
- or testing the fd to see if it has a valid file id.
- or Since its a fd, set the size param to say -1

I’m happy to say I don’t ever use void* in my own code, and would never dream if using the above methods since it would probably break across platforms.

ModPlayer has lseek, read, close all based on the 0x666, in this case all are used for file access.

Looking at the code, 0x666 is added to the internall index so that at least 1638 fd ID’s can exist before thing starts to break.
The code looks like it's been designed to work from File or buffer. Like I said its just missing some code to setup the OggVorbis_File struct.

static long f_tell(int *f)
{
	int k, d;

	if (*f >= 0x666 && *f <= 0x669)
	{
		d = (*f) - 0x666;
		k = file[d].pos;
	}
	else
		k = lseek(*f, 0, 1);

	return (long) k;
}

This isn't C++. If it were possible to pass a FILE* instead of a memory buffer there would be another function that explicitly accepted it as a parameter. Kludging a function to take void* and doing risky tests on it that may or may not tell you the actual type is retarded, unsafe and insecure. Whoever wrote those 0x666 hacks should be ashamed of themselves, the library allows you to assign different callbacks for each separate stream so there's no need for that rubbish.
The reason it takes a void* as an argument is because it wants a block of memory of no specific type - just as long as the ogg bitstream data is there. It could be a u32 array, it could be a signed char array, doesn't matter - as long as it hold the bitstream. The fread and fwrite functions are exactly the same, the second example in your original post is actually doing an implicit cast because fread expects void* - would you try and pass it a FILE* instead in a misguided attempt to copy data from one file to another?

Yes I agreed with the essence of what you say, when a first posted this I did not realize just the interface would scrutinized. I know that both of us know what possible code wise is, all this is getting silly and it was probably a mistake to post up the FILE code as it gives the wrong impressing of what I’m asking as it was a loaded question and has deflected the direction of this thread.
I was hoping for an answerer like… “that code not finished, the file stuff is a red herring it will only play from buffer” or “you need to use the open command provided by the ‘unistd’ library and then fix the mem_open function to fudge the incoming file”

But I’m still curious (and only curious as I have no intention of using the hacked side), about why this code is made this way with the inbuilt file reading support. (forget about it can be done other ways) So not forgetting it has code to support functionality for reading from file and buffer, I saw things like lseek & read all fudged into the code alongside the buffer when I made may first post.

Please consider for a moment the code is looking for us to pass either an address to a int or a pointer to a buffer since it has two playback techniques.

Now if ‘mem_open’ had a nasty test to set the internal .fd to the dereferenced file, otherwise to 0x666 as it is now. Mem_open looks to have been patched up and has mayby lost whatever functionality was needed to make this hack work. I can see this working since the incoming fd would probbalby always be less than 0x666. Hence the 0x666 since its lager than a file discriptor and is easy to test for.
Then all the file code kicks in, and vola it plays from a file discriptor.
This is the only thing I’m trting to point out, it can play from a file discriptor, its just missing something in setup.