Changeset 1927 for pjproject/trunk

Timestamp:

Apr 14, 2008 2:51:05 AM (17 years ago)

Author:

bennylp

Message:

Ticket #526: Pjsua crash after wav player destroyed inside the eof callback (thanks Tanguy Floc'h)

File:

• pjproject/trunk/pjmedia/src/pjmedia/wav_player.c (modified) (6 diffs)

pjproject/trunk/pjmedia/src/pjmedia/wav_player.c

@@ -61 +61 @@
     char            *buf;
     char            *readpos;
+    char            *eofpos;
 
     pj_off_t         fsize;
@@ -107 +108 @@
     pj_status_t status;
 
-    /* Can't read file if EOF and loop flag is disabled */
-    if (fport->eof)
-        return PJ_EEOF;
-
+    fport->eofpos = NULL;
+    
     while (size_left > 0) {
 
@@ -132 +131 @@
          */
         if (size < (pj_ssize_t)size_to_read) {
-            /* Call callback, if any. */
-            if (fport->cb) {
-                PJ_LOG(5,(THIS_FILE, 
-                          "File port %.*s EOF, calling callback",
-                          (int)fport->base.info.name.slen,
-                          fport->base.info.name.ptr));
-
-                fport->eof = PJ_TRUE;
-                status=(*fport->cb)(&fport->base,fport->base.port_data.pdata);
-                if (status != PJ_SUCCESS) {
-                    /* This will crash if file port is destroyed in the 
-                     * callback, that's why we set the eof flag before
-                     * calling the callback:
-                     fport->eof = PJ_TRUE;
-                    */
-                    return status;
-                }
-                
-                fport->eof = PJ_FALSE;
+            fport->eof = PJ_TRUE;
+            fport->eofpos = fport->buf + fport->bufsize - size_left;
+            
+            if (fport->options & PJMEDIA_FILE_NO_LOOP) {
+                /* Zero remaining buffer */
+                pj_bzero(fport->eofpos, size_left);
             }
 
-            if (fport->options & PJMEDIA_FILE_NO_LOOP) {
-                PJ_LOG(5,(THIS_FILE, "File port %.*s EOF, stopping..",
-                          (int)fport->base.info.name.slen,
-                          fport->base.info.name.ptr));
-                /* Zero remaining buffer */
-                pj_bzero(fport->buf+size, size_left);
-                /* Mark port as EOF */
-                fport->eof = PJ_TRUE;
-                /* Must return PJ_SUCCESS, otherwise this buffer 
-                 * is not read */
-                return PJ_SUCCESS;
-            } else {
-                PJ_LOG(5,(THIS_FILE, "File port %.*s EOF, rewinding..",
-                          (int)fport->base.info.name.slen,
-                          fport->base.info.name.ptr));
-                fport->fpos = fport->start_data;
-                pj_file_setpos( fport->fd, fport->fpos, PJ_SEEK_SET);
-            }
+            /* Rewind file */
+            fport->fpos = fport->start_data;
+            pj_file_setpos( fport->fd, fport->fpos, PJ_SEEK_SET);
         }
     }
@@ -522 +493 @@
 
     pj_assert(fport->base.info.signature == SIGNATURE);
+    pj_assert(frame->size <= fport->bufsize);
+
+    /* EOF is set and readpos already passed the eofpos */
+    if (fport->eof && fport->readpos >= fport->eofpos) {
+        pj_status_t status = PJ_SUCCESS;
+
+        /* Call callback, if any */
+        if (fport->cb)
+            status = (*fport->cb)(this_port, fport->base.port_data.pdata);
+
+        /* If callback returns non PJ_SUCCESS or 'no loop' is specified,
+         * return immediately (and don't try to access player port since
+         * it might have been destroyed by the callback).
+         */
+        if ((status != PJ_SUCCESS) || (fport->options & PJMEDIA_FILE_NO_LOOP)) {
+            PJ_LOG(5,(THIS_FILE, "File port %.*s EOF, stopping..",
+                      (int)fport->base.info.name.slen,
+                      fport->base.info.name.ptr));
+
+            frame->type = PJMEDIA_FRAME_TYPE_NONE;
+            frame->size = 0;
+            return PJ_EEOF;
+        }
+        
+        PJ_LOG(5,(THIS_FILE, "File port %.*s EOF, rewinding..",
+                  (int)fport->base.info.name.slen,
+                  fport->base.info.name.ptr));
+        
+        fport->eof = PJ_FALSE;
+    }
 
     //frame_size = fport->base.info.bytes_per_frame;
@@ -532 +533 @@
     frame->timestamp.u64 = 0;
 
-    if (fport->readpos + frame_size <= fport->buf + fport->bufsize) {
-
+    if ((fport->readpos + frame_size) <= (fport->buf + fport->bufsize))
+    {
         /* Read contiguous buffer. */
         pj_memcpy(frame->buf, fport->readpos, frame_size);
@@ -559 +560 @@
         pj_memcpy(frame->buf, fport->readpos, endread);
 
+        /* End Of Buffer and EOF and NO LOOP */
+        if (fport->eof && (fport->options & PJMEDIA_FILE_NO_LOOP)) {
+            fport->readpos += endread;
+            pj_bzero((char*)frame->buf + endread, frame_size - endread);
+            return PJ_SUCCESS;
+        }
+
         /* Second stage: fill up buffer, and read from the start of buffer. */
         status = fill_buffer(fport);
         if (status != PJ_SUCCESS) {
-            /* If we don't get anything, return NONE frame. Otherwise
-             * return AUDIO frame since we have partial audio.
-             */
-            if (endread == 0) {
-                frame->type = PJMEDIA_FRAME_TYPE_NONE;
-            } else {
-                pj_bzero(((char*)frame->buf)+endread, frame_size-endread);
-            }
+            frame->type = PJMEDIA_FRAME_TYPE_NONE;
+            frame->size = 0;
             fport->readpos = fport->buf + fport->bufsize;
-            return (endread? PJ_SUCCESS : status);
+            return status;
         }