vorbis icon indicating copy to clipboard operation
vorbis copied to clipboard

Allow Vorbis Read Size to be set by user

Open 8W9aG opened this issue 6 years ago • 8 comments

  • Hooking up Vorbis read callbacks to an HTTP data provider currently yields a lot of overhead due to many read requests at 2kB
  • Allowing the user to set the vorbis read size lets backend reads become more efficient by reading greater chunks at any given time

8W9aG avatar Nov 21 '17 18:11 8W9aG

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

tdaede avatar Nov 22 '17 00:11 tdaede

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

erikd avatar Nov 23 '17 06:11 erikd

Thanks for the feedback!

  • Will add some sanity checks that prevent <= 0 and > 64kB
  • Will bump the minor version number (@tdaede do you know how to do this?)
  • Will add a getter

8W9aG avatar Nov 27 '17 17:11 8W9aG

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

8W9aG avatar Nov 27 '17 17:11 8W9aG

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

tdaede avatar Dec 05 '17 03:12 tdaede

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

8W9aG avatar Dec 05 '17 03:12 8W9aG

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.

tterribe avatar Jan 08 '18 18:01 tterribe

@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.

8W9aG avatar May 21 '18 04:05 8W9aG