Skip to content

Commit

Permalink
Fix: #62: Error parsing Ogg: "FourCC contains invalid characters".
Browse files Browse the repository at this point in the history
Most likely caused due to incorrect Ogg frame encoding. The last page
should be marked with the last page of logical bitstream, in the
header_type_flag, which is not done.

Started to make use of debug module.
  • Loading branch information
Borewit committed Mar 3, 2018
1 parent 1e64fe3 commit 0925463
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 24 deletions.
4 changes: 1 addition & 3 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"coveralls": "npm run cover-test && nyc report --reporter=text-lcov | coveralls"
},
"dependencies": {
"debug": "^3.1.0",
"es6-promise": "^4.2.4",
"fs-extra": "^5.0.0",
"strtok3": "^1.3.3",
Expand Down
8 changes: 0 additions & 8 deletions src/common/Util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,6 @@ export default class Util {
}
}

public static sum(arr: number[]): number {
let s: number = 0;
for (const v of arr) {
s += v;
}
return s;
}

public static swapBytes(buffer: Buffer): Buffer {
const l = buffer.length;
assert.ok((l & 1) === 0, 'Buffer length must be even');
Expand Down
18 changes: 17 additions & 1 deletion src/ogg/Ogg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,18 @@ export interface IPageHeader {
streamSerialNumber: number,
pageSequenceNo: number,
pageChecksum: number,
segmentTable: number;
/**
* The number of segment entries to appear in the segment table.
* The maximum number of 255 segments (255 bytes each) sets the maximum possible physical page size at 65307 bytes or
* just under 64kB (thus we know that a header corrupted so as destroy sizing/alignment information will not cause a
* runaway bitstream. We'll read in the page according to the corrupted size information that's guaranteed to be a
* reasonable size regardless, notice the checksum mismatch, drop sync and then look for recapture).
*/
page_segments: number;
}

export interface ISegmentTable {
totalPageSize: number
}

export interface IPageConsumer {
Expand All @@ -57,6 +68,11 @@ export interface IPageConsumer {
* @param {Buffer} pageData Ogg page data
*/
parsePage(header: IPageHeader, pageData: Buffer);

/**
* Force to parse pending segments
*/
flush();
}

export interface IAudioParser extends IPageConsumer {
Expand Down
61 changes: 52 additions & 9 deletions src/ogg/OggParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,33 @@ import {FourCcToken} from "../common/FourCC";
import * as Ogg from "./Ogg";
import {OpusParser} from "../opus/OpusParser";
import * as Token from "token-types";
import * as _debug from "debug";

const debug = _debug("music-metadata/ogg");

export class SegmentTable implements Token.IGetToken<Ogg.ISegmentTable> {

private static sum(buf: number[], off: number, len: number): number {
let s: number = 0;
for (let i = off; i < off + len; ++i) {
s += buf[i];
}
return s;
}

public len: number;

constructor(header: Ogg.IPageHeader) {
this.len = header.page_segments;
}

public get(buf, off): Ogg.ISegmentTable {
return {
totalPageSize: SegmentTable.sum(buf, off, this.len)
};
}

}

/**
* Parser for Ogg logical bitstream framing
Expand Down Expand Up @@ -37,7 +64,7 @@ export class OggParser implements ITokenParser {
streamSerialNumber: Token.UINT32_LE.get(buf, off + 14),
pageSequenceNo: Token.UINT32_LE.get(buf, off + 18),
pageChecksum: Token.UINT32_LE.get(buf, off + 22),
segmentTable: buf.readUInt8(off + 26)
page_segments: buf.readUInt8(off + 26)
};
}
};
Expand All @@ -64,40 +91,56 @@ export class OggParser implements ITokenParser {
}

private parsePage(): Promise<void> {
debug("pos=%s, parsePage()", this.tokenizer.position);
return this.tokenizer.readToken<Ogg.IPageHeader>(OggParser.Header).then(header => {
if (header.capturePattern !== 'OggS') { // Capture pattern
throw new Error('expected ogg header but was not found');
}
this.header = header;

this.pageNumber = header.pageSequenceNo;
debug("page#=%s, Ogg.id=%s", header.pageSequenceNo, header.capturePattern);

return this.tokenizer.readToken<Buffer>(new Token.BufferType(header.segmentTable)).then(segments => {
const pageLength = common.sum(segments as any);
return this.tokenizer.readToken<Buffer>(new Token.BufferType(pageLength)).then(pageData => {
return this.tokenizer.readToken<Ogg.ISegmentTable>(new SegmentTable(header)).then(segmentTable => {
debug("totalPageSize=%s", segmentTable.totalPageSize);
return this.tokenizer.readToken<Buffer>(new Token.BufferType(segmentTable.totalPageSize)).then(pageData => {
debug("firstPage=%s, lastPage=%s, continued=%s", header.headerType.firstPage, header.headerType.lastPage, header.headerType.continued);
if (header.headerType.firstPage) {
const id = new Token.StringType(7, 'ascii').get(pageData, 0);
switch (id[1]) {
case 'v': // Ogg/Vorbis
debug("Set page consumer to Ogg/Vorbis ");
this.pageConsumer = new VorbisParser(this.options);
break;
case 'p':
case 'p': // Ogg/Opus
debug("Set page consumer to Ogg/Opus");
this.pageConsumer = new OpusParser(this.options);
break;
default:
throw new Error('Ogg audio-codec not recognized (id=' + id + ')');
throw new Error('gg audio-codec not recognized (id=' + id + ')');
}
}
this.pageConsumer.parsePage(header, pageData);

if (!header.headerType.lastPage) {
return this.parsePage();
}
});
});
}).catch (err => {
if (err.message !== "End-Of-File") {
throw err;
switch (err.message) {
case "End-Of-File":
break; // ignore this error

case "FourCC contains invalid characters":
if (this.pageNumber > 0) {
// ignore this error: work-around if last OGG-page is not marked with last-page flag
// ToDo: capture warning
return this.pageConsumer.flush();
}
throw err;

default:
throw err;
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/opus/OpusParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class OpusParser extends VorbisParser {
this.format.numberOfChannels = this.idHeader.channelCount;
}

protected parseFullPage(header: IPageHeader, pageData: Buffer) {
protected parseFullPage(pageData: Buffer) {
const magicSignature = new Token.StringType(8, 'ascii').get(pageData, 0);
switch (magicSignature) {
case 'OpusTags':
Expand Down
14 changes: 12 additions & 2 deletions src/vorbis/VorbisParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import {IFormat, INativeAudioMetadata, IOptions, ITag} from "../index";
import {Promise} from "es6-promise";
import * as Token from "token-types";
import * as Ogg from "../ogg/Ogg";
import * as _debug from "debug";

const debug = _debug("music-metadata/ogg/vorbis");

/**
* Vorbis 1 Parser.
Expand Down Expand Up @@ -38,7 +41,7 @@ export class VorbisParser implements Ogg.IAudioParser {
// Flush page segments
if (this.pageSegments.length > 0) {
const fullPage = Buffer.concat(this.pageSegments);
this.parseFullPage(header, fullPage);
this.parseFullPage(fullPage);
}
// Reset page segments
this.pageSegments = header.headerType.lastPage ? [] : [pageData];
Expand All @@ -49,6 +52,10 @@ export class VorbisParser implements Ogg.IAudioParser {
}
}

public flush() {
this.parseFullPage(Buffer.concat(this.pageSegments));
}

public getMetadata(): INativeAudioMetadata {
return {
format: this.format,
Expand All @@ -64,6 +71,7 @@ export class VorbisParser implements Ogg.IAudioParser {
* @param {Buffer} pageData
*/
protected parseFirstPage(header: Ogg.IPageHeader, pageData: Buffer) {
debug("Parse first page");
// Parse Vorbis common header
const commonHeader = Vorbis.CommonHeader.get(pageData, 0);
if (commonHeader.vorbis !== 'vorbis')
Expand All @@ -75,11 +83,13 @@ export class VorbisParser implements Ogg.IAudioParser {
this.format.sampleRate = idHeader.sampleRate;
this.format.bitrate = idHeader.bitrateNominal;
this.format.numberOfChannels = idHeader.channelMode;
debug("sample-rate=%s[hz], bitrate=%s[b/s], channel-mode=%s", idHeader.sampleRate, idHeader.bitrateNominal, idHeader.channelMode);
} else throw new Error('First Ogg page should be type 1: the identification header');
}

protected parseFullPage(header: Ogg.IPageHeader, pageData: Buffer) {
protected parseFullPage(pageData: Buffer) {
// New page
debug("Parse full page");
const commonHeader = Vorbis.CommonHeader.get(pageData, 0);
switch (commonHeader.packetType) {

Expand Down
Binary file added test/samples/issue_62.ogg
Binary file not shown.
20 changes: 20 additions & 0 deletions test/test-ogg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,26 @@ describe("Parsing Ogg", function() {
}).then(() => stream.close());
});
});

it("should handle page not finalized with the lastPage flag", () => {

const filePath = path.join(__dirname, 'samples', "issue_62.ogg");

return mm.parseFile(filePath, {native: true}).then(metadata => {

assert.deepEqual(metadata.format.tagTypes, ['vorbis'], 'format.tagTypes');
// ToDo? assert.strictEqual(metadata.format.duration, 2.0, 'format.duration = 2.0 sec');
assert.strictEqual(metadata.format.sampleRate, 22050, 'format.sampleRate = 44.1 kHz');
assert.strictEqual(metadata.format.numberOfChannels, 2, 'format.numberOfChannels = 2 (stereo)');
assert.strictEqual(metadata.format.bitrate, 56000, 'bitrate = 64 kbit/sec');

// Following is part a page which is not correctly finalized with lastPage flag
assert.isDefined(metadata.common.title, "should provide: metadata.common.title");
assert.equal(metadata.common.title, "Al-Fatihah", "metadata.common.title");
assert.equal(metadata.common.artist, "Mishary Alafasi - www.TvQuran.com", "metadata.common.artist");
});
});

});

describe("Parsing Ogg/Opus", () => {
Expand Down

0 comments on commit 0925463

Please sign in to comment.