Various fixes for unseekable, unmmapable, and non-normal files, so that fallback...

Original commit message from CVS:
Various fixes for unseekable, unmmapable, and non-normal files, so that
fallback to read() rather than mmap() works.

Allow newsegment events with start == end, so that cases where that's
correct work (e.g. filesrc on a zero-size file).

diff --git a/ChangeLog b/ChangeLog
index 8696757..21faad4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2005-09-08  Michael Smith <msmith@fluendo.com>
+
+       * gst/elements/gstfilesrc.c: (gst_file_src_map_region),
+       (gst_file_src_map_small_region), (gst_file_src_create_mmap),
+       (gst_file_src_is_seekable), (gst_file_src_get_size),
+       (gst_file_src_start):
+       * gst/elements/gstfilesrc.h:
+         Various fixes for unseekable, unmmapable, and non-normal files, so
+         that fallback to read() rather than mmap() works.
+       * gst/gstevent.c: (gst_event_new_newsegment):
+         Allow newsegment events with segment_start == segment_end, as will
+         correctly happen if you use filesrc on a zero-size file, for
+         example.
+
 2005-09-07  Jan Schmidt  <thaytan@mad.scientist.com>
 
        * gst/gstplugin.c: (gst_plugin_load_file):

diff --git a/gst/elements/gstfilesrc.c b/gst/elements/gstfilesrc.c
index 8c6785d..781817e 100644 (file)
--- a/gst/elements/gstfilesrc.c
+++ b/gst/elements/gstfilesrc.c
@@ -473,7 +473,8 @@ gst_mmap_buffer_finalize (GstMmapBuffer * mmap_buffer)
 }
 
 static GstBuffer *
-gst_file_src_map_region (GstFileSrc * src, off_t offset, size_t size)
+gst_file_src_map_region (GstFileSrc * src, off_t offset, size_t size,
+    gboolean testonly)
 {
   GstBuffer *buf;
   void *mmapregion;
@@ -515,9 +516,11 @@ gst_file_src_map_region (GstFileSrc * src, off_t offset, size_t size)
   /* ERROR */
 mmap_failed:
   {
-    GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ, (NULL),
-        ("mmap (0x%08lx, %d, 0x%llx) failed: %s",
-            (gulong) size, src->fd, offset, strerror (errno)));
+    if (!testonly) {
+      GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ, (NULL),
+          ("mmap (0x%08lx, %d, 0x%llx) failed: %s",
+              (gulong) size, src->fd, offset, strerror (errno)));
+    }
     return NULL;
   }
 }
@@ -550,7 +553,7 @@ gst_file_src_map_small_region (GstFileSrc * src, off_t offset, size_t size)
         "not on page boundaries, resizing to map to %llu+%d",
         (unsigned long long) mapbase, (gint) mapsize);
 
-    map = gst_file_src_map_region (src, mapbase, mapsize);
+    map = gst_file_src_map_region (src, mapbase, mapsize, FALSE);
     if (map == NULL)
       return NULL;
 
@@ -559,7 +562,7 @@ gst_file_src_map_small_region (GstFileSrc * src, off_t offset, size_t size)
 
     gst_buffer_unref (map);
   } else {
-    ret = gst_file_src_map_region (src, offset, size);
+    ret = gst_file_src_map_region (src, offset, size, FALSE);
   }
 
   return ret;
@@ -660,7 +663,7 @@ gst_file_src_create_mmap (GstFileSrc * src, guint64 offset, guint length,
         mapsize <<= 1;
       }
       /* create a new one */
-      src->mapbuf = gst_file_src_map_region (src, nextmap, mapsize);
+      src->mapbuf = gst_file_src_map_region (src, nextmap, mapsize, FALSE);
       if (src->mapbuf == NULL)
         goto could_not_mmap;
 
@@ -792,9 +795,11 @@ gst_file_src_create (GstBaseSrc * basesrc, guint64 offset, guint length,
 }
 
 static gboolean
-gst_file_src_is_seekable (GstBaseSrc * src)
+gst_file_src_is_seekable (GstBaseSrc * basesrc)
 {
-  return TRUE;
+  GstFileSrc *src = GST_FILE_SRC (basesrc);
+
+  return src->seekable;
 }
 
 static gboolean
@@ -805,6 +810,12 @@ gst_file_src_get_size (GstBaseSrc * basesrc, guint64 * size)
 
   src = GST_FILE_SRC (basesrc);
 
+  if (!src->seekable) {
+    /* If it isn't seekable, we won't know the length (but fstat will still
+     * succeed, and wrongly say our length is zero. */
+    return FALSE;
+  }
+
   if (fstat (src->fd, &stat_results) < 0)
     goto could_not_stat;
 
@@ -856,12 +867,30 @@ gst_file_src_start (GstBaseSrc * basesrc)
 #ifdef HAVE_MMAP
   /* FIXME: maybe we should only try to mmap if it's a regular file */
   /* allocate the first mmap'd region if it's a regular file ? */
-  src->mapbuf = gst_file_src_map_region (src, 0, src->mapsize);
+  src->mapbuf = gst_file_src_map_region (src, 0, src->mapsize, TRUE);
   if (src->mapbuf != NULL) {
     GST_DEBUG_OBJECT (src, "using mmap for file");
     src->using_mmap = TRUE;
-  }
+    src->seekable = TRUE;
+  } else
 #endif
+  {
+    /* If not in mmap mode, we need to check if the underlying file is 
+     * seekable. */
+    off_t res = lseek (src->fd, 0, SEEK_CUR);
+
+    if (res < 0) {
+      GST_LOG_OBJECT (src, "disabling seeking, not in mmap mode and lseek "
+          "failed: %s", strerror (errno));
+      src->seekable = FALSE;
+    } else {
+      src->seekable = TRUE;
+    }
+  }
+
+  /* We can only really do seeking on regular files - for other file types, we
+   * don't know their length, so seeking isn't useful/meaningful */
+  src->seekable = src->seekable && src->is_regular;
 
   return TRUE;
 
@@ -881,8 +910,8 @@ open_failed:
         break;
       default:
         GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ,
-            (_("Could not open file \"%s\" for reading."), src->filename),
-            GST_ERROR_SYSTEM);
+            (_("Could not open file \"%s\" for reading: %s."), src->filename,
+                strerror (errno)), GST_ERROR_SYSTEM);
         break;
     }
     return FALSE;

diff --git a/gst/elements/gstfilesrc.h b/gst/elements/gstfilesrc.h
index 3b43704..307f3c1 100644 (file)
--- a/gst/elements/gstfilesrc.h
+++ b/gst/elements/gstfilesrc.h
@@ -57,6 +57,7 @@ struct _GstFileSrc {
 
   gboolean touch;                      /* whether to touch every page */
   gboolean using_mmap;                  /* whether we opened it with mmap */
+  gboolean seekable;                    /* whether the file is seekable */
   gboolean is_regular;                  /* whether it's a (symlink to a)
                                            regular file */
   GstBuffer *mapbuf;

diff --git a/gst/gstevent.c b/gst/gstevent.c
index 00466b2..f3269f9 100644 (file)
--- a/gst/gstevent.c
+++ b/gst/gstevent.c
@@ -323,7 +323,7 @@ gst_event_new_newsegment (gdouble rate, GstFormat format,
     g_return_val_if_fail (start_value != -1, NULL);
 
   if (stop_value != -1)
-    g_return_val_if_fail (start_value < stop_value, NULL);
+    g_return_val_if_fail (start_value <= stop_value, NULL);
 
   return gst_event_new_custom (GST_EVENT_NEWSEGMENT,
       gst_structure_new ("GstEventNewsegment", "rate", G_TYPE_DOUBLE, rate,

diff --git a/plugins/elements/gstfilesrc.c b/plugins/elements/gstfilesrc.c
index 8c6785d..781817e 100644 (file)
--- a/plugins/elements/gstfilesrc.c
+++ b/plugins/elements/gstfilesrc.c
@@ -473,7 +473,8 @@ gst_mmap_buffer_finalize (GstMmapBuffer * mmap_buffer)
 }
 
 static GstBuffer *
-gst_file_src_map_region (GstFileSrc * src, off_t offset, size_t size)
+gst_file_src_map_region (GstFileSrc * src, off_t offset, size_t size,
+    gboolean testonly)
 {
   GstBuffer *buf;
   void *mmapregion;
@@ -515,9 +516,11 @@ gst_file_src_map_region (GstFileSrc * src, off_t offset, size_t size)
   /* ERROR */
 mmap_failed:
   {
-    GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ, (NULL),
-        ("mmap (0x%08lx, %d, 0x%llx) failed: %s",
-            (gulong) size, src->fd, offset, strerror (errno)));
+    if (!testonly) {
+      GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ, (NULL),
+          ("mmap (0x%08lx, %d, 0x%llx) failed: %s",
+              (gulong) size, src->fd, offset, strerror (errno)));
+    }
     return NULL;
   }
 }
@@ -550,7 +553,7 @@ gst_file_src_map_small_region (GstFileSrc * src, off_t offset, size_t size)
         "not on page boundaries, resizing to map to %llu+%d",
         (unsigned long long) mapbase, (gint) mapsize);
 
-    map = gst_file_src_map_region (src, mapbase, mapsize);
+    map = gst_file_src_map_region (src, mapbase, mapsize, FALSE);
     if (map == NULL)
       return NULL;
 
@@ -559,7 +562,7 @@ gst_file_src_map_small_region (GstFileSrc * src, off_t offset, size_t size)
 
     gst_buffer_unref (map);
   } else {
-    ret = gst_file_src_map_region (src, offset, size);
+    ret = gst_file_src_map_region (src, offset, size, FALSE);
   }
 
   return ret;
@@ -660,7 +663,7 @@ gst_file_src_create_mmap (GstFileSrc * src, guint64 offset, guint length,
         mapsize <<= 1;
       }
       /* create a new one */
-      src->mapbuf = gst_file_src_map_region (src, nextmap, mapsize);
+      src->mapbuf = gst_file_src_map_region (src, nextmap, mapsize, FALSE);
       if (src->mapbuf == NULL)
         goto could_not_mmap;
 
@@ -792,9 +795,11 @@ gst_file_src_create (GstBaseSrc * basesrc, guint64 offset, guint length,
 }
 
 static gboolean
-gst_file_src_is_seekable (GstBaseSrc * src)
+gst_file_src_is_seekable (GstBaseSrc * basesrc)
 {
-  return TRUE;
+  GstFileSrc *src = GST_FILE_SRC (basesrc);
+
+  return src->seekable;
 }
 
 static gboolean
@@ -805,6 +810,12 @@ gst_file_src_get_size (GstBaseSrc * basesrc, guint64 * size)
 
   src = GST_FILE_SRC (basesrc);
 
+  if (!src->seekable) {
+    /* If it isn't seekable, we won't know the length (but fstat will still
+     * succeed, and wrongly say our length is zero. */
+    return FALSE;
+  }
+
   if (fstat (src->fd, &stat_results) < 0)
     goto could_not_stat;
 
@@ -856,12 +867,30 @@ gst_file_src_start (GstBaseSrc * basesrc)
 #ifdef HAVE_MMAP
   /* FIXME: maybe we should only try to mmap if it's a regular file */
   /* allocate the first mmap'd region if it's a regular file ? */
-  src->mapbuf = gst_file_src_map_region (src, 0, src->mapsize);
+  src->mapbuf = gst_file_src_map_region (src, 0, src->mapsize, TRUE);
   if (src->mapbuf != NULL) {
     GST_DEBUG_OBJECT (src, "using mmap for file");
     src->using_mmap = TRUE;
-  }
+    src->seekable = TRUE;
+  } else
 #endif
+  {
+    /* If not in mmap mode, we need to check if the underlying file is 
+     * seekable. */
+    off_t res = lseek (src->fd, 0, SEEK_CUR);
+
+    if (res < 0) {
+      GST_LOG_OBJECT (src, "disabling seeking, not in mmap mode and lseek "
+          "failed: %s", strerror (errno));
+      src->seekable = FALSE;
+    } else {
+      src->seekable = TRUE;
+    }
+  }
+
+  /* We can only really do seeking on regular files - for other file types, we
+   * don't know their length, so seeking isn't useful/meaningful */
+  src->seekable = src->seekable && src->is_regular;
 
   return TRUE;
 
@@ -881,8 +910,8 @@ open_failed:
         break;
       default:
         GST_ELEMENT_ERROR (src, RESOURCE, OPEN_READ,
-            (_("Could not open file \"%s\" for reading."), src->filename),
-            GST_ERROR_SYSTEM);
+            (_("Could not open file \"%s\" for reading: %s."), src->filename,
+                strerror (errno)), GST_ERROR_SYSTEM);
         break;
     }
     return FALSE;

diff --git a/plugins/elements/gstfilesrc.h b/plugins/elements/gstfilesrc.h
index 3b43704..307f3c1 100644 (file)
--- a/plugins/elements/gstfilesrc.h
+++ b/plugins/elements/gstfilesrc.h
@@ -57,6 +57,7 @@ struct _GstFileSrc {
 
   gboolean touch;                      /* whether to touch every page */
   gboolean using_mmap;                  /* whether we opened it with mmap */
+  gboolean seekable;                    /* whether the file is seekable */
   gboolean is_regular;                  /* whether it's a (symlink to a)
                                            regular file */
   GstBuffer *mapbuf;