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.

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

Jump to bottom

WAVE / IFF: Accurate file offsets and new validation checks #468

Merged
merged 13 commits into from
Dec 10, 2019
Merged

WAVE / IFF: Accurate file offsets and new validation checks #468

merged 13 commits into from
Dec 10, 2019

Conversation

david-russo
Copy link
Member

@david-russo david-russo commented Aug 19, 2019 •
edited
Loading

Core:

  • In certain circumstances submessages were being dropped when creating JhoveMessages.

IFF (AIFF & WAVE):

  • Check that chunk IDs only consist of characters in the printable ASCII range.
  • Check that spaces do not precede printable characters in chunk IDs.
  • Clarified error messages and improved offset reporting accuracy.

WAVE:

  • Added reporting for unrecognized data in the top-level RIFF structure.
  • Made the Table Length field of ds64 chunks optional to better align with the specification.
  • Reinstated WAVE-HUL-4 reporting which had been lost during refactoring.
  • Corrected WAVE-HUL-15 from an Error to an Informational message.
  • Retired WAVE-HUL-16, an unused duplicate of WAVE-HUL-19.
  • Clarified error messages and greatly improved offset reporting accuracy.
  • Moved all remaining message text into translatable resource files.
  • Added test files to demonstrate new validation checks.

@david-russo
IFF: Improve validation for chunk IDs
be45549 
Values above the printable ASCII range no longer validate, nor IDs in
which spaces precede printable characters, per the IFF, AIFF, and RIFF
specifications.
@david-russo
CORE: Fix dropping of submessage information
cd18991
@david-russo
WAVE: Improve error reporting and accuracy
decc8f2 
- Improved offset reporting
- Reinstated WAVE-HUL-4 reporting
- Made more strings translatable
- Included reporting of unrecognized data in the
  top-most RIFF chunk as already existed for sub-chunks
@david-russo
WAVE: Replace manual tracking of remaining bytes
8bd24df
@david-russo david-russo force-pushed the wave-ids-and-offsets branch from 08ac3bb to 4c7500a Compare August 19, 2019 17:29
@david-russo david-russo force-pushed the wave-ids-and-offsets branch from 4c7500a to 8a112da Compare August 27, 2019 09:07
@david-russo david-russo force-pushed the wave-ids-and-offsets branch from 8a112da to f80d099 Compare August 27, 2019 09:33
@david-russo
WAVE: Allow DS64 Table Length field to be optional
d03cd53 
Also stripped example wave files of unnecessary data.
@david-russo
WAVE: Demote WAVE-HUL-15 from Error to Informational
1b8b4a7 
Like unrecognized chunks, unrecognized list types are allowed but
should be skipped, so this message is now informational. This change
also allows file processing to continue after finding an unknown list,
instead of aborting prematurely.
@david-russo
IFF: Consolidate chunk properties into Chunk class
24ac639 
This moves constants for chunk properties such as ID and size field
lengths into the Chunk class, and also adds chunk offset information
for all chunks.
@carlwilson carlwilson added this to the v1.24-m4 Release milestone Oct 18, 2019
@carlwilson carlwilson added bug A product defect that needs fixing P2 Medium priority issues to be scheduled in a future release labels Oct 18, 2019
@carlwilson carlwilson self-requested a review October 18, 2019 01:02
david-russo and others added 4 commits October 18, 2019 10:53
@david-russo
IFF & WAVE: Move remaining strings into resource files
f9cada9 
- Retired WAVE-HUL-16 which was a part of previously pruned dead code.
- All chunk names are now surrounded by quotation marks to improve
  visibility of leading and trailing spaces and have been moved into
  translatable resource files.
- Improved accuracy of WAVE messages.
@carlwilson
Merge branch 'integration' into wave-ids-and-offsets
e751e5d
@carlwilson
Merge branch 'integration' into wave-ids-and-offsets
4c3fe39
@carlwilson
Merge branch 'integration' into wave-ids-and-offsets
508f2a6
carlwilson
carlwilson reviewed Oct 22, 2019
View reviewed changes
Copy link
Member

@carlwilson carlwilson left a comment

Choose a reason for hiding this comment

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

Nice work :) Changes were sensible and easy to follow the logic.

Will add a PR to fix the tests but that will be a brute force copy of the WAV file results as the PR changes the WAV output in almost all cases. Need to bump the module version also.

@david-russo
Copy link
Member Author

david-russo commented Oct 23, 2019 •
edited
Loading

Thank you. Yeah, the version numbers should be bumped for WAVE and AIFF.

@carlwilson carlwilson mentioned this pull request Dec 10, 2019
Merged
@carlwilson carlwilson merged commit 64bac14 into openpreserve:integration Dec 10, 2019
@david-russo david-russo deleted the wave-ids-and-offsets branch December 16, 2019 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlwilson carlwilson carlwilson left review comments

Assignees

@david-russo david-russo

Labels
bug A product defect that needs fixing P2 Medium priority issues to be scheduled in a future release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@david-russo @carlwilson