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

MP3: Replaygain values from LAME tag. #36

Closed
lazka opened this issue Jul 4, 2014 · 14 comments
Closed

MP3: Replaygain values from LAME tag. #36

lazka opened this issue Jul 4, 2014 · 14 comments
Labels

Comments

@lazka
Copy link
Member

@lazka lazka commented Jul 4, 2014

Originally reported by: Christoph Reiter (Bitbucket: lazka, GitHub: lazka)


From nondicti...@gmail.com on November 05, 2009 02:21:44

The following patch gives access to Replay Gain information obtained from
an mp3 file's LAME tag if present.

Usage example:
>>> import mutagen
>>> audio = mutagen.File('quiettest.mp3')
>>> audio.info.track_peak
0.040146470069885254
>>> audio.info.track_gain
23.400000000000002

Original issue: http://code.google.com/p/mutagen/issues/detail?id=36


@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on November 04, 2009 19:51:14

Summary: MP3: Replaygain values from LAME tag.
Labels: NeedsTest

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on November 06, 2009 07:24:50

Another patch, final one.

The test has been merged into test_mp3.py.
Track peak calculation code modified yet again to address portability concerns.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on November 05, 2009 19:58:32

The following revised patch contains a test which will run provided sox, lame, and
mp3gain are present. It also contains the original patch with a minor modification.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on November 06, 2009 18:58:31

This seems like a needlessly complex test. We aren't testing ReplayGain from first
principles - we're just testing whether Mutagen can read the tags LAME writes. A
couple of test files with known values would be a lot less fragile.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on November 13, 2009 16:41:13

Another revision. This one makes it possible to differentiate whether track_gain is
meant to be 0 dB or if that data is not available, in which case the value will be None.

Attachment: mutagen-lamereplaygain-1.18-rev5.patch

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on November 09, 2009 15:42:48

Another patch. This time the test is much simpler, however, during testing I
discovered an apparent bug in LAME where encoding a 64kbps mono file with 22050Hz
sample rate would create a header frame that was one byte too small. This resulted in
me having to rewrite some additional code.

The mp3 files are for the data directory.

Attachment: replaygain-1.mp3 replaygain-2.mp3

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on November 24, 2009 16:02:15

Status: Accepted
Labels: -NeedsTest

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on January 08, 2010 11:01:17

Rather than split apart methods of MPEGInfo, I have instead rewritten it entirely
from scratch.

The patch is against rev 73 and despite the rewrite incorporates the fix in issue 46 .

A couple of unit tests had to be altered due to the way the patched version reports
the lengths of cropped files with VBR headers, and also the precise way in which VBR
headers are detected by the patched version.

Attachment: mutagen-newmpeginfo.patch

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on December 12, 2009 20:14:06

Reading this patch now, it's extremely branchy. I think to get this logic in, the RG
parsing (and maybe the LAME/Xing/VBRI parsing in general) will need to be split into
a separate function, and not have so many possible execution paths.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on January 08, 2010 12:29:50

A dict subclass was used for MPEGFrame so that MPEGInfo could pull out the values to
be made public with a one liner.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From joe.wreschnig@gmail.com on January 08, 2010 11:46:30

I'm very nervous about a rewrite of this code, especially one that removes all the
docstrings and reference links.

I do not see the benefit of using a dict subclass to store frame information, since
frame attributes are not at all freeform.
@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From nondicti...@gmail.com on January 22, 2010 13:32:41

Comments taken on board. Not sure how I can reassure though.

In addition this patch also reveals the VBR status which would close another issue if
adopted.

Attachment: mutagen-rev77-newmpeginfo-rev2.patch

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Jul 4, 2014

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


From Vitos.La...@gmail.com on January 22, 2013 23:24:47

Hi everyone, here's another take on the same subject, namely to give (read-only) access to LAME's ReplayGain and peak tags in MP3s. This patch does not do anything else, I tried to keep it as simple as possible. I also added a simple test case and an accompanying test file.

Attachment: mutagen_lame_rg_v1.patch silence-44-s-peak.mp3

@lazka

This comment has been minimized.

Copy link
Member Author

@lazka lazka commented Aug 17, 2015

Original comment by Christoph Reiter (Bitbucket: lazka, GitHub: lazka):


mp3: expose lame replaygain values. Fixes #36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.