moumar / ruby-mp3info

ruby-mp3info read low-level informations and manipulate tags on mp3 files.
http://rdoc.info/github/moumar/ruby-mp3info/master/frames
223 stars 57 forks source link

Improved the formula for frame size. #53

Closed ekaminsky closed 9 years ago

ekaminsky commented 9 years ago

Old code interpreted this frame header "fff38064" (or "11111111111100111000000001100100") as:

{:layer=>3, :bitrate=>64, :samplerate=>22050, :mpeg_version=>2, :padding=>false, :size=>417, :error_protection=>false, :private=>true, :mode_extension=>2, :copyright=>false, :original=>true, :emphasis=>0, :channel_num=>1, :channel_mode=>"JStereo"}

New code interprets the same frame header as:

{:layer=>3, :bitrate=>64, :samplerate=>22050, :mpeg_version=>2, :padding=>false, :size=>208, :error_protection=>false, :private=>true, :mode_extension=>2, :copyright=>false, :original=>true, :emphasis=>0, :channel_num=>1, :channel_mode=>"JStereo"}

They differ in opinions on the size, which is supposed to represent the number of bytes in the frame.

The new code's interpretation of the number of bytes, 208, is the correct number of bytes for the first frame, as seen in the first three frames of this mp3:

fff380640000000000000000000000000000000000496e666f0000000f000298a3021e9052000305070a0d0f1214171a1c1e212426292b2e313335393b3d404345474a4d4f5254575a5c5e616466696b6e717375787b7d808285878a8c8f9294969a9c9ea1a4a6a9abaeb1b3b5b8bbbdc0c2c5c7cacccfd2d4d6dadcdee1e4e6e9ebeef1f3f5f8fbfd000000394c414d45332e393972016e000000002e03000014402404512e000040021e9052af36afad00000000000000000000000000000000000000000000000000000000000000fff3806400000001a40000000000000348000000004c414d45332e39392e33000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000fff3826400000001a40000000000000348000000004c414d45332e39392e3300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

The second frame begins at byte 208 in the above mp3 (char 416 and 417, represent byte 208, "ff"). So the new code's interpretation of the size of the first frame is correct.

Note that the old code's interpretation of the number of bytes in the first frame was 417, which actually skips the second frame of the above mp3 entirely (char 832 and 833 represent byte 417, "ff"). That's the beginning of the third frame.

This issue probably affects only files with either {:layer=>3, :mpeg_version=>2} or {:layer=>3, :mpeg_version=>2.5}.

joseph commented 9 years ago

A bit of background: prior to this PR, we were seeing a number of CBR mp3s misidentified as VBR due to fractional differences between the average bitrate of the first 100 frames and the declared bitrate. With $DEBUG on, these files generated unsynced frame warnings during ruby-mp3info processing. Other tools, like the Unix mp3info, correctly reported these files as CBR.

Thanks for maintaining this lib!

moumar commented 9 years ago

thanks a lot for this patch. Just merged it and wrote a test for it.

ekaminsky commented 9 years ago

Awesome. Thanks!