Skip to content
This repository has been archived by the owner on Jul 28, 2021. It is now read-only.

Convert audio names to md5 digest - WIP #72

Closed
wants to merge 0 commits into from

Conversation

akilegaspi
Copy link

@akilegaspi akilegaspi commented Jan 7, 2019

Description: Extracts the base name of the audio file and transforms the base name into an md5 digest

This pull requests fixes #25, by @owickstrom

Make sure to describe (where applicable):

  • Added pureMD5-2.1.3 Package
  • Included bytestring Package

Checklist:

Please make sure to check the following items (that are applicable.)

  • Doesn't duplicate any existing PR
  • Automated tests added

How to test:

TODO: Testing instructions for reviewer.

@akilegaspi
Copy link
Author

Still have to create automated tests for this one also

Copy link
Owner

@owickstrom owickstrom left a comment

Choose a reason for hiding this comment

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

Thanks for submitting a PR! Some comments on the changes:

  • We should hash the audio file contents to calculate audio chunk file names
    • I'd recommend using cryptonite and its hashlazy function together with a lazy ByteString read
  • The non-classifying copy should also copy to a hashed name:
    let assetPath = outDir </> takeFileName srcFile

Regarding tests, it would be very nice if you'd add something! 👏

Thanks!

@akilegaspi akilegaspi changed the title Convert audio names to md5 digest Convert audio names to md5 digest - WIP Jan 14, 2019
@akilegaspi
Copy link
Author

Sorry, I haven't been able to checkout the PR for awhile now but ok, so if I understood it correctly, we use the contents of the file to create a hashname for the audio chunk.

Also what do you mean by "non-classifying copy?"

@akilegaspi
Copy link
Author

akilegaspi commented Jan 14, 2019

@owickstrom In accordance with #76 , I think it would be better if we created a helper module for generating a hash given Cipher -> Lazy.ByteString -> String since I think it's already given that we both use cryptonite and bytestring packages.

With that being said, it would be easier for us to create tests (using QuickCheck maybe?) for our respective Cipher also.

What do you think?

@Cmdv
Copy link
Contributor

Cmdv commented Jan 14, 2019

@akilegaspi feel free to help out with the helper function, I started making something but I'm using the file name to do the hashing when I think from comments we need to use the file content?

https://github.com/owickstrom/komposition/pull/76/files#diff-c0ab36ff8cac8d74b3774e1a4cb62225

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Content-addressable directory name for classified audio parts
3 participants