dash.js icon indicating copy to clipboard operation
dash.js copied to clipboard

Typescript Typings incomplete - usage of generic typing

Open Darkwinde opened this issue 1 year ago • 2 comments

Environment
  • [x] The MPD passes the DASH-IF Conformance Tool on https://conformance.dashif.org/
  • [x] The stream has correct Access-Control-Allow-Origin headers (CORS)
  • [x] There are no network errors such as 404s in the browser console when trying to play the stream
  • [x] The issue observed is not mentioned on https://github.com/Dash-Industry-Forum/dash.js/wiki/FAQ
  • [x] The issue occurs in the latest reference client on http://reference.dashif.org/dash.js/ and not just on my page
  • Link to playable MPD file: N/A
  • Dash.js version: 4.7.0
  • Browser name/version: Chromium M47
  • OS name/version: Tizen 3.0
Steps to reproduce
  1. Embed Dash.js into a Typescript project
  2. Try to use functions and properties with generic typings like "object" and "any"
Observed behavior

It is not possible to access the returned values directly, it is always required to cast which costs significant resources on low end TV sets as well as the developer cannot be sure that the casted values and types are correct.

Examples: getStreamsFromManifest(manifest: object): StreamInfo[]; getSupportedKeySystemsFromContentProtection(cps: object[]): object[]; getEventsForPeriod(period: Period): any[];

Expected behavior

There must be no generic typing used as this will annul the type safety of Typescript. Required casts cost resources on low end devices. With correct typing the developer is always sure to process data in the correct way.

My personal important typing requested to be implement:

  • getQualityFor(type: MediaType): number;
    • Return is typed as number but is an object
    • Return value must be typed like: export interface QualityInfo { absoluteIndex: number; bitrate: number; height: number; isTopBitrate: boolean; mediaInfo: MediaInfo; mediaInfoIndex: number; qualityIndex: number; representationId: string; scanType: string; width: number; }
  • getBitrateInfoListFor(type: MediaType): BitrateInfo[];
    • Return type does not fit, should be QualityInfo[] as returned object has structure like "QualityInfo" instead of "BitrateInfo".
  • getPeriodbyId(id: string): object | null;
    • Writing Error: In the official documentation (https://cdn.dashjs.org/latest/jsdoc/dash_DashAdapter.js.html) and in the SC the function name is getPeriodById
    • Return value must be typed: Period
  • getCurrentRepresentationSwitch(type: MediaType): ICurrentRepresentationSwitch;
    • Has no second parameter, as it is been stated in official examples: http://reference.dashif.org/dash.js/latest/samples/advanced/monitoring.html
  • Interface "ICurrentRepresentationSwitch" must define the "to" attribute
  • Interface "ICurrentRepresentationSwitch" defines "mt" attribute as Date but is number
  • getCurrentBufferLevel(type: MediaType): number;
    • Has no second parameter, as it is been stated in official examples: http://reference.dashif.org/dash.js/latest/samples/advanced/monitoring.html
  • getAdaptationForType(periodIndex: number, type: MediaType, streamInfo: object): object | null;
    • Return value is completely untyped and is a nested object

I tried to update the index.d.ts as far as possible, but you should have a full review on this especially as I do not know which parameter can be optional. I also like to recommend that you revise the Typescript typing in general not to have generic types offered anymore.

Adapted index.d.ts: https://github.com/Dash-Industry-Forum/dash.js/files/11624486/index.d.ts.zip

Darkwinde avatar Jun 01 '23 11:06 Darkwinde

@Darkwinde Can you issue a pull request for your changes?

dsilhavy avatar Jun 04 '23 06:06 dsilhavy

Hi @dsilhavy, sure.

Hope I did it right: https://github.com/Dash-Industry-Forum/dash.js/pull/4200

Darkwinde avatar Jun 05 '23 07:06 Darkwinde