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

Reset file position #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reset file position #95

wants to merge 1 commit into from

Conversation

Chocobo1
Copy link
Contributor

@Chocobo1 Chocobo1 commented Dec 4, 2024

The file handle might be at EOF if the input .opus file is small. So reset the file position properly before any further handling.

Fixes #78.

The file handle might be at EOF if the input .opus file is small. So
reset the file position properly before any further handling.

Fixes xiph#78.
@vlakoff
Copy link

vlakoff commented Dec 4, 2024

So, before 5d0ac20, FLAC__stream_decoder_process_until_end_of_metadata() was provided a file at position 4 (i.e. after the FLaC magic number), and it was able to process the following metadata.

Just as a suggestion, maybe fseek(in, 4, SEEK_SET)?

Reference: FLAC__stream_decoder_process_until_end_of_metadata() implementation

@vlakoff
Copy link

vlakoff commented Dec 4, 2024

Wait, according to 5d0ac20, FLAC files with ID3 tags don't start with the FLaC magic number. Instead, they start with the ID3 data, with the FLaC magic number appearing only after. Bummer…

@vlakoff
Copy link

vlakoff commented Dec 4, 2024

What I don't know about FLAC__stream_decoder_process_until_end_of_metadata(), and having the answer to these could help:

  • Does it support starting to read before the FLaC magic number? (i.e. reading through it)
  • Does it support reading through ID3 data?

@Chocobo1
Copy link
Contributor Author

Chocobo1 commented Dec 4, 2024

Does it support starting to read before the FLaC magic number? (i.e. reading through it)

Yes. It would be extremely strange/inconvenient if it couldn't do that, given that the stream decoder is just initialized and the next call is FLAC__stream_decoder_process_until_end_of_metadata():

opus-tools/src/flac.c

Lines 390 to 396 in 98f3ddc

FLAC__stream_decoder_init_stream:FLAC__stream_decoder_init_ogg_stream))(
flac->decoder,read_callback,NULL,NULL,NULL,eof_callback,
write_callback,metadata_callback,error_callback,flac)==
FLAC__STREAM_DECODER_INIT_STATUS_OK){
/*Decode until we get the file length, sample rate, the number of channels,
and the Vorbis comments (if any).*/
if(FLAC__stream_decoder_process_until_end_of_metadata(flac->decoder)&&

Does it support reading through ID3 data?

I suppose yes. FLAC do support ID3v2 format.

You can see the implementation:
https://github.com/xiph/flac/blob/d1d72aa77cfeb71357b59411f981ff52ac1a1f60/src/libFLAC/stream_decoder.c#L1609

@vlakoff
Copy link

vlakoff commented Dec 4, 2024

Assuming a regular FLAC file (i.e. much more than 64 KB filesize, and leading metadata less than 64 KB), how come it was still working after 5d0ac20? For instance, the file is already seeked after the metadata that provides the value for flac->channels.

My guess would be that FLAC__stream_decoder_init_ogg_stream and/or FLAC__stream_decoder_process_until_end_of_metadata already manage to rewind the file, except in the case that EOF has been reached (thus triggering eof_callback, unexpectedly actually).

@vlakoff
Copy link

vlakoff commented Dec 6, 2024

If you search for "hack" (and more specifically, "hack note") in stream_decoder.c, you will notice there are already a handful of hacks related to Ogg FLAC and EOF.

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 this pull request may close these issues.

Opusenc from git fails to encode very short flac files on windows
2 participants