leosongwei / mutagen

Automatically exported from code.google.com/p/mutagen
GNU General Public License v2.0
0 stars 0 forks source link

Musepack: Support new Musepack SV8 API #7

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Support for the new libmcpdec API was added to Gstreamer Plugins Bad in
version 0.10.7.

To use this revised format in Mutagen-based apps it will ne necessary to
update Mutagen as well.

The main advantage of SV8 is much faster seeking and the ability to split
CD-sized tracks into it’s constituents. SV7 files can be converted to SV8.

http://trac.musepack.net/trac/wiki/SV8Specification

Original issue reported on code.google.com by towolf on 2 Jul 2009 at 12:40

GoogleCodeExporter commented 9 years ago
Wow, they made the header format really crap.

Original comment by joe.wreschnig@gmail.com on 6 Jul 2009 at 7:00

GoogleCodeExporter commented 9 years ago

Original comment by joe.wreschnig@gmail.com on 20 Jul 2009 at 2:48

GoogleCodeExporter commented 9 years ago
Patch+Testfile attached.

I made the peak calculation the same as with SV7, but I think it is wrong in 
both
cases, or QL needs to convert them somehow.. I guess you know more about this.

Original comment by reiter.christoph@gmail.com on 24 Mar 2010 at 8:21

Attachments:

GoogleCodeExporter commented 9 years ago
I just played with your patch and found a file that will cause a
MusepackHeaderError("Invalid Frame") in line 84. The attached file is a SV8 
file with
zero audio length. I have also attached the WAV source from which I generated 
the
file. The SV7 version of the same file can be read without problems.

Other files seemed to work so far.

Original comment by ph.wolfer on 12 Apr 2010 at 7:46

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks Philipp.

Updated patch and test sample attached.

Original comment by reiter.christoph@gmail.com on 13 Apr 2010 at 9:50

Attachments:

GoogleCodeExporter commented 9 years ago
Hi, it would be nice to merge this patch into the trunk :) cheers

Original comment by vlak...@gmail.com on 15 Jul 2010 at 9:10

GoogleCodeExporter commented 9 years ago
> Hi, it would be nice to merge this patch into the trunk :) cheers
another cheers on that :D

Original comment by gbi...@gmail.com on 15 Aug 2010 at 10:37

GoogleCodeExporter commented 9 years ago
For the record, this is the #1 thing in my Mutagen pile right now; 
unfortunately Mutagen is about the #-1 thing in my overall pile. I'm hoping to 
get to it by October.

Original comment by joe.wreschnig@gmail.com on 22 Aug 2010 at 8:36

GoogleCodeExporter commented 9 years ago
Sorry it's taken so long. I guess it's still technically October...

Comments on the patch:

I really don't like the "while 1". Can it be replaced with a real loop 
invariant? I'd rather just chunk frames as we go rather than repeat the 
validation logic for each frame type we care about, stopping as soon as we find 
an AP.

Do we really need to parse every frame in the file just to get the approximate 
bitrate? It's going to be very slow (and still not accurate at all). Just let 
the file length / time case take care of it.

I don't like the get_sv8_int staticmethod. This should probably be a bare 
function or part of the cdata module. I also don't see a reason why it stops 
after 9 bytes except as a sanity check - in which case it should throw an 
exception, not return 0, after failing to stop after reading 8 bytes.

If the exceptions are going to claim it's a Musepack file with an invalid 
frame, it should explain why it's an invalid frame.

I'd like someone from the Musepack team to explain why their new Replay Gain 
format is so incredibly bad. Until then, Mutagen shouldn't try to parse it.

Original comment by joe.wreschnig@gmail.com on 31 Oct 2010 at 12:20

GoogleCodeExporter commented 9 years ago
Changes:

 - get bitrate using file size; the calculation was wrong for sv4-7: bps/1000+0.5 instead of just bps
 - move sv8_int, peak/gain in functions, IOError in sv8_int after limit is reached
 - move sv8 parsing in a seperate method
 - I've checked the replaygain calculation with GStreamer and it's correct.
 - correctly ignore empty gain/peak fields (this needs to be fixed in QL).
 - include version and bitrate in pprint

Original comment by reiter.christoph@gmail.com on 19 Apr 2011 at 3:16

Attachments:

GoogleCodeExporter commented 9 years ago
Joe? Will you merge this?

Original comment by towolf on 16 Apr 2012 at 4:35

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r122.

Original comment by reiter.christoph@gmail.com on 25 Jul 2012 at 9:06