Implement loadstreaming/savestreaming API #78

ssfrr · 2016-09-28T15:48:55Z

I figured it would be easier to discuss #77 if we had some code to look at. This PR is not close to mergeable, it's mostly to play with how this might work and get some feedback from you guys on whether you think this is a promising direction.

Here's the context (copied from the issue):

IIUC load and save are generally one-off operations, i.e. load("somefile.wav") loads the full wave file into memory.

Sometimes though, you have huge files that you might not want to load into memory all at once, or streams that have no end (like an internet radio stream). In my LibSndFile package I also export loadstream and savestream functions that instead return special stream types that you can read/write chunks of audio from.

@jongwook's MP3 package also has the same functionality, but it's not clear who should own those functions. It seems like it would make sense to have them in FileIO as they use all the same query tooling. In fact, we implement load and save as wrappers around the streaming versions.

Do these functions seem like they could fit into the FileIO architecture? What are the video packages currently doing for this sort of thing?

SimonDanisch · 2016-09-30T09:07:01Z

Thanks! I'd love to have streaming support in FileIO!
I think current video packages are not integrated yet :(

timholy · 2016-10-03T14:10:13Z

See #77 (comment). If this adds functionality on top of what's already there, great, but let's at least address what's missing.

ssfrr · 2016-10-03T22:35:03Z

The difference is that these streams produce higher-level formatted data, not just raw file bytes. For instance:

s = loadstreaming("myfile.ogg")
x = read(s, 1000) # returns 1000 frames of audio in a SampleBuf
close(s)

In this case the underlying bytestream on disk is ogg-formatted, but when I read from this formatted stream I get samples of audio. If the ogg file is very large (or infinite), I can read or process it in chunks. It also provides a unified API between opening streams to hardware capture/playback and working with files, or infinite network streams.

You can see some other examples of this API in LibSndFile.jl, though in that library it's currently called loadstream. I think loadstreaming may be a better name.

timholy · 2016-10-04T15:10:38Z

I see. That's not bad, in a multi-image file we could also load a single frame at a time. 👍

Regarding names, to me it seems that the distinguishing feature here (to avoid the kind of confusion that nabbed me) is the "chunk." loadchunk?

ssfrr · 2017-10-11T21:13:23Z

I think loadchunk implies we're loading a single chunk. IMO the difference isn't necessarily the chunkiness of the reading (you could read 1 sample at a time), it's that the thing you get from your loadX operation is itself a stream. I suppose I would be open to loadchunked, but that makes it seem like the stream you get has some inherent chunkiness rather than being a continuous stream you can read from in any size you want.

I think from a user's perspective it's a reasonable convention that load("somefile.mov") gets them the whole video, but loadstreaming("somefile.mov") gives them a video stream that they can read from (and get e.g. a Vector{Image} or some Video object or something). Another way to think about it is that the function name indicates what sort of object is returned (the output), and the argument type indicates what sort of underlying data is being used (the input).

For my purposes I generally consider streaming to be the more fundamental operation, in the sense that

x = load("mysong.wav")

is equivalent to

str = loadstreaming("mysong.wav")
x = read(str)
close(str)

or even better:

x = loadstreaming("mysong.wav") do str
    read(str)
end

We could in fact define load and save fallbacks that are defined as above, so packages can just implement loadstreaming and savestreaming and get load and save automatically.

Though I'm sure there are data types where you have to read the whole thing before you can start doing anything useful, so in those cases maybe the relationship would be reversed.

To better demo the benefit, this is a demo that works on the current LibSndFile, except the loadstream changed to loadstreaming.

loadstreaming("srcfile.wav") do src
    savestreaming("destfile.wav", nchannels(src), samplerate(src), Float32) do dest
        while write(dest, float(read(src, 2048)) / 2) == 2048 end
    end
end

And you get an output with half the amplitude of your input, while only keeping 2048 frames in memory at a time (of course in real code we'd want to pre-allocate the buffer to reduce allocation).

ssfrr · 2017-11-25T05:32:26Z

Just rebased and added some tests, should be ready for review.

There's some refactoring that could be done to remove some duplication, but I figured I'd leave it as-is for now to make the diff easier to read. I can either do the refactoring as part of this PR or a follow-up.

ssfrr · 2017-11-25T17:12:47Z

used a little metaprogramming to reduce the duplication introduced in this PR.

timholy

👍 Love it!

timholy · 2017-11-27T11:10:22Z

README.md

+end
+
+function load(q::Formatted{format"WAV"}, args...; kwargs...)
+    savestreaming(q, args...; kwargs...) do stream


loadstreaming

timholy · 2017-11-27T11:11:44Z

README.md

+    ownstream::Bool
+end
+
+function read(reader::WAVReader, frames::Int)


Is this a private read or Base.read? (FileIO's docs were written, I think, before the the whole "do you extend or not?" debate was settled, but we should certainly fix this.)

timholy · 2017-11-27T11:12:25Z

README.md

+    # read and decode audio samples from reader.io
+end
+
+function close(reader::WAVReader)


Likewise, clarify whether this is Base.close or a private close. (It's fine to explain all in just one place.)

Also, I notice your test implements eof, worth mentioning?

whoops, should be Base.[read|write|close]. I'll fix. re: eof, I'd love it if there were a well-defined and documented stream interface we could link to here. I can try at least adding a list of functions to implement here and maybe eventually migrate it to the julia docs

I went down a bit of a rabbit hole trying to figure out a more formalized stream interface. My notes are here, but basically it's not super well-defined at this point. Once that's nailed down we can link to it in these docs.

timholy · 2017-11-27T11:13:56Z

README.md

+    # do whatever cleanup the reader needs
+    reader.ownstream && close(reader.io)
+end
+loadstreaming(f::File{format"WAV"}) = WAVReader(open(f), true)


Add blank line before this one? Or does the "fallback" comment below also apply to close?

re-ordered the comment to clarify this

timholy · 2017-11-27T11:18:02Z

src/loadsave.jl

+type that can be read from in chunks, rather than loading the whole contents all
+at once
+- `loadstreaming(strm)` loads the stream from an `IOStream` or similar object. In this case,
+the magic bytes are essential.


Not quite sure what "are essential" means. Perhaps say "the format is inferred from magic bytes in strm"?

hah, copied that from the load docstring. I think the idea is that because there's no file extension the magic bytes are the only way to get the file type. I can clarify.

clarified in new commit

timholy · 2017-11-27T11:18:25Z

src/loadsave.jl

+- `loadstreaming(strm)` loads the stream from an `IOStream` or similar object. In this case,
+the magic bytes are essential.
+- `load(File(format"PNG",filename))` specifies the format directly, and bypasses inference.
+- `load(f; options...)` passes keyword arguments on to the loader.


load->loadstreaming? Also, is that really File or did you mean Stream (as below with savestreaming)?

timholy · 2017-11-27T11:22:01Z

src/loadsave.jl

+
+- `savestreaming(filename, data...)` saves the contents of a formatted file,
+trying to infer the format from `filename`.
+- `savestreaming(Stream(format"PNG",io), data...)` specifies the format directly, and bypasses inference.


So this produces an output Stream, not a file, right? Is savestreaming(File(...)) also supported? (bypassing inference)

loadstreaming and savestreaming (and load and save I believe) should support Files and Streams. For example, if I call loadstreaming on a stream coming in from a network, I should be able to read formatted data (e.g. audio frames or video frames) from the returned object, which will in turn read from the stream. calling loadstreaming on a File just opens that file and then handles it as a stream.

similarly savestreaming should handle both Files and Streams.

The new commit I just pushed adds more methods to the docstring. I also added them for load and save, which I believe should also support both.

ssfrr · 2018-03-18T03:48:35Z

I just rebased and made edits based on your comments. Apologies this sat dormant for a while, I know that makes it tougher to review. Compared to what you looked at last time this is just documentation updates.

Tests seem to be passing on 0.6 but failing on nightlies due to some Nullable issue - probably unrelated to this PR?

ssfrr · 2018-04-04T00:25:43Z

bump. 😃

SimonDanisch · 2018-04-04T08:52:02Z

Well, if @timholy loves it I don't see why we shouldn't merge this ;) If no one is against it, I merge it tonight!

ssfrr force-pushed the loadsavestreaming branch from 36126ed to d922a07 Compare November 25, 2017 04:47

ssfrr changed the title ~~[RFC] initial WIP draft to demonstrate streaming API~~ Implement loadstreaming/savestreaming API Nov 25, 2017

timholy reviewed Nov 27, 2017

View reviewed changes

evetion mentioned this pull request Jan 10, 2018

Should there be a load! function? #171

Closed

6 tasks

ssfrr mentioned this pull request Feb 20, 2018

Review of IO blocking behaviour JuliaLang/julia#24526

Open

14 tasks

ssfrr added 11 commits March 17, 2018 22:36

initial WIP draft to demonstrate general idea

0d93031

fixes do-notation so it returns whatever the given function returns

aaec881

moves docstrings to after functions are defined

c2ca8db

some tweaks to clean up the diff

7761bbf

clarifies some comments, adds Dummy reader/writer structs

6c8e1fb

adds some tests for loadstreaming and savestreaming

034d5b3

adds a test for forced-format with filename

4346a89

fix for 32-bit, apparently position(s) always returns Int64

ff9fe57

some README fixes [ci skip]

5fb37db

brings duplication down to previous levels

0f5930c

fixes some documentation issues spotted by @timholy

239af17

ssfrr force-pushed the loadsavestreaming branch from 45815e9 to 239af17 Compare March 18, 2018 03:35

ssfrr mentioned this pull request Mar 25, 2018

Interested in helping out... JuliaAudio/SampledSignals.jl#29

Closed

SimonDanisch merged commit fe36768 into JuliaIO:master Apr 5, 2018

ssfrr mentioned this pull request May 26, 2018

Support loadstreaming/savestreaming FileIO API JuliaIO/TranscodingStreams.jl#47

Open

ssfrr mentioned this pull request Aug 13, 2018

add loadstreaming and savestreaming functions #77

Closed

ssfrr mentioned this pull request Nov 21, 2019

Dummy module now defines its own load/save instead of extending #237

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement loadstreaming/savestreaming API #78

Implement loadstreaming/savestreaming API #78

ssfrr commented Sep 28, 2016

SimonDanisch commented Sep 30, 2016

timholy commented Oct 3, 2016

ssfrr commented Oct 3, 2016

timholy commented Oct 4, 2016

ssfrr commented Oct 11, 2017

ssfrr commented Nov 25, 2017

ssfrr commented Nov 25, 2017

timholy left a comment

timholy Nov 27, 2017

timholy Nov 27, 2017

timholy Nov 27, 2017

ssfrr Nov 27, 2017

ssfrr Mar 18, 2018

timholy Nov 27, 2017

ssfrr Mar 18, 2018

timholy Nov 27, 2017

ssfrr Nov 27, 2017

ssfrr Mar 18, 2018

timholy Nov 27, 2017

timholy Nov 27, 2017

ssfrr Mar 18, 2018

ssfrr commented Mar 18, 2018

ssfrr commented Apr 4, 2018

SimonDanisch commented Apr 4, 2018

Implement loadstreaming/savestreaming API #78

Implement loadstreaming/savestreaming API #78

Conversation

ssfrr commented Sep 28, 2016

SimonDanisch commented Sep 30, 2016

timholy commented Oct 3, 2016

ssfrr commented Oct 3, 2016

timholy commented Oct 4, 2016

ssfrr commented Oct 11, 2017

ssfrr commented Nov 25, 2017

ssfrr commented Nov 25, 2017

timholy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssfrr commented Mar 18, 2018

ssfrr commented Apr 4, 2018

SimonDanisch commented Apr 4, 2018