Re: [PATCH]: Add large files support.

[Jean-Baptiste Dubois <jean-baptiste dubois parrot com>]

Hi,

On 10/07/2013 11:13 AM, Jens Georg wrote:

Hi,

thanks for this patch, makes completely sense :) For the future, please
file patches in bugzilla because we have a nice review infrastructure
there :)

bug created: https://bugzilla.gnome.org/show_bug.cgi?id=709551

One question though: Why are you using the Posix stuff and not the GLib
file functions?

There is no specific reason except I'm not an guru of glib file api...

The simple engine was more meant as a demo implementation. If you want
to use it in production, please note that there is a race condition in
the memory hand-over from streaming to the main thread where sometimes
the memory is freed early.

Ok, thank you for your help.

Hi Rygel Members,

I've tested Rygel media server with success, thank you for this project.
I would like to submit a patch regarding large files support in rygel 
media server.
The major part of this patch is done inside rygel-simple-data-source 
where we try to mmap a large file in the process virtual address space 
(4GB).
As we can't mmap file > 4GB, this patch replaces mmap with simple read 
syscall.

 From 81a2fbbd077f92289f3cc5b263246676b6e788be Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Dubois <jean-baptiste dubois parrot com>
Date: Mon, 7 Oct 2013 09:49:01 +0200
Subject: [PATCH]: Add large files (> 4GB) streaming support.

---
  configure.ac                                       |    2 +
  src/media-engines/simple/Makefile.am               |    1 +
  .../simple/rygel-simple-data-source.vala           |   39 
+++++++++++++++-----
  3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 3e4982e..a460a80 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,8 @@ dnl Disable generation of static libraries
  LT_PREREQ([2.2.6])
  LT_INIT([dlopen disable-static])

+AC_SYS_LARGEFILE
+
  dnl Required versions of library packages
  dnl Not all of these are actually used, depending on the configure 
options.
  GLIB_REQUIRED=2.31.13
diff --git a/src/media-engines/simple/Makefile.am 
b/src/media-engines/simple/Makefile.am
index 3a6aa8e..67b3dfb 100644
--- a/src/media-engines/simple/Makefile.am
+++ b/src/media-engines/simple/Makefile.am
@@ -8,6 +8,7 @@ librygel_media_engine_simple_la_SOURCES = \
      rygel-simple-data-source.vala

  librygel_media_engine_simple_la_VALAFLAGS = \
+    --pkg posix \
      $(RYGEL_COMMON_LIBRYGEL_SERVER_VALAFLAGS) \
      $(RYGEL_COMMON_VALAFLAGS)

diff --git a/src/media-engines/simple/rygel-simple-data-source.vala 
b/src/media-engines/simple/rygel-simple-data-source.vala
index d0ccbd4..e4b8cf5 100644
--- a/src/media-engines/simple/rygel-simple-data-source.vala
+++ b/src/media-engines/simple/rygel-simple-data-source.vala
@@ -34,8 +34,8 @@ internal class Rygel.SimpleDataSource : DataSource, 
Object {
      private Thread<void*> thread;
      private Mutex mutex = Mutex ();
      private Cond cond = Cond ();
-    private uint64 first_byte = 0;
-    private uint64 last_byte = 0;
+    private Posix.off_t first_byte = 0;
+    private Posix.off_t last_byte = 0;
      private bool frozen = false;
      private bool stop_thread = false;
      private HTTPSeek offsets = null;
@@ -103,13 +103,25 @@ internal class Rygel.SimpleDataSource : 
DataSource, Object {
      private void* thread_func () {
          var file = File.new_for_commandline_arg (this.uri);
          debug ("Spawning new thread for streaming file %s", this.uri);
+        int fd = -1;
          try {
-            var mapped = new MappedFile (file.get_path (), false);
+            fd = Posix.open (file.get_path (), Posix.O_RDONLY, 0);
+            if (fd < 0)
+                throw new IOError.FAILED("Failed to open file '%s': %s",
+                                         file.get_path (),
+ Posix.strerror(Posix.errno));
+
              if (this.offsets != null) {
-                this.first_byte = this.offsets.start;
-                this.last_byte = this.offsets.stop + 1;
+                this.first_byte = (Posix.off_t)this.offsets.start;
+                this.last_byte = (Posix.off_t)this.offsets.stop + 1;
              } else {
-                this.last_byte = mapped.get_length ();
+                this.first_byte = 0;
+                this.last_byte = Posix.lseek (fd, 0, Posix.SEEK_END);
+                Posix.lseek(fd, 0, Posix.SEEK_SET);
+            }
+
+            if (this.first_byte != 0) {
+                 Posix.lseek(fd, this.first_byte, Posix.SEEK_SET);
              }

              while (true) {
@@ -134,9 +146,14 @@ internal class Rygel.SimpleDataSource : DataSource, 
Object {
                      stop = this.last_byte;
                  }

-                unowned uint8[] data = "" mapped.get_contents ();
-                data.length = (int) mapped.get_length ();
-                uint8[] slice = data[start:stop];
+                var slice = new uint8[stop - start];
+                var len = (int)Posix.read(fd, slice, slice.length);
+                if (len < 0)
+                    throw new IOError.FAILED("Failed to read file '%s': 
%s",
+                                             file.get_path (),
+ Posix.strerror(Posix.errno));
+
+                slice.length = len;
                  this.first_byte = stop;

                  // There's a potential race condition here.
@@ -149,7 +166,9 @@ internal class Rygel.SimpleDataSource : DataSource, 
Object {
                  });
              }
          } catch (Error error) {
-            warning ("Failed to map file: %s", error.message);
+            warning ("Failed to stream file: %s", error.message);
+        } finally {
+            Posix.close(fd);
          }

          // Signal that we're done streaming
_______________________________________________
rygel-list mailing list
rygel-list gnome org
https://mail.gnome.org/mailman/listinfo/rygel-list

_______________________________________________
rygel-list mailing list
rygel-list gnome org
https://mail.gnome.org/mailman/listinfo/rygel-list