Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finer seeking support #16

Open
est31 opened this issue May 30, 2017 · 14 comments · May be fixed by #94
Open

Finer seeking support #16

est31 opened this issue May 30, 2017 · 14 comments · May be fixed by #94

Comments

@est31
Copy link
Member

est31 commented May 30, 2017

Commit 5748b26 has implemented seeking support on a per page granularity level.

For some use cases this is not fine enough.

Please, if you use this library and the granularity is not fine enough for you, tell me so in this thread.

@est31 est31 mentioned this issue May 30, 2017
@plietar
Copy link
Contributor

plietar commented May 30, 2017

I'm not really familiar with ogg and vorbis, what sort of granularity do pages offer ?
The comment on the function describes the argument as absolute granule position. What's the unit for this ?

Thanks

@est31
Copy link
Member Author

est31 commented May 30, 2017

what sort of granularity do pages offer ?

The granularity depends on many factors. How well the stream is compressed, for example, but also on encoder choice (whether to put as many packets into a page as possible, or fewer). It would be cool if you could try it out, and I could get feedback whether its enough for you or not.

What's the unit for this ?

The absolute granule position is in our case just the (per channel) sample count, see also section A.2 of the vorbis spec.

It can also be used for displaying how far one has progressed in the stream (the absgp_page field of the Packet struct return value of the get_last_absgp function).

@mark-buer
Copy link

Hi!

Some (hopefully) helpful suggestions:

  • perhaps name this quantised seek function such that you have room to later add a non-quantised seek function,
  • add doc comments to this quantised seek function that explains the behaviour a little better (for example, does this quantised seek result in a seek to a location <= absgp, or >= absgp, and what happens when performing a quantised seek to the minimum and maximum possible absgp?)

A use-case for exact seek might be if someone requires seamless looping of sections of an ogg vorbis file. In such a scenario, being out by just a few tens of samples can result in audible clicks and pops. For my particular use-case, having accurate whole file looping is adequate. Thus, so long as one can seek the stream from the beginning, and so long as this seek results in the exact same samples being returned as were returned when first playing through, it would work for me.

Thanks for adding the seek functionality. The prior lack of seek drove me to use libvorbisfile, but in the future I'll probably reevaluate that decision.

@est31
Copy link
Member Author

est31 commented May 30, 2017

@mark-buer thanks for the interesting comment! What do you mean with "non-quantised" seek?

@mark-buer
Copy link

What do you mean with "non-quantised" seek?

By "quantised seek", I mean "per page granularity level seek".
By "non-quantised seek", I mean "exact seek that is in no way snapped to the nearest page boundary".
Sorry for the confusion.

@est31
Copy link
Member Author

est31 commented May 30, 2017

Hmmm this makes me remember, I probably need to make sure that seeking to the beginning is okay...

@plietar
Copy link
Contributor

plietar commented May 31, 2017

@est31 I've given it a try, and it works well for my usecase, and with this lewton has everything I need to replace libvorbis. Thanks for your work !

As a suggestion, could the seek_absgp_pg function return the actual position that was seeked at ? This way the feedback can be more accurate.

I probably need to make sure that seeking to the beginning is okay...

seek_absgp_pg(0) is working fine

@est31
Copy link
Member Author

est31 commented May 31, 2017

lewton has everything I need to replace libvorbis.

Great to hear!

As a suggestion, could the seek_absgp_pg function return the actual position that was seeked at ? This way the feedback can be more accurate.

What do you mean by position that was seeked at? The byte offset, or the resulting absgp? get_last_absgp is a function that gives you the absgp of the last started page, maybe read a packet after the seek to initialize it with a value and use its return value? Note that streams are not guaranteed to start with absgp == 0, so its not very useful for absolute measurements, but it should be for relative ones.

@plietar
Copy link
Contributor

plietar commented May 31, 2017

AFAIU, seek_absgp_pg positions the stream to the start of the page which contains the requested absgp. But because of the page granularity, the start of the page may differ a bit from what was reqested.

What I'd like is for seek_absgp_pg to return the absgp corresponding to the start of the page, ie where the stream has actually been seeked to, which may be slightly less than the requested position.

get_last_absgp returns None right after seeking. I must read a page before getting a value from it.

@est31
Copy link
Member Author

est31 commented May 31, 2017

seek_absgp_pg(0) is working fine

Really? Nice to hear, but from knowing the code I was expecting an AudioIsHeader read error...
In any case, if you encounter it in such a situation it should be safe to ignore the error and continue decoding, but it would be great if you could also file a bug, then I'll fix it on my side where its cleaner.

seek_absgp_pg positions the stream to the start of the page which contains the requested absgp. But because of the page granularity, the start of the page may differ a bit from what was reqested.

Right. If you want to know the details, its a bit more complicated with continued packets (packets that span multiple page boundaries). Here if you seeked for the absgp 42, and page A had absgp 38, and page B had absgp 50, and a packet would span the boundary between A and B, you'd get to page A. At least this is what the code is supposed to do, no idea whether it actually does it.

What I'd like is for seek_absgp_pg to return the absgp corresponding to the start of the page, ie where the stream has actually been seeked to, which may be slightly less than the requested position.

I see. What would you do with that value if you had it?

The issue is that obtaining the offset between the end of the page and its start where the next call to read a packet will return a value is almost as hard as actually putting you closest to the requested absgp on a per packet basis.

@plietar
Copy link
Contributor

plietar commented May 31, 2017

I was expecting an AudioIsHeader read error

Actually I do. The files I'm decoding are a bit weird, with some proprietary/undocumented extensions. With libvorbis I always get a OV_HOLE error around the start of the file which I've been ignoring. When porting to lewton I assumed AudioIsHeader was the equivalent error. Nevertheless ignoring the error works fine.

I see. What would you do with that value if you had it?

Send it back to the controlling thread, and fix the position in the GUI. This isn't too important since the seeking error is small enough and I can update the position after the next packet read using get_last_absgp. I hadn't realised this would be complicated to implement.

@est31
Copy link
Member Author

est31 commented Nov 7, 2018

The changes in RustAudio/ogg@1e54824 were originally meant to enable dbfaae5 but they also allow for more precise seeking, provided further work in lewton.

@TonalidadeHidrica
Copy link

Please, if you use this library and the granularity is not fine enough for you, tell me so in this thread.

Precise seeking is very important for my project. Probably I can contribute to this feature. (Writing here just in case)

@polina4096
Copy link

Precise seeking support would be really nice to have. My usecase is a rhythm game, where I need to keep gameplay in sync with the audio when seeking :\

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants