pub.mod/oggvorbis.mod: faulty loop?

Archives Forums/BlitzMax Bug Reports/pub.mod/oggvorbis.mod: faulty loop?

Derron

(Posted 2016) [#1]

the following code is currently used in pub.mod/oggvorbis.mod:

  int Read_Ogg(oggio *ogg,char *buf,int bytes)	// null buffer to close
  {
	int		res,bs;
  
  	if (buf==0) return ov_clear(&ogg->vf);
  
  	while (bytes>0)
  	{
  		res=ov_read(&ogg->vf,buf,bytes,endian,bits/8,sign,&bs);
		if (res<0)
		{
  			if (bs) return -1;	// Only one logical bitstream currently supported
  			return -2;			// Warning: hole in data
  		}
  		buf+=res;
  		bytes-=res;
  	}
	return 0;
  }

There are two issues:
- Read_Ogg() does return "0" instead of the amount of read bytes (which might differ from the param "bytes")
- what happens, if you read up to the last byte of the ogg file - so "ov_read" returns 0 ... I assume you are then trapped in the while-loop, as "bytes" will never reach "0"

I am not sure whether I have done it right, but I think this would solve both issues:

int Read_Ogg(oggio *ogg,char *buf,int bytes)	// null buffer to close
{
	int		res,bs,read;

	if (buf==0) return ov_clear(&ogg->vf);

	read = 0;

	while (bytes>0)
	{
		res=ov_read(&ogg->vf,buf,bytes,endian,bits/8,sign,&bs);
		if (res<0)
		{
			if (bs) return -1;	// Only one logical bitstream currently supported
			return -2;			// Warning: hole in data
		}
		else if (res == 0) // reached eof
		{
			return read;
		}
		read+=res;
		buf+=res;
		bytes-=res;
	}
	return read;
}

The returned value "read" contains the bytes loaded by the multiple calls of ov_read().

bye
Ron

col	(Posted 2016) [#2]

Derron

(Posted 2016) [#3]

ov_read():

It returns up to the specified number of bytes of decoded PCM audio in the requested endianness, signedness, and word size.

[...]

Return Values
OV_HOLE
indicates there was an interruption in the data.
(one of: garbage between pages, loss of sync followed by recapture, or a corrupt page)

OV_EBADLINK
indicates that an invalid stream section was supplied to libvorbisfile, or the requested link is corrupt.

OV_EINVAL
indicates the initial file headers couldn't be read or are corrupt, or that the initial open call for vf failed.

0
indicates EOF

n
indicates actual number of bytes read. ov_read() will decode at most one vorbis packet per invocation, so the value returned will generally be less than length.

https://www.xiph.org/vorbis/doc/vorbisfile/ov_read.html

In oggvorbis.mod/libvorbis-1.3.4/include/vorbis/codec.h the following consts are defined:

#define OV_EOF        -2
#define OV_HOLE       -3
[...]
#define OV_EBADLINK   -137

So there is an error code for reaching EOF, while the documentation mentions a returned "0" is also EOF ?

bye
Ron

col	(Posted 2016) [#4]

Perhaps the error code is only returned when continuing to read passed the EOF. So when an EOF is reached initially the return value will be 0, then upon trying again the error value is returned?

Derron

(Posted 2016) [#5]

Something to try out (you need to supply your own "sound3.ogg" - and take a small one, so it has less than 1mio samples):

SuperStrict
Import Pub.OggVorbis


local ogg:TOggFile = new TOggFile
ogg.Test()
print "finished"


Type TOggFile
	Field stream:TStream

	Method Test()
		local samplesCount:int
		local channels:int
		local freq:int
		local ogg:Byte Ptr
		local buffer:int[] = new Int[1000000]
		local bufferPtr:Byte Ptr = Byte Ptr(buffer)

		stream = ReadStream("sound3.ogg")
		ogg = Decode_Ogg(stream, readfunc, seekfunc, closefunc, tellfunc, samplesCount, channels, freq)
		local bytes:int = Read_Ogg(ogg, bufferPtr, 1000000)
		print "samples:"+samplesCount+"  channels:"+channels +"  maxBytes:"+bytes
		'close
		Read_Ogg(ogg, null, 0)
		stream.Close()


		local stream2:TStream = ReadStream("sound3.ogg")
		local ogg2:Byte Ptr
		ogg2 = Decode_Ogg(stream2, readfunc, seekfunc, closefunc, tellfunc, samplesCount, channels, freq)
		print "result with max bytecount +1 (EOF):" +Read_Ogg(ogg2, bufferPtr, bytes)
		print "result with max bytecount +1 (EOF):" +Read_Ogg(ogg2, bufferPtr, 1)
		'close
		Read_Ogg(ogg, null, 0)
		stream2.Close()
	End Method

	'adjusted from brl.mod/oggloader.mod/oggloader.bmx
	'they are "private" there... so this is needed to expose them
	Function readfunc:Int( buf:Byte Ptr, size:Int, nmemb:Int, src:Object )
		Return TStream(src).Read(buf, size * nmemb) / size
	End Function


	Function seekfunc:Int(src_obj:Object, off0:Int, off1:Int, whence:Int)
		Local src:TStream=TStream(src_obj)
	?X86
		Local off:Int = off0
	?PPC
		Local off:Int = off1
	?
		Local res:Int = -1
		Select whence
			Case 0 'SEEK_SET
				res = src.Seek(off)
			Case 1 'SEEK_CUR
				res = src.Seek(src.Pos()+off)
			Case 2 'SEEK_END
				res = src.Seek(src.Size()+off)
		End Select
		If res >= 0 Then Return 0
		Return -1
	End Function


	Function closefunc(src:Object)
		
	End Function
	

	Function tellfunc:Int(src:Object)
		Return TStream(src).Pos()
	End Function
End Type

With the default implementation, it will get trapped in the second Read_Ogg(stream2...) call (application has to get force killed).

The first call to Read_ogg() returns 0 as it did not see another error, so we cannot know if we reached EOF yet.

With the adjusted Read_ogg the first call returns the "max bytes" while the second one returns 0 (the EOF-result).

Don't know which one is better ;-)

col	(Posted 2016) [#6]

ith the default implementation, it will get trapped in the second Read_Ogg(stream2...) call (application has to get force killed).

Oh, thats not good!
Well spotted!!

Don't know which one is better ;-)

I'd say if one your algos can reflect a change of 'state' in the return value then that is the better way to go.