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 Jul 11, 2024
1 parent e3dcb2f commit f5cf0c6
Show file tree
Hide file tree
Showing 66 changed files with 108 additions and 4,123 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 @@ -12,7 +12,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 @@ -74,7 +74,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 },
codecSupportProber: null,
createDebugElement: null,
directfile: 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,7 +1,7 @@
import { describe, it, expect } from "vitest";
import MediaSourceContentInitializer from "../../../main_thread/init/media_source_content_initializer";
import mainCodecSupportProber from "../../../mse/main_codec_support_prober";
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 @@ -10,13 +10,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,
codecSupportProber: mainCodecSupportProber,
});
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 @@ -16,7 +16,7 @@

import MediaSourceContentInitializer from "../../main_thread/init/media_source_content_initializer";
import mainCodecSupportProber from "../../mse/main_codec_support_prober";
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 @@ -28,7 +28,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;
features.codecSupportProber = mainCodecSupportProber;
}
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: HTMLMediaElement) => 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 @@ -150,11 +145,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
Expand Up @@ -7,36 +7,57 @@ import { describe, it, expect, vi } from "vitest";
/* eslint-disable @typescript-eslint/no-explicit-any */

describe("init - refreshScheduledEventsList", () => {
it("should correclty refresh scheduled events", async () => {
function generateEventData() {
it("should correctly refresh scheduled events", async () => {
function generateInputEventData() {
return {
type: "dash-event-stream",
value: {
schemeIdUri: "toto",
timescale: 1,
element: document.createElement("div"),
xmlData: { data: "<div />", namespaces: [] },
},
};
}
function generateOutputEventData() {
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,
xmlData: { data: "<div />", namespaces: [] },
},
};
}
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" },
],
},
],
};
const oldScheduledEvents = [
{ start: 1000, end: 1000000, id: "must-disapear", _isBeingPlayed: true },
{ start: 0, end: 1, data: generateEventData(), id: "1" },
{ start: 0, end: 1, data: generateOutputEventData(), id: "1" },
];
const refreshScheduledEventsList = (
(await vi.importActual(
Expand All @@ -46,28 +67,28 @@ describe("init - refreshScheduledEventsList", () => {

const scheduledEvents = refreshScheduledEventsList(oldScheduledEvents, manifest);
expect(scheduledEvents).toEqual([
{ start: 0, end: 1, id: "1", data: generateEventData() },
{
start: 11,
end: 20,
id: "2",
publicEvent: { start: 11, end: 20, data: generateEventData() },
data: generateEventData(),
},
{
start: 12,
end: undefined,
id: "3",
publicEvent: { start: 12, data: generateEventData() },
data: generateEventData(),
},
{
start: 13,
end: 13.1,
id: "4",
publicEvent: { start: 13, end: 13.1, data: generateEventData() },
data: generateEventData(),
},
]);
{ start: 0, end: 1, id: "1", data: generateOutputEventData() },
{
start: 11,
end: 20,
id: "2",
publicEvent: { start: 11, end: 20, data: generateOutputEventData() },
data: generateOutputEventData(),
},
{
start: 12,
id: "3",
publicEvent: { start: 12, data: generateOutputEventData() },
data: generateOutputEventData(),
},
{
start: 13,
end: 13.1,
id: "4",
publicEvent: { start: 13, end: 13.1, data: generateOutputEventData() },
data: generateOutputEventData(),
},
]
);
});
});
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 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 f5cf0c6

Please sign in to comment.