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

RIFF/WAVE support, using ID3v2 #321

Closed
wants to merge 37 commits into from

Conversation

Borewit
Copy link
Contributor

@Borewit Borewit commented Aug 17, 2017

Initial implementation of RIFF/WAVE format.

Currently using ID3v2 tag chunk to store metadata.

Storing metadata in the RIFF/INFO tag is still something which needs to be done.

Related issues:

mutagen/wave.py Outdated
except KeyError as e:
raise error(str(e))

data = waveFormatChunk.read()
Copy link
Member

Choose a reason for hiding this comment

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

check if 16 bytes are read so the unpack() below doesn't fail for truncated files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mutagen/wave.py Outdated
self.bitrate = self.channels * block_align * self.sample_rate

try:
waveDataChunk = waveFile[u'data']
Copy link
Member

Choose a reason for hiding this comment

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

Does checking for "data" add anything useful?

Copy link
Contributor Author

@Borewit Borewit Aug 21, 2017

Choose a reason for hiding this comment

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

It is not checking, it is accessing the data chunk length to calculate the duration (length in seconds) of the audio. I put comment on top of it; removed throwing exception in following line with assigning 0 to number_of_samples.

Copy link
Member

Choose a reason for hiding this comment

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

ah, right, I missed that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

waveDataChunk = waveFile[u'data']
except KeyError as e:
raise error(str(e))

Copy link
Member

Choose a reason for hiding this comment

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

Check that block_align and self.sample_rate > 0 or we get a division by zero

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mutagen/wave.py Outdated
"""Completely removes the ID3 chunk from the AIFF file"""

try:
del RiffFile(filething.fileobj)[u'ID3']
Copy link
Member

Choose a reason for hiding this comment

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

Is "ID3" correct here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the ID3v2 chunk should be considered as a multi purpose chunk (like RIFF and LIST) it should be in capitals everywhere. But that is a different discussion. ✔

mutagen/_riff.py Outdated
raise KeyError("First chunk should be a RIFF chunk.")

# Read the RIFF file Type
self.fileType = fileobj.read(4).decode('ascii')
Copy link
Member

Choose a reason for hiding this comment

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

decode can raise

mutagen/_riff.py Outdated

# Calculate the location of the next chunk,
# considering the pad byte
# self.__next_offset += self.__next_offset % 2
Copy link
Member

Choose a reason for hiding this comment

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

What's this about?

mutagen/wave.py Outdated
# looks like this is failing if python is not started with -bb in TravisCI:
# assert isinstance(id, text_type)

return ((len(id) <= 4) and (min(id) >= u' ') and
Copy link
Member

Choose a reason for hiding this comment

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

!=4

Copy link
Contributor Author

@Borewit Borewit Aug 21, 2017

Choose a reason for hiding this comment

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

That has been derived from aiff.py

If the chunks are indexed including the space padding, the key should be 4 bytes long indeed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have any idea why assert isinstance(id, text_type) is failing in some circumstances?
What does option -bb do?

Copy link
Member

Choose a reason for hiding this comment

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

-bb makes text_type(bytes()) fail (and comparisons between bytes/text iirc)

mutagen/wave.py Outdated

def assert_valid_chunk_id(id):
if not is_valid_chunk_id(id):
raise KeyError("RIFF/WAVE-chunk-Id must be four ASCII characters.")
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: I'd prefer ValueError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to: _riff.py

mutagen/_riff.py Outdated
def is_valid_chunk_id(id):
assert isinstance(id, text_type)

return ((len(id) <= 4) and (min(id) >= u' ') and
Copy link
Member

Choose a reason for hiding this comment

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

!= 4?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The entire check is odd, why only the first character for the lower limit and the last char for the upper limit.

if len(id) != 4:
       return False

    for i in range(0, 3):
        if id[i] < u' ' or id[i] > u'~':
            return False

    return True

tests/test_wave.py Show resolved Hide resolved
mutagen/wave.py Outdated
self.tags.filename = self.filename

fileobj.seek(0, 0)
self.info = WaveStreamInfo(fileobj)
Copy link
Member

Choose a reason for hiding this comment

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

Can we load WaveStreamInfo() before the ID3 tags? Might make it fail earlier in case of a wrong format.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. ✔

mutagen/wave.py Outdated

if 'id3 ' in waveFile:
waveFile['id3 '].delete()
if 'ID3 ' in waveFile:
Copy link
Member

Choose a reason for hiding this comment

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

Is that needed with the lower() stuff in WaveFile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

self._update_size(new_data_size)


class WaveFile(RiffFile):
Copy link
Member

Choose a reason for hiding this comment

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

Why does this inherit from RiffFile if it reimplements everything?

Copy link
Contributor Author

@Borewit Borewit Aug 21, 2017

Choose a reason for hiding this comment

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

I agree it is messy.

I started of with a copy of aiff.py because I knew it had a pretty simular structure.

The idea is to move everything what is part of the RIFF standard into _riff.py, and WAVE specific stuff into wave.py. Which was easier said then done, because is was not always clear to me what belonged to core RIFF and what to MS-WAVE. Because the sub-chunks used a little endian size, in contradiction to the RIFF chuck using big endian, I figured the sub-chunk definition belonged to WAV spec. Looks like I was wrong. I found somewhere that chunks with capital tag-id's are common purpose chunks and lower case are RIFF-type specific.

Anyway, I will do my best do clean up the mess.

Borewit added a commit to Borewit/mutagen that referenced this pull request Aug 26, 2017
Ref:
- quodlibet#321 (comment)
- a5c3785ed8d6a35868bc169f07e40e889087fd2e
Borewit added a commit to Borewit/mutagen that referenced this pull request Aug 26, 2017
@Borewit
Copy link
Contributor Author

Borewit commented Sep 6, 2017

Something is going wrong in those generic unit tests, I have difficulties understanding how this testing works. I see the constructor is called multiple times with one of the test files, and at a certain point it does not seem to recognize the file anymore as a RIFF/WAVE.

RIFF/WAVE_WaveID3(ID3)RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav
Falsifying example: run(t=<test___init__._TestFileObj at 0x2d8d39f64a8>)
RIFF/WAVE __init__:  C:\Users\Borewit\code\github\mutagen\tests\data\silence-2s-PCM-16000-08-ID3v23.wav

Ran 1 test in 8.758s

FAILED (errors=1)

Error
Traceback (most recent call last):
  File "C:\Program Files\Python36\lib\unittest\case.py", line 59, in testPartExecutor
    yield
  File "C:\Program Files\Python36\lib\unittest\case.py", line 601, in run
    testMethod()
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 539, in test_mock_fileobj
    run()
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 533, in run
    def run(t):
  File "C:\Program Files\Python36\lib\site-packages\hypothesis\core.py", line 726, in wrapped_test
    state.run()
  File "C:\Program Files\Python36\lib\site-packages\hypothesis\core.py", line 619, in run
    print_example=True, is_final=True
  File "C:\Program Files\Python36\lib\site-packages\hypothesis\executors.py", line 58, in default_new_style_executor
    return function(data)
  File "C:\Program Files\Python36\lib\site-packages\hypothesis\core.py", line 115, in run
    return test(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 535, in run
    File(t)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 138, in wrapper_func
    return func(h, *args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_file.py", line 298, in File
    return Kind(fileobj, filename=filething.filename)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_file.py", line 49, in __init__
    self.load(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 159, in wrapper
    return func(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 130, in wrapper
    return func(self, h, *args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\wave.py", line 206, in load
    self.tags = _WaveID3(fileobj, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\id3\_file.py", line 77, in __init__
    super(ID3, self).__init__(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\id3\_tags.py", line 177, in __init__
    super(ID3Tags, self).__init__(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 519, in __init__
    super(DictProxy, self).__init__(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_tags.py", line 111, in __init__
    self.load(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 159, in wrapper
    return func(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 130, in wrapper
    return func(self, h, *args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\id3\_file.py", line 147, in load
    self._pre_load_header(fileobj)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\wave.py", line 107, in _pre_load_header
    fileobj.seek(WaveFile(fileobj)[u'id3 '].data_offset)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\wave.py", line 34, in __init__
    raise ValueError("Expected RIFF/WAVE.")
ValueError: Expected RIFF/WAVE.

@lazka
Copy link
Member

lazka commented Sep 7, 2017

Something is going wrong in those generic unit tests, I have difficulties understanding how this testing works. I see the constructor is called multiple times with one of the test files, and at a certain point it does not seem to recognize the file anymore as a RIFF/WAVE.

Yeah, it gets called with variations of truncated files and expects a subclass of MutagenError to be raised or to succeed. This for uncovering wrong/missing error handling in the parsing code.

In your example traceback: load() should not raise ValueError for invalid files.

@Borewit
Copy link
Contributor Author

Borewit commented Mar 1, 2018

If I have this failing test case:

Falsifying example: run(t=<test___init__._TestFileObj at 0x25595f0ac18>)

You can reproduce this example by temporarily adding @reproduce_failure('3.44.26', b'AAEA') as a decorator on your test case

Error
Traceback (most recent call last):
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\unittest\case.py", line 59, in testPartExecutor
    yield
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\unittest\case.py", line 605, in run
    testMethod()
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 376, in test_test_fileobj_delete
    run()
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 369, in run
    h, lambda t: o.delete(fileobj=t)))
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\site-packages\hypothesis\core.py", line 1002, in wrapped_test
    state.run()
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\site-packages\hypothesis\core.py", line 818, in run
    falsifying_example.__expected_traceback,
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\site-packages\hypothesis\core.py", line 579, in execute
    result = self.test_runner(data, run)
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\site-packages\hypothesis\executors.py", line 58, in default_new_style_executor
    return function(data)
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\site-packages\hypothesis\core.py", line 571, in run
    return test(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 369, in run
    h, lambda t: o.delete(fileobj=t)))
  File "C:\Users\Borewit\AppData\Local\Programs\Python\Python36\lib\site-packages\hypothesis\core.py", line 518, in test
    result = self.test(*args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 372, in run
    o.delete(fileobj=t)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 140, in wrapper
    return func(self, h, *args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_file.py", line 120, in delete
    return self.tags.delete(filething)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_util.py", line 140, in wrapper
    return func(self, h, *args, **kwargs)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\wave.py", line 143, in delete
    waveFile = WaveFile(fileobj)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\wave.py", line 35, in __init__
    RiffFile.__init__(self, fileobj)
  File "C:\Users\Borewit\code\github\mutagen\mutagen\_riff.py", line 140, in __init__
    fileobj.seek(0)
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 254, in seek
    self._check_fail()
  File "C:\Users\Borewit\code\github\mutagen\tests\test___init__.py", line 216, in _check_fail
    raise IOError("fail")
OSError: fail

How can I execute just this test case?

It is stating @reproduce_failure('3.44.26', b'AAEA') as a decorator on your test case, I tried to add this to a few places, but was not able to isolate the test case.

@Borewit
Copy link
Contributor Author

Borewit commented Apr 18, 2018

@lazka Can you please can give some guidance?

phw pushed a commit to phw/mutagen that referenced this pull request Sep 11, 2018
Ref:
- quodlibet#321 (comment)
- a5c3785ed8d6a35868bc169f07e40e889087fd2e
@phw
Copy link
Collaborator

phw commented Sep 11, 2018

@Borewit I tested your code, rebased it against master and fixed the tests at https://github.com/phw/mutagen/commits/feature/riff-wave

I also have a local patch for Picard to use this and tested this with Picard, seems to work just fine.

You can either update your PR here or I can open a new PR, whatever you prefer.

regarding the code itself I'm not so much into mutagen's internals. What worries me a bit is how the code in _riff.py duplicates a lot of the code for IFF in aiff.py. But I'm also not enough into these formats to say how much this separation is warranted, all I know is that both AIFF and RIFF are based on IFF. @lazka What are your thoughts?

@Borewit
Copy link
Contributor Author

Borewit commented Sep 11, 2018

@phw thank you so much for picking up this work!

It would be fantastic if the functionality ends up in Picard. Due to a lack of experience programming in Python and knowledge of Mutagen I got stuck and had to give up.

AIFF and RIFF are somewhat simular. For example the size in AIFF chunk is big endian, in RIFF the size is encoded in little endian.

I will include your changes in my PR. I created PR on my own PR. I haved added you as collaberator, which should give you the freedom to push changes on top of my PR.

You don't hurt my feelings if you PR my changes directly (I have done that one time, and folks became pissed of, as if I comitted some plagiarism. I never understood that).

@phw
Copy link
Collaborator

phw commented Sep 12, 2018

Thanks for including my changes. Looks like we still have some issues with Python 2, I had not tested this locally. I will see whether I can provide fixes for this later this week, and if so will directly push it to this PR.

You don't hurt my feelings if you PR my changes directly (I have done that one time, and folks became pissed of, as if I comitted some plagiarism. I never understood that).

Yes, I also don't mind if people put their corrections on top of my PRs (as long as they leave my authorship intact and don't claim it to be their own work). But I know people sometimes take issue on this hence I rather ask before doing anything :)

@phw
Copy link
Collaborator

phw commented Sep 12, 2018

This was easier than expected, just a few instances where for Python 2 a proper unicode string is expected. Builds are running and so far it is looking good :)

@Borewit
Copy link
Contributor Author

Borewit commented Sep 12, 2018

Looks like @phw nailed the last issues.

@lazka Can you please review?

@lazka
Copy link
Member

lazka commented Sep 12, 2018

regarding the code itself I'm not so much into mutagen's internals. What worries me a bit is how the code in _riff.py duplicates a lot of the code for IFF in aiff.py. But I'm also not enough into these formats to say how much this separation is warranted, all I know is that both AIFF and RIFF are based on IFF. @lazka What are your thoughts?

I don't mind the duplication.

@lazka Can you please review?

ok

@Borewit
Copy link
Contributor Author

Borewit commented Sep 12, 2018

Thanks @lazka.

@phw, regarding:

I also have a local patch for Picard to use this and tested this with Picard, seems to work just fine.

I do as well, online or offline, have to dig it up. Can we allign the changes for a PR on Picard?
Related Picard issue.

@phw
Copy link
Collaborator

phw commented Sep 13, 2018

I do as well, online or offline, have to dig it up. Can we allign the changes for a PR on Picard?

Yes please :) I have my proposed changes linked in the Picard ticket.

mutagen/_riff.py Outdated
raise ValueError("Invalid RIFF-chunk-ID.")


class RiffChunkHeader():
Copy link
Member

Choose a reason for hiding this comment

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

inherit from object

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed

mutagen/aiff.py Outdated
@@ -44,9 +44,6 @@ def is_valid_chunk_id(id):


def assert_valid_chunk_id(id):

assert isinstance(id, text_type)
Copy link
Member

Choose a reason for hiding this comment

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

why is this removed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This was readded

mutagen/wave.py Outdated
try:
waveDataChunk = waveFile[u'data']
self.number_of_samples = waveDataChunk.data_size / block_align
except KeyError:
Copy link
Member

Choose a reason for hiding this comment

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

ZeroDivisionError

mutagen/wave.py Outdated
sample_size (`int`): The audio sample size
"""

length = 0
Copy link
Member

Choose a reason for hiding this comment

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

0.0

mutagen/wave.py Outdated

try:
self.info = WaveStreamInfo(fileobj)
except ValueError as e:
Copy link
Member

Choose a reason for hiding this comment

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

Where is ValueError raised? Is this needed?

"""WAVE stream parsing errors."""


class WaveFile(RiffFile):
Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this to "_WaveFile"?

mutagen/_riff.py Outdated
raise KeyError(
"%r has no %r chunk" % (self._fileobj, id_))

def __delitem__(self, id_):
Copy link
Member

Choose a reason for hiding this comment

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

I know aiff has the same code, but can you change this to delete_chunk(). It's confusing that del foo['id'] writes to the file and can raise IOError.

mutagen/wave.py Outdated
"""Completely removes the ID3 chunk from the RIFF file"""

try:
del RiffFile(filething.fileobj)[u'id3']
Copy link
Member

Choose a reason for hiding this comment

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

change to delete_chunk(), see above

mutagen/wave.py Outdated
RiffFile.__init__(self, fileobj)

if self.fileType != u'WAVE':
raise MutagenError("Expected RIFF/WAVE.")
Copy link
Member

Choose a reason for hiding this comment

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

MutagenError -> error

Borewit added a commit to Borewit/mutagen that referenced this pull request Sep 2, 2019
Ref:
- quodlibet#321 (comment)
- a5c3785ed8d6a35868bc169f07e40e889087fd2e
@Borewit Borewit force-pushed the feature/riff-wave branch 4 times, most recently from 5b5f636 to 6ff97bf Compare September 2, 2019 19:29
@Borewit
Copy link
Contributor Author

Borewit commented Oct 15, 2019

Thank you so much for pushing this effort forward @phw!

@Borewit
Copy link
Contributor Author

Borewit commented Oct 15, 2019

If you check out your branch please be careful.

Copy that, I will not overwrite it.

@phw
Copy link
Collaborator

phw commented Oct 19, 2019

I looked at the issue reported in #392 in detail, and examined the file structure exactly. Two results:

  1. The issue is likely a foobar2000 bug, I reported it there
  2. The RIFF / WAVE implementation here is very solid. I double checked all the code that adds and removes chunk, especially the file calculation, and checked the actual file structure after every action. All working as intended. Not even a slightly abnormal file as generated by f2k is breaking this. @Borewit great work!

@lazka I think this is good for a final review. If you want I can cleanup the git history a bit and combine some commits (but make sure to maintain authorship).

@phw
Copy link
Collaborator

phw commented Oct 21, 2019

@lazka Looks like this needs some more work, also for AIFF actually. See my comments on #392

@phw
Copy link
Collaborator

phw commented Oct 23, 2019

New PR with updated code at #408

@lazka
Copy link
Member

lazka commented May 4, 2020

See #408

@lazka lazka closed this May 4, 2020
@phw phw deleted the feature/riff-wave branch February 14, 2023 09:41
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.

None yet

3 participants