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

[test] Add 24khz sample to testing, remove the log line nudging users #69

Closed
wants to merge 3 commits into from
Closed

Conversation

petercsiba
Copy link

Uploaded an audio generated by OpenAI Text to speech model which outputs in 24khz;
https://platform.openai.com/docs/guides/text-to-speech/do-i-own-the-outputted-audio-files

Note that just using flac2wav doesn't yield a .wav file that I can play (via Apple Music or QuickTime), so I told myself lets add it here and see what the tests say.
The file should be playable though, as proven by converting via ffmpeg:

ffmpeg -i testdata/24000-tts-sf.flac -acodec pcm_s16le converted.wav

My flac2wav logspam was getting real, so I told myself I can give it a shot, please direct me this is the first open-source PR of mine 😅

2023/11/17 07:56:12 frame.Frame.parseHeader: The flac library test cases do not yet include any audio files with sample rate 24000. If possible please consider contributing this audio sample to improve the reliability of the test cases.
2023/11/17 07:56:12 frame.Frame.parseHeader: The flac library test cases do not yet include any audio files with sample rate 24000. If possible please consider contributing this audio sample to improve the reliability of the test cases.

@mewmew
Copy link
Member

mewmew commented Nov 17, 2023

Hi @Petrzlen,

Glad to see you helping to strengthen the tests of the library and stress test it further!

When you get the chance, please do upload a sample FLAC file that can be used for testing.

Cheerful regards,
Robin & Henry

@petercsiba
Copy link
Author

Hi @Petrzlen,

Glad to see you helping to strengthen the tests of the library and stress test it further!

When you get the chance, please do upload a sample FLAC file that can be used for testing.

Cheerful regards, Robin & Henry

Thanks for the quick nudge!

Realized .gitignore contains testdata;
so I -f added it there.

Please LMK what should I do next! Thanks

https://github.com/mewkiz/flac/blob/e49c0d0becf2d9bdc60699080912a8a641e86e28/testdata/24000-tts-sf.flac

@mewmew
Copy link
Member

mewmew commented Nov 17, 2023

Thanks for the quick nudge!

Realized .gitignore contains testdata;
so I -f added it there.

Great. Thanks for uploading the test FLAC file.

Please LMK what should I do next! Thanks

I guess the next step is trying to figure out what makes this FLAC file special, and why the test cases are failing.

I'm celebrating an early Christmas with my family this weekend, and have a rather hectic work+uni life in the weeks to come.

So, for anyone who's curious to try and figure out what makes this FLAC file special, and why the test cases are failing, you are most warmly invited : )

With cheerful regards,
Robin

@wader
Copy link

wader commented Nov 18, 2023

Had a quick look. The FrameHash test i guess fails because the streaminfo md5 is zero and i don't see any special handling for test

$ fq '.metadatablocks[0].md5' testdata_24000-tts-sf.flac
    │00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f│0123456789abcdef│
0x10│                              00 00 00 00 00 00│          ......│.metadatablocks[0].md5: "00000000000000000000000000000000" (raw bits)
0x20│00 00 00 00 00 00 00 00 00 00                  │..........      │

$ flac -t testdata_24000-tts-sf.flac
flac 1.4.3
Copyright (C) 2000-2009  Josh Coalson, 2011-2023  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

testdata_24000-tts-sf.flac: WARNING, cannot check MD5 signature since it was unset in the STREAMINFO
ok

Also noticed that total number of samples is zero. I think the new IETF flac spec should have some additional info about that field as i remember asking about it.

$ fq '.metadatablocks[0].total_samples_in_stream' testdata_24000-tts-sf.flac
    │00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 0e 0f│0123456789abcdef│
0x10│               f0 00 00 00 00                  │     .....      │.metadatablocks[0].total_samples_in_stream: 0

@mewmew
Copy link
Member

mewmew commented Nov 18, 2023

Had a quick look. The FrameHash test i guess fails because the streaminfo md5 is zero and i don't see any special handling for test

Yeah, that makes sense. We can probably add a skip of the FrameHash test of the source file has MD5 hash 0.

Edit: as of commit fbea67c we not skip frame hash tests for FLAC files that do not contain a MD5 hash sum of the audio stream in StreamInfo.

Also noticed that total number of samples is zero. I think the new IETF flac spec should have some additional info about that field as i remember asking about it.

Cool! Thanks for digging into this @wader : )

@petercsiba petercsiba closed this by deleting the head repository May 1, 2024
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.

3 participants