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

List:       kde-multimedia
Subject:    [PATCH][RFC] bugfixes (races) + handling of ogg rc3
From:       Roger Larsson <roger.larsson () norran ! net>
Date:       2002-02-02 0:18:18
[Download RAW message or body]

Hi again,

Found a race in my code...
(the producer should not sleep on the consumer, since halt waits on producer 
and might be called by consumer...)

I have tested it with ogg rc3 - no race problems that I have seen...

Audio quality for non 44100 Hz streams are not good, but
better than nothing... (anyone that can help me on this one?)

Hmm.. the part that reads comments should probably be moved to
kfile...

/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/02/01 23:36:23
***************
*** 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,162 ****
  			//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,184 ----
***************
*** 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;
  }
  
--- 199,205 ----
  	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 ****
--- 232,238 ----
  	mState = posIdle;
  	union semun foo;
  
+ 	arts_debug("oggvorbis: halt");
  	if (child_pid) {
  		arts_debug("oggvorbis: killing decoder process");
  		foo.val = 2*BACKBUFSIZ;
***************
*** 222,227 ****
--- 245,259 ----
  	// 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,306 ****
  
  void oggPlayObject_impl::calculateBlock(unsigned long samples)
  {
! 	int samplesAvailable = 0;
  
  	//arts_debug("oggvorbis: calculateBlock");
  
  	if (mState == posPlaying) {
  		//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++;
  	}
  }
  
--- 281,372 ----
  
  void oggPlayObject_impl::calculateBlock(unsigned long samples)
  {
! 	unsigned long samplesWritten = 0;
  
  	//arts_debug("oggvorbis: calculateBlock");
  
  	if (mState == posPlaying) {
  		//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 ****
--- 376,384 ----
  }
  
  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/02/01 23:36:23
***************
*** 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