WavPack icon indicating copy to clipboard operation
WavPack copied to clipboard

The option -r should not add any file wrappers

Open moubctez opened this issue 1 year ago • 2 comments

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

moubctez avatar Nov 12 '24 06:11 moubctez

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!

dbry avatar Nov 13 '24 17:11 dbry

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?

moubctez avatar Nov 14 '24 18:11 moubctez