xiph / vorbis

Reference implementation of the Ogg Vorbis audio format.
BSD 3-Clause "New" or "Revised" License
467 stars 188 forks source link

Allow Vorbis Read Size to be set by user #36

Open 8W9aG opened 6 years ago

8W9aG commented 6 years ago
tdaede commented 6 years ago

Looks okay, but adding API surface will probably need a soname minor bump. Also, it might be wise to sanity check the read size.

erikd commented 6 years ago

Yeah, really need sanity checking on that. Pretty sure really bad things would happen with negative numbers and very large positive numbers.

8W9aG commented 6 years ago

Thanks for the feedback!

8W9aG commented 6 years ago

I think the version bump is stuffing up running the CI tests, can someone advise how to do this properly?

tdaede commented 6 years ago

I can just pull without the minor commit number for now and do that separately.

8W9aG commented 6 years ago

@tdaede Thanks i'll reset this branch to 3ef0b16

tterribe commented 6 years ago

Just FYI, but doing a separate HTTP request for every read operation is going to have significant latency and overhead, regardless of the read size. A much better approach is to request the entire resource, and just close the connection if the next read doesn't pick up where the previous one left off. That means you don't need to change the read size Vorbis uses at all. You run the risk of buffering some extra data in the OS's socket buffers, but this is much cheaper than all of the round-trips required to issue a new request in the common case.

You can use a more sophisticated strategy that maintains multiple connections and issues smaller requests when seeking (gradually building back up to larger ones as streaming resumes). This is substantially more complicated to implement, though. See https://git.xiph.org/?p=opusfile.git;a=blob;f=src/http.c;hb=HEAD for an example implementation for libopusfile (using largely the same callback API as libvorbisfile). However, even in this case, the size of each HTTP request is completely independent of the amount you read off the socket in each callback.

8W9aG commented 6 years ago

@tterribe We can't really request the entire resource at once, it adds massive latency to the start of our program. We also want to explicitly support seeking which means we cannot close the connection if the next read doesn't pick up where the previous one left off. The overhead produced by HTTP when using large payloads for the ogg read size is insignificant, and shrinks as the payload size is increased. Since we are developing a cross platform application that takes advantage of mobile and has features such as OAuth header injection, we cannot easily use the http module provided by vorbis, therefore I am still of the opinion that this is the best option, since it gives the user of the library more control and doesn't force them to write complicated wrappers specifically for dealing with this use case.