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

Update seek API to use absolute rather than relative sample number #45

Closed
mewmew opened this issue Jan 23, 2021 · 2 comments
Closed

Update seek API to use absolute rather than relative sample number #45

mewmew opened this issue Jan 23, 2021 · 2 comments

Comments

@mewmew
Copy link
Member

mewmew commented Jan 23, 2021

After playing around with the seek API implemented in #44, @karlek and I noticed one minor issue with the API. The flac.Stream does not keep track of the current sample number, so relative seek becomes inaccurate.

The flac.Stream can only keep track of where the reader is in the frame, and as such position the reader such that the next call to stream.ParseNext returns a frame which includes the sample number the user wants to seek to.

A high-level audio playback API or application can keep track of the current sample number (i.e. the current index into the frame.SubFrames[i].Samples slice), but such information is unavailable to the FLAC frame decoder, as it only works on the resolution of frames (and not sample numbers).

Therefore, the best the low-level mewkiz/flac API can do is to position the reader of the flac.Stream such that the next call to stream.ParseNext returns the frame which includes the frame number the user wants to seek to.

And since the flac.Stream does not keep track of the current sample number (at best, it can track the first sample number of the current frame, but not with higher precision on sample number than that) the best approach would be to use absolute rather than relative sample number in the API of Seek.

@karlek has a proof of concept implementation on his laptop. He'll submit a PR in the following days.

Another aspect is that the seek API should seek to the frame containing the sample number the user wants to seek to. The current implementation seeks to the closest seek point.

All in all, these are minor changes to the seek API, but essential to have a solid low-level API on which high-level audio playback APIs and applications can build upon. It would be the task of such high-level APIs to keep track of the current sample number.

The low-level mewkiz/flac API only handles decoding of FLAC streams at the resolution of frames, not the resolution of samples.

So, the suggested see API would look as follows:

// Seek seeks to the frame containing the given absolute sample number. The return value
// specifies the first sample number of the frame containing sampleNum.
func (stream *Stream) Seek(sampleNum uint64) (frameSampleNum uint64, err error)
@mewmew
Copy link
Member Author

mewmew commented Jan 23, 2021

Just a small note to keep track of this. There is an off-by-one in the computed seek table. The cause is that the offset is recorded after the call to ParseNext in makeSeekTable rather than before. As such, the offset tracks the next frame rather intended frame of the seek point.

-               f, err := stream.ParseNext()
-               if err == io.EOF {
-                       break
-               }
-
+               o, err := rs.Seek(0, io.SeekCurrent)
                if err != nil {
                        return err
                }
 
-               o, err := rs.Seek(0, io.SeekCurrent)
+               f, err := stream.ParseNext()
                if err != nil {
+                       if err == io.EOF {
+                               break
+                       }
                        return err
                }

Embedded seek table of FLAC file (source of truth):

   seekTable: &meta.SeekTable{
        Points: {
            {SampleNum:0x0, Offset:0x0, NSamples:0x1000},
            {SampleNum:0x1000, Offset:0x11b5, NSamples:0x1000},

Computed seek table (of makeSeekTable):

    seekTable: &meta.SeekTable{
        Points: {
            {SampleNum:0x0, Offset:0x11b5, NSamples:0x1000},
            {SampleNum:0x6b000, Offset:0xd3f64, NSamples:0x1000},

@karlek karlek mentioned this issue Jan 25, 2021
@mewmew
Copy link
Member Author

mewmew commented Jan 27, 2021

Resolved in #46.

@mewmew mewmew closed this as completed Jan 27, 2021
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

No branches or pull requests

1 participant