[flac-dev] Feedback on implementation of decoding of chained streams

[Martijn van Beurden]

For some reason, I didn't get Brians email, but I noticed it was
replied to, so I'll copy from the archive

> Is there an overview - in plain English - summarizing the API changes?

No, not yet, as it isn't final yet. However, for the API changes, one
only has to look at the header files, not the actual source files.

> Without diving in to the source changes, my first question is: Would it be possible to leave the existing API as it is, but only add new functions for continuous stream decoding? If only new function names are used, then existing code will not have to change or be recompiled.

You are confusing two things here. The API changes are compatible with
any application already using libFLAC. To expose the new
functionality, one has to use the new function
FLAC__stream_decoder_set_decode_chained_stream(). If that is not used,
the API is exactly the same. However, the ABI has necessarily changed,
so to use the new libFLAC, I presume one has to recompile, without any
source changes.

> Questions such as the above are why I was hoping for an overview. It would be nice to see a brief summary of what the original API was (probably) intended to support; whether we have changed our expectations or merely found flaws in the original design; and what sorts of changes were necessary, and why. It would be especially helpful if there was already discussion of the pros and cons of adding new functions for the new support, versus altering the API of existing functions to enhance existing support to broader use cases.

I'll summarize. Four functions were added to libFLAC:
- FLAC__stream_decoder_set_decode_chained_stream
- FLAC__stream_decoder_get_decode_chained_stream
- FLAC__stream_decoder_finish_link
- FLAC__stream_decoder_process_until_end_of_link

In addition, the next functions no longer fail if invoked after a
stream has been initialized
- FLAC__stream_decoder_set_metadata_respond
- FLAC__stream_decoder_set_metadata_respond_application
- FLAC__stream_decoder_set_metadata_respond_all
- FLAC__stream_decoder_set_metadata_ignore
- FLAC__stream_decoder_set_metadata_ignore_all

That means the metadata filters can now be changed after
initialization, which wasn't possible before.

Additionally, two structs were changed. To
FLAC__StreamDecoderReadStatus,
FLAC__STREAM_DECODER_READ_STATUS_END_OF_LINK was added. To
FLAC__StreamDecoderState, FLAC__STREAM_DECODER_END_OF_LINK was added.
These two additions break the ABI compatibility.

Op zo 1 sep 2024 om 23:55 schreef Timothy B. Terriberry <tterribe at xiph.org>:
>
> brianw wrote:
> > On Sep 1, 2024, at 12:44 PM, Martijn van Beurden wrote:
> >> Everything works, but as is with any change to the API, I don't know
> >> whether it is convenient for API users other than the flac command
> >> line tool. So, I'd like some feedback.
>
> I only looked briefly, but I had a few questions.
>
> How does FLAC__stream_decoder_get_total_samples() work for chained
> streams? I.e., is there a way to get the total samples for all chained
> links (to know what the range of valid values for target_sample is for
> seek_to_absolute_sample_())?

It returns the number of samples stated in the metadata of the current
active link.

> Since chained streams can have different sample rates, how would one go
> about seeking to a specific _time_?

I assume one would first use the sample rate of the first link to
guess the sample number, seek to that point, correct if it turns out
one of the links that passed has a different sample rate, seek again
etc.

> Am I reading the seeking implementation correctly that the only way to
> seek to a future link is to scan forward through all of the file data?

Yes, that is correct.