[linux-dvb] Re: Bug in ring buffer implementation

[Oliver Endriss <o.endriss@gmx.de>]

On Saturday 08 February 2003 04:10, Oliver Endriss wrote:
> while reading through the driver sources, I found that the free byte
> count of the ring buffers is not calculated correctly.

The following patch (against dvb head 2003-02-08) fixes the following 
ring buffer issues in av7110.c:

- ring_buffer_free():
  now returns number of free bytes (not free bytes +1)

- ring_buffer_write():
  fixed case todo>free, which resulted in writing one byte more than
  available (because 'free' was 1 byte off)

- ci_ll_reset():
  need >= 16 bytes if both slots are being reset at the same time


--- av7110.c.orig	Sat Feb  8 02:42:01 2003
+++ av7110.c	Sun Feb  9 18:53:23 2003
@@ -571,7 +571,7 @@
         free=rbuf->pread - rbuf->pwrite;
         if (free<=0)
                 free+=rbuf->size;
-        return free;
+        return free-1;
 }
 
 static inline 
@@ -605,11 +605,11 @@
         int free, split;
     
         while (todo > 0) {
-                if (ring_buffer_free(rbuf)<=2048) {
+                if (ring_buffer_free(rbuf)<2048) {
                         if (nonblock)
                                 return count-todo;
                         if (wait_event_interruptible(rbuf->queue,
-                                                     (ring_buffer_free(rbuf)>2048)))
+                                                     (ring_buffer_free(rbuf)>=2048)))
                         	return count-todo;
                 }   
                 dprintk ("function: %s pread=%08x pwrite=%08x\n", __FUNCTION__,
@@ -621,7 +621,7 @@
                         free+=rbuf->size;
                         split-=rbuf->pwrite;
                 }
-                if (free > todo)
+                if (--free > todo)
                         free = todo;
                         
                 if (split < free) {
@@ -3693,7 +3693,7 @@
 {
 	int i;
 
-        if (ring_buffer_free(cibuf)<8)
+        if (ring_buffer_free(cibuf)<16)
                 return -EBUSY;
 	for (i=0; i<2; i++) 
 		if (slots&(1<<i)) {


BTW, why does ring_buffer_write() wait for 2048 bytes buffer space?
- example: count=todo=100, free=200
  In this case there is no need to wait at all, why not use
	  if (ring_buffer_free(rbuf) < min(todo,2048)) {
  
- Is this necessary at all? The dvb_play() routines check for 20*1024
  free space before the ipack routines are being called.
  Or did I miss something?

Oliver



-- 
Info:
To unsubscribe send a mail to listar@linuxtv.org with "unsubscribe linux-dvb" as subject.