[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-multimedia
Subject:    Ogg vorbis (rc3) support for non 44100 Hz files - races detected
From:       Roger Larsson <roger.larsson () norran ! net>(by way of Roger Larsson <roger ! larsson
Date:       2002-01-29 22:55:39
[Download RAW message or body]

[found the list and remembered my wish list item]

Hi all (since I have no idea who is responsible for the code)

I have made "some" modifications to oggvorbis_artsplugin to make it handle
 non 44100 Hz files.

44100 Hz files is a lot easier since you decode to the same frequency as arts
uses internally...

The possibility to use other sampling frequencies and mono was added in
ogg rc3.

wav(f Hz, c Channels) => encode => ogg rc3 => decode => wav(f Hz, c Channels)

This gives some problems when trying to handle the standard arts frequency.
You basically need to resample - I have included a simple resampler.

Audio quality has not been my priority up to this point - now it gives right
audio for the correct time. (it works but gives bad audio quality -
overtones, a filter with the band width of the original sound will help...)
My feeling is that there are probably working resamplers included somewere in
the arts libraries... Help please?

BUT:

I think I detected quite a number of races while working with the code.

As an example:
when the file is read to end then this code was executed...

!               //signal completion
!               foo.val = 0;
!               // no more data available
!               semctl(buflen_sem, 0, SETVAL, foo);
!               // and no room either (ie, none coming)
!               semctl(buflen_sem, 1, SETVAL, foo);

Two problems:
* writing zero to sem 0 will not let the audio file play to the end.
  all samples that were buffered got removed in one stroke.
* writing zero to sem 1 might race with main process updating sem 1
  and the expected condition will never happen...

/RogerL

--
Roger Larsson
Skellefteċ
Sweden



["kde_oggvorbis_artsplugin.patch" (text/x-diff)]

? oggarts.mcoptype
? oggarts.h
? oggarts.cc
? oggarts.mcopclass
Index: oggPlayObject_impl.cpp
===================================================================
RCS file: /home/kde/kdemultimedia/oggvorbis_artsplugin/oggPlayObject_impl.cpp,v
retrieving revision 1.3
diff -c -r1.3 oggPlayObject_impl.cpp
*** oggPlayObject_impl.cpp	2001/12/14 20:54:43	1.3
--- oggPlayObject_impl.cpp	2002/01/29 22:02:00
***************
*** 32,41 ****
--- 32,43 ----
  
  
  #include "oggPlayObject_impl.h"
+ #define SAMPLESIZE sizeof(short)
  
  using namespace Arts;
  
  oggPlayObject_impl::oggPlayObject_impl()
+ 	: fract_bufpos(0), reserved(0)
  {
  	struct shmid_ds bleh;
  	shm_id = shmget(IPC_PRIVATE, sizeof(*shm_buf), 0600);
***************
*** 95,114 ****
  
  	FILE *ogg_fp;
  	ogg_fp = fopen(filename.c_str(), "r");
  	int current_section=0;
  
  	ov_open(ogg_fp, &vf, NULL, 0);
  	if (!(child_pid = fork())) {
  		arts_debug("oggvorbis: child process");
  		do {
! 			// get more data
  
  			int seekTo = semctl(buflen_sem, 2, GETVAL, foo);
  			if (seekTo) {
  				arts_debug("oggvorbis: seeking to %d", seekTo);
  				// we have a seek command
  				int ret = ov_time_seek(&vf, (double)seekTo-1);
! 				arts_debug("oggvorbis: ov_time_seek returned: %d\n", ret);
  				foo.val=0;
  				semctl(buflen_sem, 2, SETVAL, foo); // we've done it
  			}
--- 97,135 ----
  
  	FILE *ogg_fp;
  	ogg_fp = fopen(filename.c_str(), "r");
+ 	if (ogg_fp == NULL) {
+ 	    perror(filename.c_str());
+ 	    return false;
+ 	}
+ 
  	int current_section=0;
  
  	ov_open(ogg_fp, &vf, NULL, 0);
+  	arts_debug("oggvorbis: Encoded by: %s",ov_comment(&vf,-1)->vendor);
+  
+  	// Check user comments and info
+  	char **ptr=ov_comment(&vf,-1)->user_comments;
+  	while(*ptr){
+  	    arts_debug("oggvorbis: %s",*ptr);
+  	    ++ptr;
+  	}
+ 
+  	vorbis_info *vi=ov_info(&vf,-1);
+  	arts_debug("\noggvorbis: Bitstream is %d channel, %ldHz (hw %d \
Hz)",vi->channels,vi->rate, samplingRate); + 	shm_buf->rate = vi->rate;
+    
+  
  	if (!(child_pid = fork())) {
  		arts_debug("oggvorbis: child process");
  		do {
! 			// seek and indicate position
  
  			int seekTo = semctl(buflen_sem, 2, GETVAL, foo);
  			if (seekTo) {
  				arts_debug("oggvorbis: seeking to %d", seekTo);
  				// we have a seek command
  				int ret = ov_time_seek(&vf, (double)seekTo-1);
! 				arts_debug("oggvorbis: ov_time_seek returned: %d", ret);
  				foo.val=0;
  				semctl(buflen_sem, 2, SETVAL, foo); // we've done it
  			}
***************
*** 116,135 ****
  			if (foo.val == -1) foo.val=0;
  			semctl(buflen_sem, 3, SETVAL, foo);
  
! 			int thisPass = ov_read(&vf, (char *)decode_buf, BACKBUFSIZ*sizeof(short), 0, 2, \
                1, &current_section);
! 			if (thisPass == 0) {
  				// we're done, or we errored (in which case we're done)
  				break;
  			}
! 			thisPass = (thisPass / 4);
  
  
  			semoper.sem_num = 1;
  			semoper.sem_op = -thisPass;
  			semop(buflen_sem, &semoper, 1);
  
! 			// block until there's enough space to stick in this frame
  			int roomFor = semctl(buflen_sem, 1, GETVAL, foo);
  			if (roomFor > BACKBUFSIZ) {
  				arts_debug("oggvorbis: exit requested, bye!");
  				// this can never go above BACKBUFSIZ in normal operation,
--- 137,162 ----
  			if (foo.val == -1) foo.val=0;
  			semctl(buflen_sem, 3, SETVAL, foo);
  
! 			// get more data
! 
! 			// ALWAYS read 16 bit little endian, that specific format will be converted to \
                float later...
! 			int thisPass = ov_read(&vf, (char *)decode_buf, sizeof(decode_buf), 0, \
                SAMPLESIZE, 1, &current_section);
! 			if (thisPass <= 0) {
  				// we're done, or we errored (in which case we're done)
+ 				arts_debug("oggvorbis: ov_read returned: thisPass=%d", thisPass);
  				break;
  			}
! 			thisPass = (thisPass / (SAMPLESIZE * vi->channels));
  
  
+ 			// block until there's enough space to stick in this frame
  			semoper.sem_num = 1;
  			semoper.sem_op = -thisPass;
  			semop(buflen_sem, &semoper, 1);
  
! 			// check if exit is requested - non blocking
  			int roomFor = semctl(buflen_sem, 1, GETVAL, foo);
+ 
  			if (roomFor > BACKBUFSIZ) {
  				arts_debug("oggvorbis: exit requested, bye!");
  				// this can never go above BACKBUFSIZ in normal operation,
***************
*** 138,145 ****
  			}
  
  			for (int i=0 ; i <thisPass ; ++i, buf_pos = ((buf_pos + 1) % BACKBUFSIZ)) {
! 				shm_buf->left[buf_pos] = conv_16le_float(decode_buf[2*i]);
! 				shm_buf->right[buf_pos] = conv_16le_float(decode_buf[2*i+1]);
  			}
  
  			//arts_debug("oggvorbis: enqueued them");
--- 165,174 ----
  			}
  
  			for (int i=0 ; i <thisPass ; ++i, buf_pos = ((buf_pos + 1) % BACKBUFSIZ)) {
! 				// Sort of works for all number of channels...
! 				// TODO: proper handling of multichannel, do not waste work from mono
! 				shm_buf->left[buf_pos] = conv_16le_float(decode_buf[vi->channels*i]);
! 				shm_buf->right[buf_pos] = \
conv_16le_float(decode_buf[vi->channels*i+(vi->channels-1)]);  }
  
  			//arts_debug("oggvorbis: enqueued them");
***************
*** 150,161 ****
  			//arts_debug("oggvorbis: calculated %d more samples",shm_buf->back$
  		} while(1);
  
! 		//signal completion
! 		foo.val = 0;
! 		// no more data available
! 		semctl(buflen_sem, 0, SETVAL, foo);
! 		// and no room either (ie, none coming)
! 		semctl(buflen_sem, 1, SETVAL, foo);
  
  		arts_debug("oggvorbis: decoder process exiting");
  		exit(0);
--- 179,189 ----
  			//arts_debug("oggvorbis: calculated %d more samples",shm_buf->back$
  		} while(1);
  
! 		// block until all produced data has been consumed
! 		arts_debug("oggvorbis: decoder process waiting");
! 		semoper.sem_num = 0;
! 		semoper.sem_op =  0;
! 		semop(buflen_sem, &semoper, 1);
  
  		arts_debug("oggvorbis: decoder process exiting");
  		exit(0);
***************
*** 177,183 ****
  	if (time.seconds == -1)
  		time.seconds = 0; // Eek infinite loop time.
  	time.ms=0;
! 	//arts_debug("oggvorbis: time is now %d\n", time.seconds);
  	return time;
  }
  
--- 205,211 ----
  	if (time.seconds == -1)
  		time.seconds = 0; // Eek infinite loop time.
  	time.ms=0;
! 	//arts_debug("oggvorbis: time is now %d", time.seconds);
  	return time;
  }
  
***************
*** 210,215 ****
--- 238,244 ----
  	mState = posIdle;
  	union semun foo;
  
+ 	arts_debug("oggvorbis: halt");
  	if (child_pid) {
  		arts_debug("oggvorbis: killing decoder process");
  		foo.val = 2*BACKBUFSIZ;
***************
*** 222,227 ****
--- 251,265 ----
  	// mainly this is to ensure that the decoder wakes up to notice
  }
  
+ void oggPlayObject_impl::halt_on_decoderTerminated() {
+ 	if (child_pid && 
+ 	    waitpid(child_pid, NULL, WNOHANG)) { // child has already terminated!
+ 		child_pid = 0;
+ 		halt();
+ 	}
+ }
+ 
+ 
  void oggPlayObject_impl::seek(const class poTime &t)
  {
  	union semun foo;
***************
*** 249,255 ****
  
  void oggPlayObject_impl::calculateBlock(unsigned long samples)
  {
! 	int samplesAvailable = 0;
  
  	//arts_debug("oggvorbis: calculateBlock");
  
--- 287,293 ----
  
  void oggPlayObject_impl::calculateBlock(unsigned long samples)
  {
! 	unsigned long samplesWritten = 0;
  
  	//arts_debug("oggvorbis: calculateBlock");
  
***************
*** 257,306 ****
  		//arts_debug("oggvorbis: calculateBlock, %d(%d) of %d samples in buffer",
  		//shm_buf->buflen - bufpos, shm_buf->backbuflen,samples);
  
! 		struct sembuf bleh;
  
! 		bleh.sem_num = 0;
! 		bleh.sem_flg = IPC_NOWAIT;
! 		//arts_debug("oggvorbis: %d samples wanted", samplesAvailable);
! 		bleh.sem_op = -samples; // does the buffer have sufficient samples?
! 		if (semop(buflen_sem, &bleh, 1) == -1) {
! 			if (errno == EAGAIN) {
! 				union semun foo;
  				arts_debug("oggvorbis: buffer underrun");
! 				samplesAvailable = semctl(buflen_sem, 0, GETVAL, foo);
! 				// no samples AND no room is the decoder's way of signalling completion
! 				if (semctl(buflen_sem, 1, GETVAL, foo) == 0) {
! 					halt();
! 					samplesAvailable = 0;
  				}
  			} else {
! 				// something awful has happened
  				halt();
! 				samplesAvailable = 0;
  			}
- 		} else {
- 			samplesAvailable = samples; // number of samples we pushed from buffers
- 			// used to calculate the number we should zero out for an underrun
  		}
- 		bleh.sem_flg = 0; // back to normal now
- 
- 		//arts_debug("oggvorbis: %d samples available",samplesAvailable);
- 		for (int i = 0; i < samplesAvailable;
- 			++i, buf_pos = ((buf_pos + 1) % BACKBUFSIZ)) {
  
! 			left[i] = shm_buf->left[buf_pos];
! 			right[i] = shm_buf->right[buf_pos];
  		}
  
! 		bleh.sem_num = 1;
! 		bleh.sem_op = samplesAvailable;
! 		semop(buflen_sem, &bleh, 1); // mark the now-free space
  	}
  	// zero out any samples we didn't have enough to complete
! 	while(static_cast<unsigned long>(samplesAvailable) < samples) {
! 		left[samplesAvailable] = 0.0;
! 		right[samplesAvailable] = 0.0;
! 		samplesAvailable++;
  	}
  }
  
--- 295,378 ----
  		//arts_debug("oggvorbis: calculateBlock, %d(%d) of %d samples in buffer",
  		//shm_buf->buflen - bufpos, shm_buf->backbuflen,samples);
  
! 		int additional_reserve;
! 		if (shm_buf->rate == samplingRate) {
! 			additional_reserve = samples;
! 		} else {
! 			additional_reserve = samples*shm_buf->rate/samplingRate + 1 - reserved;
! 		}
  
! 		while (additional_reserve > 0) {
! 
! 			struct sembuf bleh;
! 
! 			bleh.sem_num = 0;
! 			bleh.sem_flg = IPC_NOWAIT;
! 			bleh.sem_op = -additional_reserve;
! 			if (semop(buflen_sem, &bleh, 1) == 0) { // likely
! 				// Success!
! 				reserved += additional_reserve;
! 				additional_reserve = 0;
! 			} else if (errno == EAGAIN) {
  				arts_debug("oggvorbis: buffer underrun");
! 
! 				// Check the number of samples available, this does not reserve them!
! 				union semun foo;
! 				int samplesAvailable = semctl(buflen_sem, 0, GETVAL, foo);
! 
! 				// No samples! Why? Has decoder terminated?
! 				if (samplesAvailable == 0) {
! 					halt_on_decoderTerminated();
  				}
+ 
+ 				additional_reserve = samplesAvailable;
  			} else {
! 				// something awful/unexpected has happened
  				halt();
! 				additional_reserve = 0;
  			}
  		}
  
! 		////////////////////////
! 		// use from reserved
! 		//
! 		int used = 0;
! 		while (samplesWritten < samples && used < reserved) {
! 
! 		    left[samplesWritten] = shm_buf->left[buf_pos];
! 		    right[samplesWritten] = shm_buf->right[buf_pos];
! 		    samplesWritten++;
! 
! 		    fract_bufpos += shm_buf->rate;
! 
! 		    if (fract_bufpos >= samplingRate) {
! 			fract_bufpos -= samplingRate;
! 			buf_pos = ((buf_pos + 1) % BACKBUFSIZ);
! 			used++;
! 		    }
  		}
  
! 		///////////////////////////////////////////////////////
! 		// finalize, avoid semop with sem_op==0 => will sleep
! 		//
! 		reserved -= used;
! 		artsdebug("reserved=%d, used=%d\n", reserved, used);
! 
! 		if (used > 0) {
! 			struct sembuf freesem;
! 			freesem.sem_num = 1;
! 			freesem.sem_op = +used;
! 			semop(buflen_sem, &freesem, 1); // mark the now-free space
! 		}
  	}
+ 
+ 	////////////////////////////////////////////////////////////
  	// zero out any samples we didn't have enough to complete
! 	//
! 	while(samplesWritten < samples) {
! 		left[samplesWritten] = 0.0;
! 		right[samplesWritten] = 0.0;
! 		samplesWritten++;
  	}
  }
  
***************
*** 310,312 ****
--- 382,390 ----
  }
  
  REGISTER_IMPLEMENTATION(oggPlayObject_impl);
+ 
+ 
+ 
+ 
+ 
+ 
Index: oggPlayObject_impl.h
===================================================================
RCS file: /home/kde/kdemultimedia/oggvorbis_artsplugin/oggPlayObject_impl.h,v
retrieving revision 1.2
diff -c -r1.2 oggPlayObject_impl.h
*** oggPlayObject_impl.h	2001/12/14 20:54:43	1.2
--- oggPlayObject_impl.h	2002/01/29 22:02:00
***************
*** 50,60 ****
--- 50,75 ----
  	struct buf_t{
  		float left[BACKBUFSIZ];
  		float right[BACKBUFSIZ];
+ 	        int   rate;
  	} *shm_buf;
  	int shm_id, child_pid;
  	int buflen_sem;
+ 
+  private:
+ 	int reserved;     // reserved, but unused input (shm_buf)
+ 	int fract_bufpos; // per samplingRate, initialized to 0
+ 
+ 	void halt_on_decoderTerminated();
  };
  
  };
  
  #endif
+ 
+ 
+ 
+ 
+ 
+ 
+ 
+ 


_______________________________________________
kde-multimedia mailing list
kde-multimedia@mail.kde.org
http://mail.kde.org/mailman/listinfo/kde-multimedia

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic