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

Adds support for SoundCheck #97

Merged
merged 2 commits into from Feb 7, 2013

Conversation

Projects
None yet
2 participants
@daveisadork
Collaborator

daveisadork commented Feb 6, 2013

This works, but it's probably a bit messy. You will probably want to refactor it somehow. It's working well for me, but it could definitely use some more testing.

  • Adds 2 functions to convert between SoundCheck and ReplayGain
  • Adds relevant StorageStyle to rg_track_* for SoundCheck
  • Adds a new packing type for SoundCheck
  • Modifies StorageStyle to accept a lang argument
  • Modifies StorageStyle to allow specifying Packed out_type
  • Modifies MediaField to write COMM frames with a lang attribute
Adds support for SoundCheck
 * Adds 2 functions to convert between SoundCheck and ReplayGain
 * Adds relevant StorageStyle to rg_track_* for SoundCheck
 * Adds a new packing type for SoundCheck
 * Modifies StorageStyle to accept a lang argument
 * Modifies StorageStyle to allow specifying Packed out_type
 * Modifies MediaField to write COMM frames with a lang attribute
Show outdated Hide outdated beets/mediafile.py
@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Feb 7, 2013

Member

This all looks great, @daveisadork. I like the approach of expanding the packing styles to include the new binary format.

It looks like we might need some more error-handling code in the SC-to-RG and vice-versa code. The tests have flagged a few cases where the log call in _sc2rg gets zero as an argument. And I imagine it might be a good idea to handle struct.error when we unpack the binary data with struct.unpack in case the data is invalid. In cases like this, I think it's fine to bail out and write nothing (or read zero).

Member

sampsyo commented Feb 7, 2013

This all looks great, @daveisadork. I like the approach of expanding the packing styles to include the new binary format.

It looks like we might need some more error-handling code in the SC-to-RG and vice-versa code. The tests have flagged a few cases where the log call in _sc2rg gets zero as an argument. And I imagine it might be a good idea to handle struct.error when we unpack the binary data with struct.unpack in case the data is invalid. In cases like this, I think it's fine to bail out and write nothing (or read zero).

Deal with unexpected or missing SoundCheck values
* Deals with gain values of 0 in SoundCheck files by interpreting them
  as 0.0dB rather than causing an exception
* Return default values if SoundCheck tag is malformed in some way
* Other misc changes for compactness

sampsyo added a commit that referenced this pull request Feb 7, 2013

@sampsyo sampsyo merged commit 91759d0 into beetbox:master Feb 7, 2013

1 check passed

default The Travis build passed
Details

sampsyo added a commit that referenced this pull request Feb 8, 2013

@sampsyo

This comment has been minimized.

Show comment
Hide comment
@sampsyo

sampsyo Feb 8, 2013

Member

Thanks for braving this task. For dealing with both this and ASF support (another ordeal that required slogging through partial documentation for proprietary formats), I'd like to award you a ⭐️ medal of beets valor ⭐️. Truly awesome.

I've just made a few little tweaks, including some refactoring on my end that removed the need for your functions to (de-)stringify the ReplayGain data.

Member

sampsyo commented Feb 8, 2013

Thanks for braving this task. For dealing with both this and ASF support (another ordeal that required slogging through partial documentation for proprietary formats), I'd like to award you a ⭐️ medal of beets valor ⭐️. Truly awesome.

I've just made a few little tweaks, including some refactoring on my end that removed the need for your functions to (de-)stringify the ReplayGain data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment