From e3c71ee6047fe0f10a5485e1205064dac044d68e Mon Sep 17 00:00:00 2001
From: Paul Berberian <paul.berberian+github@proton.me>
Date: Mon, 29 Jul 2024 17:15:51 +0200
Subject: [PATCH] Simplify MetaPlaylist Manifest loading by relying on the same
 code than other transports

I was PoCing protocol-detection when initially parsing the Manifest, so
application developers wouldn't e.g. have to tell to us that they want
to play "dash".

While doing that, I noticed that all transports had close to the same
logic for Manifest fetching. Only differences I've seen are:

  - DASH may add integrity checks if the `checkManifestIntegrity` option
    is set

  - DASH may ask the browser to obtain the response as an ArrayBuffer
    (and not as a default JS string) if it is probable that we will rely
    on the WASM MPD parser

  - the `local` transport throws if no custom `manifestLoader` is
    declared.

As such, those differences look minor and straightforward enough so the
corresponding logic can be factorized into e.g. a single parameterized
`createManifestLoader` function.

This was already done for DASH and Smooth, but I don't know why we
didn't use it for the experimental MetaPlaylist feature. This commit
fixes this.

We could also use it for the experimental `local` transport in the
future, though I'm unsure of how we could generically make sense of its
peculiar rule (no real loading).
---
 .../metaplaylist/manifest_loader.ts           | 74 -------------------
 src/transports/metaplaylist/pipelines.ts      | 12 ++-
 2 files changed, 8 insertions(+), 78 deletions(-)
 delete mode 100644 src/transports/metaplaylist/manifest_loader.ts

diff --git a/src/transports/metaplaylist/manifest_loader.ts b/src/transports/metaplaylist/manifest_loader.ts
deleted file mode 100644
index e9f8a5cd92..0000000000
--- a/src/transports/metaplaylist/manifest_loader.ts
+++ /dev/null
@@ -1,74 +0,0 @@
-/**
- * Copyright 2015 CANAL+ Group
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import type { IManifestLoader, ILoadedManifestFormat } from "../../public_types";
-import request from "../../utils/request";
-import type { CancellationSignal } from "../../utils/task_canceller";
-import type { IManifestLoaderOptions, IRequestedData } from "../types";
-import addQueryString from "../utils/add_query_string";
-import callCustomManifestLoader from "../utils/call_custom_manifest_loader";
-
-/**
- * Manifest loader triggered if there was no custom-defined one in the API.
- * @param {string} initialUrl
- * @param {Object} loaderOptions
- * @param {Object} cancelSignal
- */
-function regularManifestLoader(
-  initialUrl: string | undefined,
-  loaderOptions: IManifestLoaderOptions,
-  cancelSignal: CancellationSignal,
-): Promise<IRequestedData<ILoadedManifestFormat>> {
-  if (initialUrl === undefined) {
-    throw new Error("Cannot perform HTTP(s) request. URL not known");
-  }
-
-  const url =
-    loaderOptions.cmcdPayload?.type === "query"
-      ? addQueryString(initialUrl, loaderOptions.cmcdPayload.value)
-      : initialUrl;
-
-  return request({
-    url,
-    headers:
-      loaderOptions.cmcdPayload?.type === "headers"
-        ? loaderOptions.cmcdPayload.value
-        : undefined,
-    responseType: "text",
-    timeout: loaderOptions.timeout,
-    connectionTimeout: loaderOptions.connectionTimeout,
-    cancelSignal,
-  });
-}
-
-/**
- * Generate a manifest loader for the application
- * @param {Function} [customManifestLoader]
- * @returns {Function}
- */
-export default function generateManifestLoader({
-  customManifestLoader,
-}: {
-  customManifestLoader?: IManifestLoader | undefined;
-}): (
-  url: string | undefined,
-  loaderOptions: IManifestLoaderOptions,
-  cancelSignal: CancellationSignal,
-) => Promise<IRequestedData<ILoadedManifestFormat>> {
-  return typeof customManifestLoader !== "function"
-    ? regularManifestLoader
-    : callCustomManifestLoader(customManifestLoader, regularManifestLoader);
-}
diff --git a/src/transports/metaplaylist/pipelines.ts b/src/transports/metaplaylist/pipelines.ts
index 722b21bcd2..1b6f99afdf 100644
--- a/src/transports/metaplaylist/pipelines.ts
+++ b/src/transports/metaplaylist/pipelines.ts
@@ -45,7 +45,7 @@ import type {
   ITransportOptions,
   ITransportPipelines,
 } from "../types";
-import generateManifestLoader from "./manifest_loader";
+import generateManifestLoader from "../utils/generate_manifest_loader";
 
 /**
  * Get base - real - content from an offseted metaplaylist content.
@@ -113,9 +113,13 @@ function getMetaPlaylistPrivateInfos(segment: ISegment): IMetaPlaylistPrivateInf
 export default function (options: ITransportOptions): ITransportPipelines {
   const transports: Partial<Record<string, ITransportPipelines>> = {};
 
-  const manifestLoader = generateManifestLoader({
-    customManifestLoader: options.manifestLoader,
-  });
+  const manifestLoader = generateManifestLoader(
+    {
+      customManifestLoader: options.manifestLoader,
+    },
+    "text",
+    null,
+  );
 
   // remove some options that we might not want to apply to the
   // other streaming protocols used here