Hi, thanks for this patch, makes completely sense :) For the future, please file patches in bugzilla because we have a nice review infrastructure there :)
One question though: Why are you using the Posix stuff and not the GLib file functions? 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. > 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 <[email protected]> > 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 = (uint8[]) 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 > [email protected] > https://mail.gnome.org/mailman/listinfo/rygel-list _______________________________________________ rygel-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/rygel-list
