Skip to content

Preserve redundant cbsize = 0 with --keep-foreign-metadata #878

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.

Sign up for GitHub

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

Merged
ktmf01 merged 5 commits into from
Feb 23, 2026
Merged

Preserve redundant cbsize = 0 with --keep-foreign-metadata #878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
74f95ed
Fix decoding of fmt chunk with cbSize == 0
tobinus Feb 17, 2026
cf43c55
Avoid magic when making test file with cbsize = 0
tobinus Feb 18, 2026
247ac5c
Move fmt chunk size calculations closer to writing
tobinus Feb 18, 2026
d028149
Avoid redundant WAVE64 conditionals
tobinus Feb 18, 2026
0d89278
Make WAVE64 fmt chunk padding independent of inner logic
tobinus Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 78 additions & 10 deletions src/flac/decode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,10 @@ static int DecoderSession_finish_ok(DecoderSession *d);
static int DecoderSession_finish_error(DecoderSession *d);
static FLAC__bool canonicalize_until_specification(utils__SkipUntilSpecification *spec, const char *inbasefilename, uint32_t sample_rate, FLAC__uint64 skip, FLAC__uint64 total_samples_in_input);
static FLAC__bool write_iff_headers(FILE *f, DecoderSession *decoder_session, FLAC__uint64 samples);
static FLAC__bool write_riff_wave_fmt_chunk_body(FILE *f, FLAC__bool is_waveformatextensible, uint32_t bps, uint32_t channels, uint32_t sample_rate, FLAC__uint32 channel_mask);
static FLAC__uint64 calculate_total_riff_wave_fmt_chunk_size(FileFormat format, FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize);
static FLAC__uint64 calculate_riff_wave_fmt_chunk_size_field(FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize);
static FLAC__uint64 calculate_wave64_fmt_chunk_size_field(FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize);
static FLAC__bool write_riff_wave_fmt_chunk_body(FILE *f, FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize, uint32_t bps, uint32_t channels, uint32_t sample_rate, FLAC__uint32 channel_mask);
static FLAC__bool write_aiff_form_comm_chunk(FILE *f, FLAC__uint64 samples, uint32_t bps, uint32_t channels, uint32_t sample_rate, FileFormat format, FileSubFormat subformat, FLAC__uint32 comm_length);
static FLAC__bool write_little_endian_uint16(FILE *f, FLAC__uint16 val);
static FLAC__bool write_little_endian_uint32(FILE *f, FLAC__uint32 val);
Expand Down Expand Up @@ -739,6 +742,7 @@ FLAC__bool write_iff_headers(FILE *f, DecoderSession *decoder_session, FLAC__uin

FLAC__uint64 iff_size;
uint32_t foreign_metadata_size = 0; /* size of all non-audio non-fmt/COMM foreign metadata chunks */
FLAC__bool preserve_dummy_cbsize = false;
Copy link
Contributor Author

@tobinus tobinus Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to call this variable preserve_dummy_cbsize because

  • preserve makes it clear that we are bringing something over from the original WAV file
  • dummy communicates that this only applies to cbSize when it is not strictly needed, so this has no bearing on cbSize when is_waveformatextensible is true

Some possible alternatives:

  • keep_dummy_cbsize using the same verb as the CLI flag, --keep-foreign-metadata
  • preserve_redundant_cbsize avoiding the word dummy
  • is_waveformatex using the same naming convention as the existing is_waveformatextensible variable (referencing WAVEFORMATEX), with the downside of being a prefix of the existing variable, so developers would likely get the wrong variable when using autocomplete

foreign_metadata_t *fm = decoder_session->foreign_metadata;
size_t i;

Expand Down Expand Up @@ -772,22 +776,26 @@ FLAC__bool write_iff_headers(FILE *f, DecoderSession *decoder_session, FLAC__uin
if(i != fm->format_block && i != fm->audio_block)
foreign_metadata_size += fm->blocks[i].size;
}
/* preserve dummy cbSize = 0 at the end of the fmt chunk if the encoded file's fmt chunk's size */
/* indicates that it includes cbSize and nothing more */
if(fm->blocks[fm->format_block].size == calculate_total_riff_wave_fmt_chunk_size(format, /*is_waveformatextensible=*/false, /*preserve_dummy_cbsize=*/true))
preserve_dummy_cbsize = true;
}

if(samples == 0)
iff_size = 0;
else if(format == FORMAT_WAVE || format == FORMAT_RF64)
/* 4 for WAVE form bytes */
/* +{36,0} for ds64 chunk */
/* +8+{40,16} for fmt chunk header and body */
/* +fmt chunk header and body */
/* +8 for data chunk header */
iff_size = 4 + (format==FORMAT_RF64?36:0) + 8+(is_waveformatextensible?40:16) + 8 + foreign_metadata_size + aligned_data_size;
iff_size = 4 + (format == FORMAT_RF64 ? 36 : 0) + calculate_total_riff_wave_fmt_chunk_size(format, is_waveformatextensible, preserve_dummy_cbsize) + 8 + foreign_metadata_size + aligned_data_size;
else if(format == FORMAT_WAVE64)
/* 16+8 for RIFF GUID and size field */
/* +16 for WAVE GUID */
/* +16+8+{40,16} for fmt chunk header (GUID and size field) and body */
/* +fmt chunk header and body */
/* +16+8 for data chunk header (GUID and size field) */
iff_size = 16+8 + 16 + 16+8+(is_waveformatextensible?40:16) + 16+8 + foreign_metadata_size + aligned_data_size;
iff_size = 16 + 8 + 16 + calculate_total_riff_wave_fmt_chunk_size(format, is_waveformatextensible, preserve_dummy_cbsize) + 16 + 8 + foreign_metadata_size + aligned_data_size;
else if(format == FORMAT_AIFF)
iff_size = 46 + foreign_metadata_size + aligned_data_size;
else /* AIFF-C */
Expand Down Expand Up @@ -867,21 +875,30 @@ FLAC__bool write_iff_headers(FILE *f, DecoderSession *decoder_session, FLAC__uin
if(format != FORMAT_WAVE64) {
if(flac__utils_fwrite("fmt ", 1, 4, f) != 4)
return false;
if(!write_little_endian_uint32(f, is_waveformatextensible? 40 : 16)) /* chunk size */
if(!write_little_endian_uint32(f, calculate_riff_wave_fmt_chunk_size_field(is_waveformatextensible, preserve_dummy_cbsize))) /* chunk size */
return false;
}
else { /* Wave64 */
/* fmt GUID 20746D66-ACF3-11D3-8CD1-00C04F8EDB8A */
if(flac__utils_fwrite("\x66\x6D\x74\x20\xF3\xAC\xD3\x11\x8C\xD1\x00\xC0\x4F\x8E\xDB\x8A", 1, 16, f) != 16)
return false;
/* chunk size (+16+8 for GUID and size fields) */
if(!write_little_endian_uint64(f, 16+8+(is_waveformatextensible?40:16)))
/* chunk size */
if(!write_little_endian_uint64(f, calculate_wave64_fmt_chunk_size_field(is_waveformatextensible, preserve_dummy_cbsize)))
return false;
}

if(!write_riff_wave_fmt_chunk_body(f, is_waveformatextensible, decoder_session->bps, decoder_session->channels, decoder_session->sample_rate, decoder_session->channel_mask))
if(!write_riff_wave_fmt_chunk_body(f, is_waveformatextensible, preserve_dummy_cbsize, decoder_session->bps, decoder_session->channels, decoder_session->sample_rate, decoder_session->channel_mask))
return false;

if(format == FORMAT_WAVE64) {
unsigned padding = ((8 - ftello(f) % 8) % 8);
Copy link
Contributor Author

@tobinus tobinus Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to get the number of bytes we need to pad with to align at 8 bytes? I had to use a modulo of the final result in order to get 0 instead of 8 when no padding is needed (checking for padding < 8 below seemed a bit counter-intuitive).

if(padding > 0) {
/* fmt chunk must be padded in order to align with 8-byte alignment */
if(flac__utils_fwrite("\x00\x00\x00\x00\x00\x00\x00", 1, padding, f) != padding)
return false;
}
}

Comment on lines +893 to +901
Copy link
Contributor Author

@tobinus tobinus Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had some logic here specific to the case where format == FORMAT_WAVE64 && !is_waveformatextensible && preserve_dummy_cbsize, since this is the only case where the size of the fmt chunk isn't a multiple of 8. However, that would create a strong coupling between this conditional and the contents of the write_riff_wave_fmt_chunk_body function, spreading the knowledge across multiple locations. I therefore rewrote this part so that we are able to add padding without knowing about what causes write_riff_wave_fmt_chunk_body to write a non-aligned fmt chunk.

Is it okay to use ftello(f) like this?

decoder_session->fm_offset2 = ftello(f);

if(fm) {
Expand Down Expand Up @@ -971,7 +988,54 @@ FLAC__bool write_iff_headers(FILE *f, DecoderSession *decoder_session, FLAC__uin
return true;
}

FLAC__bool write_riff_wave_fmt_chunk_body(FILE *f, FLAC__bool is_waveformatextensible, uint32_t bps, uint32_t channels, uint32_t sample_rate, FLAC__uint32 channel_mask)
FLAC__uint64 calculate_total_riff_wave_fmt_chunk_size(FileFormat format, FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize)
{
if(format == FORMAT_WAVE64) {
/* round up to nearest 8 bytes */
return ((calculate_wave64_fmt_chunk_size_field(is_waveformatextensible, preserve_dummy_cbsize) + 7) & (~7u));
}
else {
/* +8 for fmt chunk header */
return (8 + calculate_riff_wave_fmt_chunk_size_field(is_waveformatextensible, preserve_dummy_cbsize));
}
}

FLAC__uint64 calculate_wave64_fmt_chunk_size_field(FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize)
{
if(is_waveformatextensible)
/* +16+8 for GUID and size fields */
/* +16 for regular fmt body */
/* +2 for cbSize */
/* +22 for remaining WAVEFORMATEXTENSIBLE structure */
return (16 + 8 + 16 + 2 + 22);
else if(preserve_dummy_cbsize)
/* +16+8 for GUID and size fields */
/* +16 for regular fmt body */
/* +2 for cbSize */
return (16 + 8 + 16 + 2);
else
/* +16+8 for GUID and size fields */
/* +16 for regular fmt body */
return (16 + 8 + 16);
}

FLAC__uint64 calculate_riff_wave_fmt_chunk_size_field(FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize)
{
if(is_waveformatextensible)
/* +16 for regular fmt body */
/* +2 for cbSize */
/* +22 for remaining WAVEFORMATEXTENSIBLE structure */
return (16 + 2 + 22);
else if(preserve_dummy_cbsize)
/* +16 for regular fmt body */
/* +2 for cbSize */
return (16 + 2);
else
/* +16 for regular fmt body */
return 16;
}

FLAC__bool write_riff_wave_fmt_chunk_body(FILE *f, FLAC__bool is_waveformatextensible, FLAC__bool preserve_dummy_cbsize, uint32_t bps, uint32_t channels, uint32_t sample_rate, FLAC__uint32 channel_mask)
{
if(!write_little_endian_uint16(f, (FLAC__uint16)(is_waveformatextensible? 65534 : 1))) /* compression code */
return false;
Expand Down Expand Up @@ -1005,6 +1069,10 @@ FLAC__bool write_riff_wave_fmt_chunk_body(FILE *f, FLAC__bool is_waveformatexten
if(flac__utils_fwrite("\x01\x00\x00\x00\x00\x00\x10\x00\x80\x00\x00\xaa\x00\x38\x9b\x71", 1, 16, f) != 16)
return false;
}
else if(preserve_dummy_cbsize) {
if(!write_little_endian_uint16(f, (FLAC__uint16)0)) /* cbSize */
return false;
}

return true;
}
Expand Down
116 changes: 116 additions & 0 deletions src/test_streams/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,39 @@ static FLAC__bool generate_wackywavs(void)
return false;
}

/* WAV with cbSize = 0 at the end of the fmt chunk */
static FLAC__bool generate_wackywav_cbsize0(void)
{
FILE *f;
FLAC__byte wav_cbsize0[] = {
'R', 'I', 'F', 'F', 90, 0, 0, 0,
'W', 'A', 'V', 'E', 'j', 'u', 'n', 'k',
4, 0, 0, 0 , 'b', 'l', 'a', 'h',
'p', 'a', 'd', ' ', 4, 0, 0, 0,
'B', 'L', 'A', 'H', 'f', 'm', 't', ' ',
18, 0, 0, 0, 1, 0, 1, 0,
0x44,0xAC, 0, 0,0x88,0x58,0x01, 0,
/* cbSize */
2, 0, 16, 0, 0, 0, 'd', 'a',
't', 'a', 16, 0, 0, 0, 0, 0,
1, 0, 4, 0, 9, 0, 16, 0,
25, 0, 36, 0, 49, 0, 'p', 'a',
'd', ' ', 4, 0, 0, 0, 'b', 'l',
'a', 'h'
};

if(0 == (f = fopen("wacky_cbsize0.wav", "wb")))
return false;
if(fwrite(wav_cbsize0, 1, 98, f) < 98) {
fclose(f);
return false;
}

fclose(f);

return true;
}

static FLAC__bool write_simple_wavex_header (FILE * f, unsigned samplerate, unsigned channels, unsigned bytespersample, unsigned frames)
{
unsigned datalen = channels * bytespersample * frames ;
Expand Down Expand Up @@ -1144,6 +1177,49 @@ static FLAC__bool generate_wackywav64s(void)
return false;
}

/* WAVE64 with cbSize = 0 at the end of the fmt chunk */
static FLAC__bool generate_wackywav64_cbsize0(void)
{
FILE *f;
FLAC__byte wav_cbsize0[] = {
0x72,0x69,0x66,0x66,0x2E,0x91,0xCF,0x11, /* RIFF GUID */
0xA5,0xD6,0x28,0xDB,0x04,0xC1,0x00,0x00,
192, 0, 0, 0, 0, 0, 0, 0,
0x77,0x61,0x76,0x65,0xF3,0xAC,0xD3,0x11, /* WAVE GUID */
0x8C,0xD1,0x00,0xC0,0x4F,0x8E,0xDB,0x8A,
0x6A,0x75,0x6E,0x6B,0xF3,0xAC,0xD3,0x11, /* junk GUID */
0x8C,0xD1,0x00,0xC0,0x4F,0x8E,0xDB,0x8A,
32, 0, 0, 0 , 0, 0, 0, 0,
'b', 'l', 'a', 'h', 'b', 'l', 'a', 'h',
0x66,0x6D,0x74,0x20,0xF3,0xAC,0xD3,0x11, /* fmt GUID */
0x8C,0xD1,0x00,0xC0,0x4F,0x8E,0xDB,0x8A,
42, 0, 0, 0 , 0, 0, 0, 0,
1, 0, 1, 0,0x44,0xAC, 0, 0,
0x88,0x58,0x01, 0, 2, 0, 16, 0,
/* cbSize, p a d d i n g */
0, 0, 0, 0, 0, 0, 0, 0,
0x64,0x61,0x74,0x61,0xF3,0xAC,0xD3,0x11, /* data GUID */
0x8C,0xD1,0x00,0xC0,0x4F,0x8E,0xDB,0x8A,
40, 0, 0, 0 , 0, 0, 0, 0,
0, 0, 1, 0, 4, 0, 9, 0,
16, 0, 25, 0, 36, 0, 49, 0,
0x6A,0x75,0x6E,0x6B,0xF3,0xAC,0xD3,0x11, /* junk GUID */
0x8C,0xD1,0x00,0xC0,0x4F,0x8E,0xDB,0x8A,
32, 0, 0, 0 , 0, 0, 0, 0,
'b', 'l', 'a', 'h', 'b', 'l', 'a', 'h'
};
Comment on lines +1184 to +1210
Copy link
Contributor Author

@tobinus tobinus Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should note: I don't have a usecase with WAVE64 at work, only WAV/RF64. The reason I added support for WAVE64 is because it also has support for the fmt chunk, so it theoretically could have the same quirk. The test file wacky_cbsize0.w64 is therefore made by hand, I don't know if there actually exists an encoder that would write WAVE64 files like this.


if(0 == (f = fopen("wacky_cbsize0.w64", "wb")))
return false;
if(fwrite(wav_cbsize0, 1, wav_cbsize0[16], f) < wav_cbsize0[16]) {
fclose(f);
return false;
}

fclose(f);
return true;
}

static FLAC__bool generate_wackyrf64s(void)
{
FILE *f;
Expand Down Expand Up @@ -1186,6 +1262,43 @@ static FLAC__bool generate_wackyrf64s(void)
return false;
}

/* RF64 with cbSize = 0 at the end of the fmt chunk */
static FLAC__bool generate_wackyrf64_cbsize0(void)
{
FILE *f;
FLAC__byte wav_cbsize0[] = {
'R', 'F', '6', '4', 255, 255, 255, 255,
'W', 'A', 'V', 'E', 'd', 's', '6', '4',
28, 0, 0, 0, 126, 0, 0, 0,
0, 0, 0, 0, 16, 0, 0, 0,
0, 0, 0, 0, 8, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
'j', 'u', 'n', 'k',
4, 0, 0, 0, 'b', 'l', 'a', 'h',
'p', 'a', 'd', ' ', 4, 0, 0, 0,
'B', 'L', 'A', 'H', 'f', 'm', 't', ' ',
18, 0, 0, 0, 1, 0, 1, 0,
0x44,0xAC, 0, 0,0x88,0x58,0x01, 0,
/* cbSize */
2, 0, 16, 0, 0, 0, 'd', 'a',
't', 'a', 255, 255, 255, 255, 0, 0,
1, 0, 4, 0, 9, 0, 16, 0,
25, 0, 36, 0, 49, 0, 'p', 'a',
'd', ' ', 4, 0, 0, 0, 'b', 'l',
'a', 'h'
};

if(0 == (f = fopen("wacky_cbsize0.rf64", "wb")))
return false;
if(fwrite(wav_cbsize0, 1, 134, f) < 134) {
fclose(f);
return false;
}

fclose(f);
return true;
}

static FLAC__bool generate_replaygain_tone (unsigned samplerate)
{
FILE *f;
Expand Down Expand Up @@ -1378,8 +1491,11 @@ int main(int argc, char *argv[])
if(!generate_noise("noise.raw", 65536 * 8 * 3)) return 1;
if(!generate_noise("noise8m32.raw", 32)) return 1;
if(!generate_wackywavs()) return 1;
if(!generate_wackywav_cbsize0()) return 1;
if(!generate_wackywav64s()) return 1;
if(!generate_wackywav64_cbsize0()) return 1;
if(!generate_wackyrf64s()) return 1;
if(!generate_wackyrf64_cbsize0()) return 1;
if(!generate_noisy_sine()) return 1;
for(channels = 1; channels <= 8; channels *= 2) {
unsigned bits_per_sample;
Expand Down
6 changes: 6 additions & 0 deletions test/test_flac.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1203,17 +1203,23 @@ echo "Testing --keep-foreign-metadata..."

rt_test_wav wacky1.wav '--keep-foreign-metadata'
rt_test_wav wacky2.wav '--keep-foreign-metadata'
rt_test_wav wacky_cbsize0.wav '--keep-foreign-metadata'
rt_test_w64 wacky1.w64 '--keep-foreign-metadata'
rt_test_w64 wacky2.w64 '--keep-foreign-metadata'
rt_test_w64 wacky_cbsize0.w64 '--keep-foreign-metadata'
rt_test_rf64 wacky1.rf64 '--keep-foreign-metadata'
rt_test_rf64 wacky2.rf64 '--keep-foreign-metadata'
rt_test_rf64 wacky_cbsize0.rf64 '--keep-foreign-metadata'

rt_test_wav_autokf wacky1.wav '--keep-foreign-metadata'
rt_test_wav_autokf wacky2.wav '--keep-foreign-metadata'
rt_test_wav_autokf wacky_cbsize0.wav '--keep-foreign-metadata'
rt_test_w64_autokf wacky1.w64 '--keep-foreign-metadata'
rt_test_w64_autokf wacky2.w64 '--keep-foreign-metadata'
rt_test_w64_autokf wacky_cbsize0.w64 '--keep-foreign-metadata'
rt_test_rf64_autokf wacky1.rf64 '--keep-foreign-metadata'
rt_test_rf64_autokf wacky2.rf64 '--keep-foreign-metadata'
rt_test_rf64_autokf wacky_cbsize0.rf64 '--keep-foreign-metadata'

testdatadir=${top_srcdir}/test/foreign-metadata-test-files

Expand Down
Loading