[MPlayer-dev-eng] [PATCH] bmovl bug report + fix

[Detlev Droege]

Hi folks,

I needed to put some overlay in the output (of mencoder, to be exact)
and using ... -vop bmovl=0:0:NameOfPipe ... showed to be a nice tool
to do so.
However, when providing the image using the RGBA32 command,
any subsequent commands were not recognized. It turned out, that
the image was not entirely read from the pipe providing the data.
Using debug level 2 (-v -v) revealed, that only 36899 or so  bytes were read
allthough it should have been 38016 (RGBA32 paramters provided
correct values, width = 198, height = 48, 4 bytes per pixel gives 38016).

So I looked at the source code (MPlayer-1.0pre2/libmpcodecs/vf_bmovl.c)
and it showed up, that the data was not completely delivered by
a single call of read() (approx. line 270). 
In fact, it is legal behaviour of the read() call not to provide all data 
at once, especially when reading from pipes (and some other special devices).
See "man 2 read" for details.

I fixed the code to perform multiple reads if necessary and from then on
mencoder worked as expected.
(I had the same problem with another program some years ago. Fortunately, 
I remembered that unexpected (but documented) behaviour of read().)

I provide a unified diff below. 

The patch is relative to MPlayer-1.0pre2 but applies to the current CVS
version as well (I inspected it via the web interface). It also should
work for e.g. MPlayer-0.92 (though with a little offset in the line numbers).

System information:
  SuSE Linux 8.2, Kernel 2.4.21, gcc 3.3

Anyhow, mplayer/mencoder is really great. Congratulations to
all you guys who have put their effort into this project.
  
	Happy mplaying
		Detlev
PS:
  I posted this to mplayer-users before but then was told, that it 
  belongs here.
================================================
--- vf_bmovl.c_orig	2003-10-05 16:17:29.000000000 +0200
+++ vf_bmovl.c	2003-11-06 10:43:20.000000000 +0100
@@ -199,6 +199,7 @@
 static int
 put_image(struct vf_instance_s* vf, mp_image_t* mpi){
 	int buf_x=0, buf_y=0, buf_pos=0;
+	int have, got, want;
 	int xpos=0, ypos=0, pos=0;
 	unsigned char red=0, green=0, blue=0;
 	int  alpha;
@@ -267,7 +268,32 @@
 			    	mp_msg(MSGT_VFILTER, MSGL_WARN, "\nvf_bmovl: Couldn't allocate temporary buffer! Skipping...\n\n");
 					return vf_next_put_image(vf, dmpi);
 			    }
-			    mp_msg(MSGT_VFILTER, MSGL_DBG2, "Got %d bytes...\n", read( vf->priv->stream_fd, buffer, (imgw*imgh*pxsz) ) );
+  /* we need to take care of the fact that read() might not deliver all bytes we wanted in one call.
+     This is correct behavior and might especially occur when using pipes. 'man 2 read' says:
+     RETURN VALUE
+       On success, the number of bytes read is returned (zero indicates end of  file),  and  the  file  position  is
+       advanced  by  this  number.  It is not an error if this number is smaller than the number of bytes requested;
+       this may happen for example because fewer bytes are actually available right now (maybe because we were close
+       to end-of-file, or because we are reading from a pipe, or from a terminal), or because read() was interrupted
+       by a signal.
+     So lets try it until we have all we want -- or EOF is reached.
+     fixed by D. Droege, droege at uni-koblenz.de, 2003-11-05
+   */
+			    want = (imgw*imgh*pxsz);
+			    have = 0;
+			    while (have < want) {
+				got = read( vf->priv->stream_fd, buffer+have, want-have );
+				if (got == 0) {
+			    	    mp_msg(MSGT_VFILTER, MSGL_WARN, "\nvf_bmovl: premature EOF...\n\n");
+				    break;
+				}
+				if (got < 0) {
+			    	    mp_msg(MSGT_VFILTER, MSGL_WARN, "\nvf_bmovl: read error: %s\n\n", strerror(errno));
+				    break;
+				}
+				have += got;
+			    }
+			    mp_msg(MSGT_VFILTER, MSGL_DBG2, "Got %d bytes... (wanted %d)\n", have, want );
 
 				if(clear) {
 					memset( vf->priv->bitmap.y,   0, vf->priv->w*vf->priv->h );
================================================
---
Detlev Droege, Uni Koblenz, FB Informatik,P.O.Box 201602,56016 Koblenz,Germany
    Fon: +49 261 287-2769          |  Email: droege@  
    Fax: +49 261 287-100-2769      |                uni-koblenz.de