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

The option -r should not add any file wrappers #190

Open
moubctez opened this issue Nov 12, 2024 · 2 comments
Open

The option -r should not add any file wrappers #190

moubctez opened this issue Nov 12, 2024 · 2 comments

Comments

@moubctez
Copy link

If I compress a RIFF-WAV file, I end up with the header stored in WavPack file. This is expected:

% wavpack -q audio.wav
% wvunpack -ss audio.wv
[...]
source format:     Microsoft RIFF with 'wav' extension
file wrapper:      44 byte RIFF header

When I add -r option, WavPack creates its own RIFF header, which is longer than the original one:

% wavpack -qr audio.wav
% wvunpack -ss audio.wv
[...]
source format:     Microsoft RIFF with 'wav' extension
file wrapper:      80 byte RIFF header

I would expect -r didn't add any file wrappers at all. To achieve that, I removed this section from the source code:

--- src/pack_utils.c.orig       2024-11-11 23:51:28.946320254 +0000
+++ src/pack_utils.c
@@ -680,13 +680,6 @@ int WavpackPackSamples (WavpackContext *
         unsigned int samples_to_copy;
         int stream_index;
 
-        if (!wpc->riff_header_added && !wpc->riff_header_created && !wpc->file_format) {
-            char riff_header [128];
-
-            if (!add_to_metadata (wpc, riff_header, create_riff_header (wpc, wpc->total_samples, riff_header), ID_RIFF_HEADER))
-                return FALSE;
-        }
-
         if (wpc->acc_samples + sample_count > wpc->max_samples)
             samples_to_copy = wpc->max_samples - wpc->acc_samples;
         else

Now, the file is what I expected:

source format:     Microsoft RIFF with 'wav' extension
file wrapper:      none stored
@dbry
Copy link
Owner

dbry commented Nov 13, 2024

What exactly is the problem with storing a generic header in the WavPack file? The purpose of this is to allow very old versions of wvunpack to unpack WavPack files to valid WAVs (at one point it could not generate a header if one was not provided).

But more generally, if you force WavPack to not put on a generic header (as you have), the current version of wvunpack will simply generate the same exact header itself, so there's actually no practical difference. As far as I know, no other programs even look at those stored headers except to see if there's other RIFF metadata in there (which in this case there would not be).

Of course, it adds 80+ bytes to the file. Is that the issue?

BTW, the reason that the header is bigger than the standard 44-byte version is that it reserves space for the RF64 header which is required with 2+ GB files (which might not be known in advance if the source is stdin). I could improve this, but it's a little tricky because this must be updated in-place.

Thanks!

@moubctez
Copy link
Author

Thank you for the explanation.

Yes, I would like to get rid of the extra 80 bytes. :)

Also, the description for -r option says "do not store the headers in the resulting WavPack file", but in the end, there is a header stored in the file, so that's a little bit misleading. I would very much like to have no headers at all when -r has been given. Maybe you could consider another option for that purpose?

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

2 participants