Skip to content

Commit

Permalink
DASH: Remove "native" JS parser
Browse files Browse the repository at this point in the history
From the v4.1.0, we had 3 MPD parsers:

  - The "native" one, relying on the web's DOMParser API.

    This is historically the first one we used and still the one we rely
    by default on main thread.

  - The "WebAssembly" one, added in 2021.

    This one was initially written when we had to manage multi-megabytes
    MPD. Those took forever to parse on low-end devices but worse even
    on a PC, parsing it often (e.g. for a live content) could lead to a
    crash after several hours due to GC pressure.

    The idea was to parse the MPD in another thread (the initial
    MULTI_THREAD attempts date back from here :p) and to rely on
    WebAssembly to better control memory usage and performance (also the
    "native" one wasn't usable anyway on a WebWorker due to browser
    limitations).

    It turned out that the WebAssembly version was so light (note: we
    also rely on XML StAX parsing instead of DOM parsing which may have
    helped for that part) and fast that we didn't yet need the
    complexity of bringing another thread here.

  - The "fast js" one, added at the last `v4.1.0` release.

    This one follows attempts to make the `MULTI_THREAD` feature usable
    on non-WebAssembly devices. We noticed that other developers had
    recently made attempts for fast JS parsing without even needing the
    use of the DOMParser. They reported even faster performance due to
    much fewer XML integrity checks (which is OK for us, as MPD parsing
    performance is one of the most important aspect for us).

This commit proposes that we remove the "native" one to just replace it
by the "fast js" one.

The "fast js" one has already been used in production for more than 6
months without issue, is equal-to-faster than the native one and it
would lead to much less code.
  • Loading branch information
peaBerberian committed Nov 15, 2024
1 parent 0a66b5c commit b39f2dd
Show file tree
Hide file tree
Showing 66 changed files with 99 additions and 4,119 deletions.
4 changes: 2 additions & 2 deletions src/core/main/worker/worker_main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {
IReferenceUpdateMessage,
} from "../../../multithread_types";
import { MainThreadMessageType, WorkerMessageType } from "../../../multithread_types";
import DashFastJsParser from "../../../parsers/manifest/dash/fast-js-parser";
import DashJsParser from "../../../parsers/manifest/dash/js-parser";
import DashWasmParser from "../../../parsers/manifest/dash/wasm-parser";
import { ObservationPosition } from "../../../playback_observer";
import type { IWorkerPlaybackObservation } from "../../../playback_observer/worker_playback_observer";
Expand Down Expand Up @@ -73,7 +73,7 @@ export default function initializeWorkerMain() {
// TODO allow worker-side feature-switching? Not sure how
const dashWasmParser = new DashWasmParser();
features.dashParsers.wasm = dashWasmParser;
features.dashParsers.fastJs = DashFastJsParser;
features.dashParsers.js = DashJsParser;
features.transports.dash = createDashPipelines;

/**
Expand Down
2 changes: 1 addition & 1 deletion src/features/features_object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import type { IFeaturesObject } from "./types";
* @type {Object}
*/
const features: IFeaturesObject = {
dashParsers: { wasm: null, native: null, fastJs: null },
dashParsers: { wasm: null, js: null },
createDebugElement: null,
directfile: null,
decrypt: null,
Expand Down
6 changes: 3 additions & 3 deletions src/features/list/__tests__/dash.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, it, expect } from "vitest";
import MediaSourceContentInitializer from "../../../main_thread/init/media_source_content_initializer";
import nativeDashParser from "../../../parsers/manifest/dash/native-parser";
import dashJsParser from "../../../parsers/manifest/dash/js-parser";
import DASHFeature from "../../../transports/dash";
import type { IFeaturesObject } from "../../types";
import addDASHFeature from "../dash";
Expand All @@ -9,13 +9,13 @@ describe("Features list - DASH", () => {
it("should add DASH in the current features", () => {
const featureObject = {
transports: {},
dashParsers: { fastJs: null, native: null, wasm: null },
dashParsers: { js: null, wasm: null },
mainThreadMediaSourceInit: null,
} as unknown as IFeaturesObject;
addDASHFeature(featureObject);
expect(featureObject).toEqual({
transports: { dash: DASHFeature },
dashParsers: { native: nativeDashParser, fastJs: null, wasm: null },
dashParsers: { js: dashJsParser, wasm: null },
mainThreadMediaSourceInit: MediaSourceContentInitializer,
});
expect(featureObject.transports.dash).toBe(DASHFeature);
Expand Down
5 changes: 2 additions & 3 deletions src/features/list/__tests__/dash_wasm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ describe("Features list - DASH WASM Parser", () => {

const featureObject = {
transports: {},
dashParsers: { native: null, fastJs: null, wasm: null },
dashParsers: { js: null, wasm: null },
} as unknown as IFeaturesObject;
DASH_WASM._addFeature(featureObject);
expect(featureObject.transports).toEqual({ dash: DASHFeature });
expect(featureObject.dashParsers.native).toEqual(null);
expect(featureObject.dashParsers.fastJs).toEqual(null);
expect(featureObject.dashParsers.js).toEqual(null);
expect(featureObject.dashParsers.wasm).toBeInstanceOf(DashWasmParser);
});
});
4 changes: 2 additions & 2 deletions src/features/list/dash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import MediaSourceContentInitializer from "../../main_thread/init/media_source_content_initializer";
import dashJsParser from "../../parsers/manifest/dash/native-parser";
import dashJsParser from "../../parsers/manifest/dash/js-parser";
import dash from "../../transports/dash";
import type { IFeaturesObject } from "../types";

Expand All @@ -27,7 +27,7 @@ function addDASHFeature(features: IFeaturesObject): void {
if (features.transports.dash === undefined) {
features.transports.dash = dash;
}
features.dashParsers.native = dashJsParser;
features.dashParsers.js = dashJsParser;
features.mainThreadMediaSourceInit = MediaSourceContentInitializer;
}

Expand Down
13 changes: 2 additions & 11 deletions src/features/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,7 @@ export type IHTMLTextTracksBuffer = new (
*/
export type INativeTextTracksBuffer = new (mediaElement: IMediaElement) => SegmentSink;

export type IDashNativeParser = (
dom: Document,
args: IMPDParserArguments,
) => IDashParserResponse<string>;

export type IDashFastJsParser = (
export type IDashJsParser = (
xml: string,
args: IMPDParserArguments,
) => IDashParserResponse<string>;
Expand Down Expand Up @@ -142,11 +137,7 @@ export interface IFeaturesObject {
/**
* Entirely JavaScript-based Manifest DASH parser.
*/
fastJs: IDashFastJsParser | null;
/**
* JavaScript+Browser's DOMParser-based Manifest DASH parser.
*/
native: IDashNativeParser | null;
js: IDashJsParser | null;
};
/** Implement text track rendering through `<track>` HTML elements. */
nativeTextDisplayer: typeof NativeTextDisplayer | null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, it, expect, vi } from "vitest";
import type { IManifest } from "../../../../manifest";
import type { IParsedStreamEventData } from "../../../../parsers/manifest";
import type { IStreamEventData } from "../../../../public_types";
import type IRefreshScheduledEventsList from "../stream_events_emitter/refresh_scheduled_events_list";
import type {
Expand All @@ -9,28 +10,48 @@ import type {

describe("init - refreshScheduledEventsList", () => {
it("should correctly refresh scheduled events", async () => {
function generateEventData(): IStreamEventData {
function generateInputEventData(): IParsedStreamEventData {
return {
type: "dash-event-stream",
value: {
schemeIdUri: "toto",
timescale: 1,
element: document.createElement("div"),
xmlData: { data: "<div />", namespaces: [] },
},
};
}
function generateOutputEventData(): IStreamEventData {
const parsedDom = new DOMParser().parseFromString(
"<toremove><div /></toremove>",
"application/xml",
).documentElement;

const element =
parsedDom.children.length > 0
? parsedDom.children[0]
: (parsedDom.childNodes[0] as HTMLElement);

return {
type: "dash-event-stream",
value: {
schemeIdUri: "toto",
timescale: 1,
element,
},
};
}
const manifest = {
periods: [
{
start: 0,
streamEvents: [{ start: 0, end: 1, data: generateEventData(), id: "1" }],
streamEvents: [{ start: 0, end: 1, data: generateInputEventData(), id: "1" }],
},
{
start: 10,
streamEvents: [
{ start: 11, end: 20, data: generateEventData(), id: "2" },
{ start: 12, data: generateEventData(), id: "3" },
{ start: 13, end: 13.1, data: generateEventData(), id: "4" },
{ start: 11, end: 20, data: generateInputEventData(), id: "2" },
{ start: 12, data: generateInputEventData(), id: "3" },
{ start: 13, end: 13.1, data: generateInputEventData(), id: "4" },
],
},
],
Expand All @@ -41,22 +62,22 @@ describe("init - refreshScheduledEventsList", () => {
start: 1000,
end: 1000000,
id: "must-disapear",
data: generateEventData(),
data: generateOutputEventData(),
publicEvent: {
start: 1000,
end: 1000000,
data: generateEventData(),
data: generateOutputEventData(),
},
},
{
start: 0,
end: 1,
data: generateEventData(),
data: generateOutputEventData(),
id: "1",
publicEvent: {
start: 1000,
end: 1000000,
data: generateEventData(),
data: generateOutputEventData(),
},
},
];
Expand All @@ -70,33 +91,32 @@ describe("init - refreshScheduledEventsList", () => {
start: 0,
end: 1,
id: "1",
data: generateEventData(),
data: generateOutputEventData(),
publicEvent: {
start: 1000,
end: 1000000,
data: generateEventData(),
data: generateOutputEventData(),
},
},
{
start: 11,
end: 20,
id: "2",
publicEvent: { start: 11, end: 20, data: generateEventData() },
data: generateEventData(),
publicEvent: { start: 11, end: 20, data: generateOutputEventData() },
data: generateOutputEventData(),
},
{
start: 12,
end: undefined,
id: "3",
publicEvent: { start: 12, data: generateEventData() },
data: generateEventData(),
publicEvent: { start: 12, data: generateOutputEventData() },
data: generateOutputEventData(),
},
{
start: 13,
end: 13.1,
id: "4",
publicEvent: { start: 13, end: 13.1, data: generateEventData() },
data: generateEventData(),
publicEvent: { start: 13, end: 13.1, data: generateOutputEventData() },
data: generateOutputEventData(),
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ function refreshScheduledEventsList(
}

let element: Element;
if (data.value.element !== undefined) {
element = data.value.element;
} else if (data.value.xmlData !== undefined) {
if (data.value.xmlData !== undefined) {
// First, we will create a parent Element defining all namespaces that
// should have been encountered until know.
// This is needed because the DOMParser API might throw when
Expand All @@ -66,7 +64,10 @@ function refreshScheduledEventsList(
} else {
return;
}
const actualData = { type: data.type, value: { ...data.value, element } };
const actualData = {
type: data.type,
value: { ...data.value, xmlData: undefined, element },
};
if (end === undefined) {
const newScheduledEvent = {
start,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,64 +2,11 @@ import { describe, it, expect, vi } from "vitest";
import log from "../../../../../../../log";
import type { ITNode } from "../../../../../../../utils/xml-parser";
import { parseXml } from "../../../../../../../utils/xml-parser";
import { parseSHTMLElement, parseSElementNode } from "../parse_s_element";
import { parseSElementNode } from "../parse_s_element";

function testNumberAttribute(attributeName: string, variableName?: string): void {
const _variableName = variableName ?? attributeName;

it(`should correctly parse an HTML S element with a correct ${attributeName} attribute`, () => {
const spyLog = vi.spyOn(log, "warn").mockImplementation(vi.fn());
const element1 = new DOMParser().parseFromString(
`<S ${attributeName}="012" />`,
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element1)).toEqual({ [_variableName]: 12 });

const element2 = new DOMParser().parseFromString(
`<S ${attributeName}="0" />`,
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element2)).toEqual({ [_variableName]: 0 });

const element3 = new DOMParser().parseFromString(
`<S ${attributeName}="-50" />`,
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element3)).toEqual({ [_variableName]: -50 });

expect(spyLog).not.toHaveBeenCalled();
spyLog.mockRestore();
});

it(`should correctly parse an HTML S element with an incorrect ${attributeName} attribute`, () => {
const spyLog = vi.spyOn(log, "warn").mockImplementation(vi.fn());
const element1 = new DOMParser().parseFromString(
`<S ${attributeName}="toto" />`,
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element1)).toEqual({});
expect(spyLog).toHaveBeenCalledTimes(1);
expect(spyLog).toHaveBeenCalledWith(`DASH: invalid ${attributeName} ("toto")`);

const element2 = new DOMParser().parseFromString(
`<S ${attributeName}="PT5M" />`,
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element2)).toEqual({});
expect(spyLog).toHaveBeenCalledTimes(2);
expect(spyLog).toHaveBeenCalledWith(`DASH: invalid ${attributeName} ("PT5M")`);

const element3 = new DOMParser().parseFromString(
`<S ${attributeName}="" />`,
"text/xml",
).childNodes[0] as Element;

expect(parseSHTMLElement(element3)).toEqual({});
expect(spyLog).toHaveBeenCalledTimes(3);
expect(spyLog).toHaveBeenCalledWith(`DASH: invalid ${attributeName} ("")`);
spyLog.mockRestore();
});

it(`should correctly parse a node S element with a correct ${attributeName} attribute`, () => {
const spyLog = vi.spyOn(log, "warn").mockImplementation(vi.fn());
const element1 = parseXml(`<S ${attributeName}="012" />`)[0] as ITNode;
Expand Down Expand Up @@ -97,43 +44,10 @@ function testNumberAttribute(attributeName: string, variableName?: string): void
}

describe("DASH Node Parsers - S", () => {
it("should correctly parse an HTML S element without attributes", () => {
const element = new DOMParser().parseFromString("<S />", "text/xml")
.childNodes[0] as Element;
expect(parseSHTMLElement(element)).toEqual({});
});

it("should correctly parse a Node S element without attributes", () => {
const element = parseXml("<S />")[0] as ITNode;
expect(parseSElementNode(element)).toEqual({});
});

testNumberAttribute("t", "start");
testNumberAttribute("r", "repeatCount");
testNumberAttribute("d", "duration");

it("should correctly parse an HTML S Element with every attributes", () => {
const element1 = new DOMParser().parseFromString(
'<S t="0" d="4" r="12" />',
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element1)).toEqual({
start: 0,
repeatCount: 12,
duration: 4,
});

const element2 = new DOMParser().parseFromString(
'<S t="99" d="4" r="0" />',
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element2)).toEqual({
start: 99,
repeatCount: 0,
duration: 4,
});
});

it("should correctly parse a node S Element with every attributes", () => {
const element1 = parseXml('<S t="0" d="4" r="12" />')[0] as ITNode;
expect(parseSElementNode(element1)).toEqual({
Expand All @@ -150,22 +64,6 @@ describe("DASH Node Parsers - S", () => {
});
});

it("should correctly parse an HTML S Element with unknown attributes", () => {
const element1 = new DOMParser().parseFromString(
'<S t="0" d="4" r="12" f="9" />',
"text/xml",
).childNodes[0] as Element;
expect(parseSHTMLElement(element1)).toEqual({
start: 0,
repeatCount: 12,
duration: 4,
});

const element2 = new DOMParser().parseFromString('<S b="7000" />', "text/xml")
.childNodes[0] as Element;
expect(parseSHTMLElement(element2)).toEqual({});
});

it("should correctly parse a node S Element with unknown attributes", () => {
const element1 = parseXml('<S t="0" d="4" r="12" f="9" />')[0] as ITNode;
expect(parseSElementNode(element1)).toEqual({
Expand Down
Loading

0 comments on commit b39f2dd

Please sign in to comment.