pjaiprakakamai
commented
1 year ago Hi Evan,
Regarding point 1, I’m using dash-mpd-parser( @.***/dash-mpd-parser ) which is published by Akamai developer Shige Fukushima . I see this license file in his repo : https://git.source.akamai.com/projects/A2S/repos/ew-media-dash-mpd-video-bitrate-filter/browse/LICENSE-APACHE-2.0 . Can I use the same license file and add it to the repo ?
Regards, Poornima.
From: evan-hughes @.> Reply-To: akamai/edgeworkers-examples @.> Date: Thursday, November 24, 2022 at 2:15 AM To: akamai/edgeworkers-examples @.> Cc: "Jaiprakash, Poornima" @.>, Author @.***> Subject: Re: [akamai/edgeworkers-examples] Feature/media dash module (PR #165)
@evan-hughes requested changes on this pull request.
I have the following concerns:
- The dash parser appears to include a number of other modules, but I don't see a listing of their licenses, etc.
- parseMPD() and stringifyMPD() operate on a static field, which seems to be shared outside the DashParser and then passed back in to be modified with the other functions on the class. Don't do this: the static field persists between requests and is a potential data leak. Either create an object that has the appropriate stringify, updated, filter, etc functions, or remove the static variable holding the JSON and pass the mpdJson value between the functions.
I have a few other style comments about code duplication and stripping stack traces, but those aren't as significant.
In delivery/media/dash/apis/README.mdhttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030774263__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf45r7bRT$:
@@ -0,0 +1,4 @@
+For the latest JWT API documentation, please visit this page
Is that a publicly accessible IP? Do you want to reference it in a publicly available module?
In delivery/media/dash/examples/manifestmanipulation-1/README.mdhttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030775784__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf6LWSxx5$:
@@ -0,0 +1,30 @@
+# Example
+The examples in this section detail how to use Dash module for manifest manipulation
+Manifest Personalization helps to dynamically create personalized versions of Dash manifests based on parameters like device type, user geography, request headers or query string parameters.This example demonstrates use of query string parameters.
+main.js provides examples using ReadableStream, WritableStream for different use cases as defined below:
+filterVariantsByBandwidth which filters out dash MPD representations by bandwidth value passed using query param br_in_range and br_in
+example curl :
+curl -vk -H "Pragma: akamai-x-ew-debug-subs,akamai-x-ew-debug,akamai-x-ew-debug,akamai-x-ew-debug-rp" "https://hostname/sample1.mpd?br_in_range=400000-600000,800000-1000000"
The curls need some sort of markup around them, otherwise they get mixed in with the preceding text.
In delivery/media/dash/lib/README.mdhttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030778265__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf5yooQSg$:
@@ -0,0 +1,19 @@
+# JWT Module
+
+The Dash module can be used to demonstrate EW's capabilities of dynamically creating personalized versions of DASH manifests based on parameters like device type, user geography, request headers or query string parameters.
+
+## Limitations
+
+
+## Files
+* media-delivery-dash-parser.js is the main class you import in your main.js file. This file provides helper classes such as parseMPD for parsing MPD to JSON objects.
+* media-delivery-dash-parse.d.ts is the typescript declaration file for DashParser module.
TypeScript should be capitalized.
In delivery/media/dash/lib/media-delivery-dash-parser.d.tshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030779802__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf-hmQOt6$:
@@ -0,0 +1,88 @@
+declare const MIME_TYPE_VIDEO_PREFIX: string;
+declare const MIME_TYPE_AUDIO_PREFIX: string;
+declare const MIME_TYPE_TEXT_PREFIX: string;
+/**
-
- DashParser class with necessary API's for implementing Dash manifest manipulation usecase for media delivery.
-
- Consumers of Dash manifest manipulation feature can import this class and call the necessay operations.
-
- More details on Dash manifest manipulation product requirement can be found here: @.*** https://collaborate.akamai.com/confluence/display/MDE/EW+DASH+Parser+-+Component+Design}
I believe collaborate.akamai.com is only accessible to Akamai employees. Is there a customer-facing document that offers the same information?
In delivery/media/dash/lib/media-delivery-dash-parser.d.tshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030779998__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfyOBJHCh$:
@@ -0,0 +1,88 @@
+declare const MIME_TYPE_VIDEO_PREFIX: string;
+declare const MIME_TYPE_AUDIO_PREFIX: string;
+declare const MIME_TYPE_TEXT_PREFIX: string;
+/**
-
- DashParser class with necessary API's for implementing Dash manifest manipulation usecase for media delivery.
-
- Consumers of Dash manifest manipulation feature can import this class and call the necessay operations.
Typo in necessary.
In delivery/media/dash/lib/media-delivery-dash-parser.d.tshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030781326__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf3bZtRIR$:
+declare const MIME_TYPE_AUDIO_PREFIX: string;
+declare const MIME_TYPE_TEXT_PREFIX: string;
+/**
-
- DashParser class with necessary API's for implementing Dash manifest manipulation usecase for media delivery.
-
- Consumers of Dash manifest manipulation feature can import this class and call the necessay operations.
-
- More details on Dash manifest manipulation product requirement can be found here: @.*** https://collaborate.akamai.com/confluence/display/MDE/EW+DASH+Parser+-+Component+Design}
-
*/
+declare class DashParser {
-
/**
-
- This function parses the string input provided using streaming and buffering the MPD file
-
- @param mpdXml string
-
- @return none
-
*/
-
static parseMPD(mpdXml: string): void;
-
/**
-
- This function returns the JSON object converted using parseMPD
A @.***} to the parseMPD() function would be helpful.
In delivery/media/dash/lib/media-delivery-dash-parser.d.tshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030782511__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf8ZdzhkI$:
+declare const MIME_TYPE_TEXT_PREFIX: string;
+/**
-
- DashParser class with necessary API's for implementing Dash manifest manipulation usecase for media delivery.
-
- Consumers of Dash manifest manipulation feature can import this class and call the necessay operations.
-
- More details on Dash manifest manipulation product requirement can be found here: @.*** https://collaborate.akamai.com/confluence/display/MDE/EW+DASH+Parser+-+Component+Design}
-
*/
+declare class DashParser {
-
/**
-
- This function parses the string input provided using streaming and buffering the MPD file
-
- @param mpdXml string
-
- @return none
-
*/
-
static parseMPD(mpdXml: string): void;
-
/**
-
- This function returns the JSON object converted using parseMPD
-
- @param none
Please remove the @param tag - it shouldn't be specified if there aren't any parameters.
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030788369__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfw-QYkNA$:
- let filteredRenditions = adaptationSet[DashConstants.REPRESENTATION].filter(filterFnForUnknownAdaptationSet);
-
adaptationSet[DashConstants.REPRESENTATION] = filteredRenditions;
-
}
-
}));
-
}));
-
}
+}, Attr = function(name) {
- return "@" + name;
+}, MIME_TYPE_VIDEO_PREFIX = "video/", MIME_TYPE_AUDIO_PREFIX = "audio/", MIME_TYPE_TEXT_PREFIX = "application/";
+
+class DashParser {
-
static parseMPD(mpdXml) {
-
try {
-
dashMpd.parse(mpdXml);
-
} catch (error) {
-
throw new Error(
DashParser: failed to parseMPD due to ${error.message});
Why catch and rethrow the error? Dropping the original exception means the customer can't see the stack trace.
Unless there's a good reason to strip the stack trace, I suggest removing the try/catch.
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030800734__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf_epVfP2$:
- adaptationSet[DashConstants.REPRESENTATION] = filteredRenditions;
-
} else if ("" === _contentType && "" === _mimeType) {
-
let filteredRenditions = adaptationSet[DashConstants.REPRESENTATION].filter(filterFnForUnknownAdaptationSet);
-
adaptationSet[DashConstants.REPRESENTATION] = filteredRenditions;
-
}
-
}));
-
}));
-
}
+}, Attr = function(name) {
- return "@" + name;
+}, MIME_TYPE_VIDEO_PREFIX = "video/", MIME_TYPE_AUDIO_PREFIX = "audio/", MIME_TYPE_TEXT_PREFIX = "application/";
+
+class DashParser {
-
static parseMPD(mpdXml) {
-
try {
-
dashMpd.parse(mpdXml);
Please return an instance of the dashMpd class instead of keeping the current request's state in a static variable.
It's cleaner and it prevents customer coding errors from accidentally referencing the wrong file.
In general this is a pattern we discourage. If the customer decides they want to cache within their EdgeWorker, they can, but it shouldn't be done in the domain parsing/manipulation layer.
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030808942__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf1zB9deN$:
- crarr: "↵",
-
lceil: "⌈",
-
rceil: "⌉",
-
lfloor: "⌊",
-
rfloor: "⌋",
-
loz: "◊",
-
spades: "♠",
-
clubs: "♣",
-
hearts: "♥",
-
diams: "♦"
-
}), entities), sax = requireSax(), XMLReader = sax.XMLReader, ParseError = sax.ParseError, DOMImplementation = domParser.DOMImplementation = requireDom().DOMImplementation;
-
return domParser.XMLSerializer = requireDom().XMLSerializer, domParser.DOMParser = DOMParser,
-
domParser.__DOMHandler = DOMHandler, domParser;
+}
+
+var X2JS = X2JS$1.exports = function(config) {
Where does this class come from? There's a GitHub project that has similar codehttps://urldefense.com/v3/__https:/github.com/abdolence/x2js/blob/master/xml2json.js__;!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfyNmgs8P$, but it's under an Apache license. If that's the source, then you need to include the license, attribution, and changes listinghttps://urldefense.com/v3/__https:/en.wikipedia.org/wiki/Apache_License*Licensing_conditions__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf4ktB0tm$.
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030811142__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf6Uca6tl$:
- dashMpd.parse(mpdXml);
-
} catch (error) {
-
throw new Error(
DashParser: failed to parseMPD due to ${error.message}); -
}
-
}
-
static getJSON() {
-
return dashMpd.getJSON();
-
}
-
static setJSON(mpdJson) {
-
dashMpd.setJSON(mpdJson);
-
}
-
static stringifyMPD() {
-
try {
-
return dashMpd.getMPD();
-
} catch (error) {
-
throw new Error(
DashParser: failed to parseMPD due to ${error.message});
This discards the stack trace from error, which will make it harder for developers to use the API.
Please remove this.
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030819048__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf7J0zP-4$:
- return this.mpd;
-
}
-
setJSON(mpdJson) {
-
this.mpd = mpdJson;
-
}
-
getMPD() {
-
return new X2JS({
-
attributePrefix: "@",
-
useDoubleQuotes: !0,
-
space: this.indent
-
}).json2xml_str(this.mpd);
-
}
-
setIndent(space) {
-
this.indent = space;
-
}
-
attr(name) {
Why redefine this function? Attr appears to do the same thing.
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030820540__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf8dbVKXc$:
- if (!mpdJson) throw new Error("DashParser: filterVariantsByBandwidth api failed ,dash mpd json object cannot be empty");
-
if (!bitrates) throw new Error("DashParser: filterVariantsByBandwidth api failed,bitrates cannot be empty");
-
if (!Array.isArray(bitrates)) throw new Error("DashParser: filterVariantsByBandwidth api failed,bitrates should be array");
-
if (!bitrates.every((item => "string" == typeof item))) throw new Error("DashParser: filterVariantsByBandwidth api failed,bitrates should be of strings!");
-
const filterFnForVideoAdaptationSet = knownMimeType => element => {
-
let result = !1;
-
const match = {};
-
if (!knownMimeType) {
-
const _mimeType = element[Attr(DashConstants.ATTR_MIME_TYPE)] ? element[Attr(DashConstants.ATTR_MIME_TYPE)] : "", _contentType = element[Attr(DashConstants.ATTR_CONTENT_TYPE)] ? element[Attr(DashConstants.ATTR_CONTENT_TYPE)] : "";
-
if (logger.log("filterVariantsByBandwidth filterFnForVideoAdaptationSet mimetype : %s", _mimeType),
-
logger.log("filterVariantsByBandwidth filterFnForVideoAdaptationSet contenttype : %s", _contentType),
-
_mimeType && !_mimeType.startsWith("video/") || _contentType && "video" != _contentType) return !0;
-
}
-
return bitrates.forEach((range => {
-
if (range) {
-
const intermediateResult = filter(range, element[Attr(DashConstants.ATTR_BANDWIDTH)], tolerance);
AFAICT every instance of DashConstants.ATTR_BANDWIDTH is wrapped by a call to Attr, attr, or Attr$1, all of which prepend an @.
Would it be possible to remove that function call and embed the @ into the constant?
In delivery/media/dash/lib/media-delivery-dash-parser.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030821609__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfxc4lT3N$:
@@ -0,0 +1,2006 @@
+import { logger } from "log";
+
+const DashConstants = new class {
-
constructor() {
-
this.ACCESSIBILITY = "Accessibility", this.ADAPTATION_SET = "AdaptationSet", this.ASSET_IDENTIFIER = "AssetIdentifier",
A large number of these constants aren't referenced in this module (AFAICT) and they don't appear to be exported from this module.
Could you use a tree shaker to remove the unused constants?
In delivery/media/dash/examples/manifestmanipulation-1/main.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030832506__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfzmyAxb6$:
- completeProcessing.then(() => readController.close());
-
}
-
});
-
}
+}
+
+export function onClientResponse (request, response) {
- response.setHeader(
Request Parameters: scheme:${request.scheme}, hostname:${request.host}, path:${request.path});
+}
+
+export function responseProvider (request) {
-
try {
-
return httpRequest(
${request.scheme}://${request.host}${request.path}).then(response => { -
return createResponse(
-
response.status,
-
response.headers,
Do not copy headers directly from the subrequest. That can corrupt the response.
See the green block under https://techdocs.akamai.com/edgeworkers/docs/http-request#http-sub-requests.
In delivery/media/dash/examples/manifestmanipulation-1/main.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030841808__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf37OkraR$:
- let write = function (msg) { readController.enqueue(
${msg});};
-
let completeProcessing = Promise.resolve();
-
this.writable = new WritableStream({
-
write (text) {
-
completeProcessing = processStream(text, false);
-
},
-
close () {
-
processStream('', true); // Signaling EOS
-
completeProcessing.then(() => readController.close());
-
}
-
});
-
}
+}
+
+export function onClientResponse (request, response) {
- response.setHeader(
Request Parameters: scheme:${request.scheme}, hostname:${request.host}, path:${request.path});
Please update this to:
- Set the value in the second argument.
- Remove the space from the header name.
See https://techdocs.akamai.com/edgeworkers/docs/response-object#setheader
In delivery/media/dash/examples/manifestmanipulation-2/main.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030843618__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf-JIbT08$:
+
-
super({ start, transform, flush: flush});
-
}
+}
+
+
+export function onClientResponse (request, response) {
- response.setHeader(
Request Parameters: scheme:${request.scheme}, hostname:${request.host}, path:${request.path});
+}
+
+export function responseProvider (request) {
-
try {
-
return httpRequest(
${request.scheme}://${request.host}${request.path}).then(response => { -
return createResponse(
-
response.status,
-
response.headers,
See the previous comment about reusing the response.headers.
In delivery/media/dash/examples/manifestmanipulation-2/main.jshttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*discussion_r1030843826__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfx-WZwlA$:
- const langs_array = (langs && langs.split(',')) || [];
-
DashParser.filterVariantsByAudioLanguage(mpdJson, langs_array);
-
DashParser.filterVariantsBySubtitlesLanguage(mpdJson, langs_array);
-
}
-
const mpdXml = DashParser.stringifyMPD();
-
controller.enqueue(mpdXml);
-
logger.log("D-T:flush");
-
}
-
super({ start, transform, flush: flush});
-
}
+}
+
+
+export function onClientResponse (request, response) {
- response.setHeader(
Request Parameters: scheme:${request.scheme}, hostname:${request.host}, path:${request.path});
See the previous comment about setHeader parameters.
— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/akamai/edgeworkers-examples/pull/165*pullrequestreview-1192129383__;Iw!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qfxlIul7m$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/A3VOX5NHIVYT2ZC5UE2EXCLWJZ65BANCNFSM6AAAAAASHEAKYU__;!!GjvTz_vk!QnCa2xAr_rcwxvFEYC1vBhsOa8RCfvyaUWLyULKk78Ow0c9a9Yspwx53FAXCowjORoV3RGQ-R9t7_h9qf2vpGRw-$. You are receiving this because you authored the thread.Message ID: @.***>
evan-hughes
This commit includes code changes for dash manifest personalization. Refer to class 'DashParser' in media-delivery-dash-parser.js file . This is the class written by us to implement manifest personalization. The other classes in the file refers to the imported library classes to help in parsing of mpd file from string to JSON object , converting JSON object back to string representing dash media presentation description file.