-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

The DAAP plugin currently operates in push mode; the most appropriate
mode for its type. However, GStreamer's push mode suffers from poor
seeking support in a number of plugins:
  - MPEG-4 demuxer will not seek in push mode.
  - AAC decoder will not seek in push mode.
  - OGG demuxer will not seek in push mode.

90% of my music collection is unseekable as a result. My initial
attempts to add support for push mode seeking to these plugins failed,
due in part to poor documentation and few examples. Pull mode receives a
lot more attention from plugin developers because it is the method most
media players use to access a library.

Thus this patch: make the DAAP plugin use pull mode instead of push
mode. Aside from some extra code to buffer streamed data, I see no
particular flaw in doing this. The pull mode plugin seeks correctly with
all of the elements listed above.

As this is a workaround rather than an architecturally correct solution,
this patch is open for comments. If seeking functionality is deemed
important enough to accept this slight hack, I will submit a bug report
with this patch attached for review.

Many thanks,

- --
Jay L. T. Cornwall
http://www.jcornwall.me.uk/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHCRXkdQczt3VeX8URAhW3AKC8CYE+r2GHsNyTXq4UziyOgq7MdgCffDHC
t781MYuKpCjgIjoHLKpTXZg=
=Gggm
-----END PGP SIGNATURE-----
--- ../rhythmbox-svn/plugins/daap/rb-daap-src.c 2007-09-02 15:33:38.000000000 
+0100
+++ plugins/daap/rb-daap-src.c  2007-10-07 18:10:13.000000000 +0100
@@ -39,8 +39,8 @@
 
 #include <glib/gi18n.h>
 #include <gst/gst.h>
+#include <gst/base/gstadapter.h>
 #include <gst/base/gstbasesrc.h>
-#include <gst/base/gstpushsrc.h>
 
 #include "rb-daap-source.h"
 #include "rb-daap-src.h"
@@ -66,7 +66,7 @@
 
 struct _RBDAAPSrc
 {
-       GstPushSrc parent;
+       GstBaseSrc parent;
 
        /* uri */
        gchar *daap_uri;
@@ -80,17 +80,20 @@
        gboolean chunked;
        gboolean first_chunk;
 
-       gint64 size;
+       gint64 bytes_left;
+       gint64 bytes_total;
 
        /* Seek stuff */
        gint64 curoffset;
        gint64 seek_bytes;
-       gboolean do_seek;
+
+       /* Buffers data for arbitrary length create() calls. */
+       GstAdapter *adapter;
 };
 
 struct _RBDAAPSrcClass
 {
-       GstPushSrcClass parent_class;
+       GstBaseSrcClass parent_class;
 };
 
 static GstStaticPadTemplate srctemplate = GST_STATIC_PAD_TEMPLATE ("src",
@@ -127,7 +130,7 @@
                        &urihandler_info);
 }
 
-GST_BOILERPLATE_FULL (RBDAAPSrc, rb_daap_src, GstElement, GST_TYPE_PUSH_SRC, 
_do_init);
+GST_BOILERPLATE_FULL (RBDAAPSrc, rb_daap_src, GstElement, GST_TYPE_BASE_SRC, 
_do_init);
 
 static void rb_daap_src_finalize (GObject *object);
 static void rb_daap_src_set_property (GObject *object,
@@ -143,8 +146,7 @@
 static gboolean rb_daap_src_stop (GstBaseSrc *bsrc);
 static gboolean rb_daap_src_is_seekable (GstBaseSrc *bsrc);
 static gboolean rb_daap_src_get_size (GstBaseSrc *src, guint64 *size);
-static gboolean rb_daap_src_do_seek (GstBaseSrc *src, GstSegment *segment);
-static GstFlowReturn rb_daap_src_create (GstPushSrc *psrc, GstBuffer **outbuf);
+static GstFlowReturn rb_daap_src_create (GstBaseSrc *bsrc, guint64 offset, 
guint size, GstBuffer **outbuf);
 
 void
 rb_daap_src_set_plugin (RBPlugin *plugin)
@@ -176,14 +178,12 @@
        GObjectClass *gobject_class;
        GstElementClass *gstelement_class;
        GstBaseSrcClass *gstbasesrc_class;
-       GstPushSrcClass *gstpushsrc_class;
 
        gobject_class = G_OBJECT_CLASS (klass);
        gstelement_class = GST_ELEMENT_CLASS (klass);
        gstbasesrc_class = (GstBaseSrcClass *) klass;
-       gstpushsrc_class = (GstPushSrcClass *) klass;
 
-       parent_class = g_type_class_ref (GST_TYPE_PUSH_SRC);
+       parent_class = g_type_class_ref (GST_TYPE_BASE_SRC);
 
        gobject_class->set_property = rb_daap_src_set_property;
        gobject_class->get_property = rb_daap_src_get_property;
@@ -200,9 +200,7 @@
        gstbasesrc_class->stop = GST_DEBUG_FUNCPTR (rb_daap_src_stop);
        gstbasesrc_class->is_seekable = GST_DEBUG_FUNCPTR 
(rb_daap_src_is_seekable);
        gstbasesrc_class->get_size = GST_DEBUG_FUNCPTR (rb_daap_src_get_size);
-       gstbasesrc_class->do_seek = GST_DEBUG_FUNCPTR (rb_daap_src_do_seek);
-
-       gstpushsrc_class->create = GST_DEBUG_FUNCPTR (rb_daap_src_create);
+       gstbasesrc_class->create = GST_DEBUG_FUNCPTR (rb_daap_src_create);
 }
 
 static void
@@ -212,6 +210,9 @@
        src->sock_fd = -1;
        src->curoffset = 0;
        src->bytes_per_read = 4096 * 2;
+       src->bytes_left = 0;
+       src->bytes_total = 0;
+       src->adapter = gst_adapter_new ();
 }
 
 static void
@@ -228,6 +229,8 @@
                src->sock_fd = -1;
        }
 
+       g_object_unref (src->adapter);
+
        G_OBJECT_CLASS (parent_class)->finalize (object);
 }
 
@@ -585,15 +588,20 @@
                                val = g_hash_table_lookup (header_table, 
"Content-Length");
                                if (val) {
                                        char *e;
-                                       src->size = strtoul ((char *)val->data, 
&e, 10);
+                                       src->bytes_left = strtoul ((char 
*)val->data, &e, 10);
                                        if (e == val->data) {
                                                GST_ELEMENT_ERROR (src, 
RESOURCE, OPEN_READ, (NULL),
                                                                   ("Couldn't 
read HTTP content length \"%s\"", val->data));
                                                ok = FALSE;
                                        }
+
+                                       /* In addition to the number of bytes 
remaining, record the total
+                                        * stream length - since we are 
operating in pull mode. */
+                                       src->bytes_total = src->bytes_left + 
src->seek_bytes;
                                } else {
                                        GST_DEBUG_OBJECT (src, "Response 
doesn't have a content length");
-                                       src->size = 0;
+                                       src->bytes_left = 0;
+                                       src->bytes_total = 0;
                                }
                        }
 
@@ -641,7 +649,8 @@
                src->curoffset = src->seek_bytes;
                if (src->chunked) {
                        src->first_chunk = TRUE;
-                       src->size = 0;
+                       src->bytes_left = 0;
+                       src->bytes_total = 0;
                }
                return TRUE;
        } else {
@@ -659,70 +668,87 @@
 }
 
 static GstFlowReturn
-rb_daap_src_create (GstPushSrc *psrc, GstBuffer **outbuf)
+rb_daap_src_create (GstBaseSrc *bsrc, guint64 offset, guint size, GstBuffer 
**outbuf)
 {
-       RBDAAPSrc *src;
-       size_t readsize;
-       GstBuffer *buf = NULL;
+       RBDAAPSrc *src = RB_DAAP_SRC (bsrc);
 
-       src = RB_DAAP_SRC (psrc);
-       if (src->do_seek) {
-               if (src->sock_fd != -1) {
-                       close (src->sock_fd);
-                       src->sock_fd = -1;
-               }
-               if (!rb_daap_src_start (GST_BASE_SRC (src)))
-                       return GST_FLOW_ERROR;
-               src->do_seek = FALSE;
-       }
+       /* Restart the stream at the requested offset if it differs from the
+        * current offset adjusted for buffered data. */
+       if (offset != src->curoffset - gst_adapter_available (src->adapter)) {
+               src->seek_bytes = offset;
 
-       /* get a new chunk, if we need one */
-       if (src->chunked && src->size == 0) {
-               if (!rb_daap_src_read_chunk_size (src, src->first_chunk, 
&src->size)) {
+               if (!rb_daap_src_start (GST_BASE_SRC (src))) {
                        return GST_FLOW_ERROR;
-               } else if (src->size == 0) {
-                       /* EOS */
-                       return GST_FLOW_UNEXPECTED;
                }
-               src->first_chunk = FALSE;
+
+               /* Flush any buffers accumulated before this seek. */
+               gst_adapter_clear (src->adapter);
        }
 
-       readsize = src->bytes_per_read;
-       if (src->chunked && readsize > src->size)
-               readsize = src->size;
+       /* Fill the adapter until it has enough data. */
+       while (gst_adapter_available (src->adapter) < size) {
+               GstBuffer *buf;
+               size_t bytes_to_read, bytes_read;
+
+               /* Get a new chunk if we need one. */
+               if (src->chunked && src->bytes_left == 0) {
+                       if (!rb_daap_src_read_chunk_size (src, 
src->first_chunk, &src->bytes_left)) {
+                               return GST_FLOW_ERROR;
+                       } else if (src->bytes_left == 0) {
+                               /* EOS */
+                               return GST_FLOW_UNEXPECTED;
+                       }
+                       src->first_chunk = FALSE;
+               }
 
-       buf = gst_buffer_new_and_alloc (readsize);
+               /* Read pre-sized data into a local buffer. */
+               bytes_to_read = src->bytes_per_read;
+               if (src->chunked && bytes_to_read > src->bytes_left) {
+                       bytes_to_read = src->bytes_left;
+               }
 
-       GST_LOG_OBJECT (src, "Reading %d bytes", readsize);
-       readsize = rb_daap_src_read (src, GST_BUFFER_DATA (buf), readsize);
-       if (readsize < 0) {
-               GST_ELEMENT_ERROR (src, RESOURCE, READ, (NULL), 
GST_ERROR_SYSTEM);
-               gst_buffer_unref (buf);
-               return GST_FLOW_ERROR;
-       }
+               buf = gst_buffer_new_and_alloc (bytes_to_read);
 
-       if (readsize == 0) {
-               GST_DEBUG ("blocking read returns 0, EOS");
-               gst_buffer_unref (buf);
-               return GST_FLOW_UNEXPECTED;
+               GST_LOG_OBJECT (src, "Reading %d bytes", bytes_to_read);
+               bytes_read = rb_daap_src_read (src, GST_BUFFER_DATA (buf), 
bytes_to_read);
+               if (bytes_read < 0) {
+                       GST_ELEMENT_ERROR (src, RESOURCE, READ, (NULL), 
GST_ERROR_SYSTEM);
+                       gst_buffer_unref (buf);
+                       return GST_FLOW_ERROR;
+               }
+
+               if (bytes_read == 0) {
+                       GST_DEBUG ("blocking read returns 0, EOS");
+                       gst_buffer_unref (buf);
+                       return GST_FLOW_UNEXPECTED;
+               }
+
+               /* Update stream position. */
+               if (src->chunked) {
+                       src->bytes_left -= bytes_read;
+               }
+               src->curoffset += bytes_read;
+
+               /* Pass buffer to the adapter. */
+               gst_adapter_push (src->adapter, buf);
        }
 
-       if (src->chunked)
-               src->size -= readsize;
+       /* Take the requested number of bytes from the adapter. */
+       *outbuf = gst_adapter_take_buffer (src->adapter, size);
 
-       GST_BUFFER_OFFSET (buf) = src->curoffset;
-       GST_BUFFER_SIZE (buf) = readsize;
-       GST_BUFFER_TIMESTAMP (buf) = GST_CLOCK_TIME_NONE;
-       src->curoffset += readsize;
+       /* Annotate buffer with stream information. */
+       GST_BUFFER_OFFSET (*outbuf) = offset;
+       GST_BUFFER_SIZE (*outbuf) = size;
+       GST_BUFFER_TIMESTAMP (*outbuf) = GST_CLOCK_TIME_NONE;
 
        GST_LOG_OBJECT (src,
                        "Returning buffer from _get of size %d, ts %"
                        GST_TIME_FORMAT ", dur %" GST_TIME_FORMAT
                        ", offset %" G_GINT64_FORMAT ", offset_end %" 
G_GINT64_FORMAT,
-                       GST_BUFFER_SIZE (buf), GST_TIME_ARGS 
(GST_BUFFER_TIMESTAMP (buf)),
-                       GST_TIME_ARGS (GST_BUFFER_DURATION (buf)),
-                       GST_BUFFER_OFFSET (buf), GST_BUFFER_OFFSET_END (buf));
-       *outbuf = buf;
+                       GST_BUFFER_SIZE (*outbuf), GST_TIME_ARGS 
(GST_BUFFER_TIMESTAMP (*outbuf)),
+                       GST_TIME_ARGS (GST_BUFFER_DURATION (*outbuf)),
+                       GST_BUFFER_OFFSET (*outbuf), GST_BUFFER_OFFSET_END 
(*outbuf));
+
        return GST_FLOW_OK;
 }
 
@@ -733,24 +759,12 @@
 }
 
 gboolean
-rb_daap_src_do_seek (GstBaseSrc *bsrc, GstSegment *segment)
-{
-       RBDAAPSrc *src = RB_DAAP_SRC (bsrc);
-       if (segment->format == GST_FORMAT_BYTES) {
-               src->do_seek = TRUE;
-               src->seek_bytes = segment->start;
-               return TRUE;
-       } else {
-               return FALSE;
-       }
-}
-
-gboolean
 rb_daap_src_get_size (GstBaseSrc *bsrc, guint64 *size)
 {
        RBDAAPSrc *src = RB_DAAP_SRC (bsrc);
-       if (src->chunked == FALSE && src->size > 0) {
-               *size = src->size;
+
+       if (src->chunked == FALSE && src->bytes_total > 0) {
+               *size = src->bytes_total;
                return TRUE;
        }
        return FALSE;

Attachment: rhythmbox-daap-pull.patch.sig
Description: Binary data

_______________________________________________
rhythmbox-devel mailing list
[email protected]
http://mail.gnome.org/mailman/listinfo/rhythmbox-devel

Reply via email to