Skip to content

Commit 4e5a797

Browse files
author
Benjamin Russell
authored
Fix MP3 VBR Headers (#113)
**Description**: This is basically a rewrite of the VBR header implementation for MP3 files. It offers a much more robust solution for determining duration of MP3s and best efforts at determining MP3 bitrates. **Testing**: Brand new testing of the VBR headers and MPEG audio headers --- * Code for reading VBR headers is waaaaay improved. Only thing remaining is taking into consideration encoder delays from sample count * Incorporate encoder delay/padding from LAME headers * Incorporate delay for VBRI headers * Adding sample files * Working on tests... * 100% coverage of MpegAudioHeader * Unit tests for VBRI headers * Unit tests for VBR headers * Updating MPEG header to prevent reserved bitrate/audio sample rates from being propagated * Rollback "allow zero" changes, patch up the rest of the unit tests * Removing properties tests from mp3 id3vx tests * Fix MPEG audio stream detection ALSO UNCOVER A HUGE BUG IN APE FILE INTEGRATION TESTS??? WTF * Fixing a thing after merging develop * Enabling one more test
1 parent 39a345a commit 4e5a797

21 files changed

Lines changed: 1424 additions & 1165 deletions

src/combinedTag.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,12 @@ export default abstract class CombinedTag extends Tag {
432432
if (!t) { return false; }
433433

434434
// Block falsy values (false, 0, null, undefined, NaN), allow ""
435-
const val = propertyFn(t);
436-
return val || (typeof(val) === "string" && val === "");
435+
const value = propertyFn(t);
436+
437+
// @TODO: Just return undefined if the field isn't set... this is a pain
438+
return value !== undefined
439+
&& value !== null
440+
&& !this.isDefaultValue(value, defaultValue);
437441
});
438442
return tagWithProperty ? propertyFn(tagWithProperty) : defaultValue;
439443
}
@@ -460,5 +464,13 @@ export default abstract class CombinedTag extends Tag {
460464
}
461465
}
462466

467+
private isDefaultValue<T>(value: T, defaultValue: T) {
468+
if (Number.isNaN(defaultValue) && Number.isNaN(value)) {
469+
return true;
470+
}
471+
472+
return value === defaultValue;
473+
}
474+
463475
// #endregion
464476
}

src/mpeg/mpegAudioFile.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,17 @@ export default class MpegAudioFile extends SandwichFile {
3131
}
3232

3333
// @TODO: if readStyle is higher than average, scan the entire file to accurately calculate
34-
// the duration.
34+
// the duration and bitrate
3535

36-
// Skip if we're not reading the properties
37-
this._firstHeader = MpegAudioHeader.find(this, this.mediaStartPosition, 0x4000);
36+
const searchStart = this.mediaStartPosition;
37+
const searchEnd = searchStart + 0x400;
38+
const streamLength = this.mediaEndPosition - this.mediaStartPosition;
39+
40+
this._firstHeader = MpegAudioHeader.fromFile(this, searchStart, searchEnd, streamLength);
3841
if (!this._firstHeader) {
3942
throw new CorruptFileError("MPEG audio header not found");
4043
}
4144

42-
this._firstHeader.streamLength = this.mediaEndPosition - this.mediaStartPosition;
4345
return new Properties(this._firstHeader.durationMilliseconds, [this._firstHeader]);
4446
}
4547
}

0 commit comments

Comments
 (0)