Issue 4197: Typescript typings #4200
Conversation
|
@Darkwinde Thanks for your PR. We need a signed CLA to merge these changes. I think Stefan and Görkem already discussed this with you? Following our contribution guidelines, and since this seems to be your first PR, could you please send me a signed copy of dash.js feedback agreement? |
| @@ -1330,7 +1408,7 @@ declare namespace dashjs { | |||
|
|
|||
| setTextTrack(idx: number): void; | |||
|
|
|||
| getBitrateInfoListFor(type: MediaType): BitrateInfo[]; | |||
| getBitrateInfoListFor(type: MediaType): QualityInfo[]; | |||
There was a problem hiding this comment.
Why was this type changed?
There was a problem hiding this comment.
In general I am not sure that the definition of "BitrateInfo" is correct...you defined it as a class but use it as an array object.
I changed it for "getBitrateInfoListFor" as I have seen during my testing it has more properties compared to "BitrateInfo" and they are the same like "QualityInfo".
| @@ -1316,7 +1394,7 @@ declare namespace dashjs { | |||
|
|
|||
| getDashMetrics(): DashMetrics; | |||
|
|
|||
| getQualityFor(type: MediaType): number; | |||
| getQualityFor(type: MediaType): QualityInfo; | |||
There was a problem hiding this comment.
This only returns a number
There was a problem hiding this comment.
Nope, not what I see / get it is an quality object back...but this is fine and even better, as there are all information that I need. Index requires another call / request to get the correct quality information :) Maybe not intended, but a good from my point of view.
console.log('getQualityFor("video")', this.dashjsplayerInstance.playerInstance.getQualityFor("video"));
There was a problem hiding this comment.
Is this based on the AdaptationSet Switching branch? Because for development it should be a number
There was a problem hiding this comment.
Yes we currently use the ASS branch.
I checked with the public 4.7.1 NPM version and this is a number.
Good to know.
So this is something to be changed then in a future release? 4.7.2, 5.0?
Need to know then it is planned to be merged, as other partner also rely on this method to get the Bitrate and Rendition and currently cannot obtain the correct information.
| index: number; | ||
| duration: number; | ||
| start: number; | ||
| mpd: Mpd; | ||
| nextPeriodId: string | null; | ||
| } | ||
|
|
||
| export interface IAdaption { | ||
| ContentProtection: IContentProtection[]; | ||
| ContentProtection_asArray: IContentProtection[] // Why? ContentProtection is already type array |
There was a problem hiding this comment.
Depends on how many entires there are. If there is only one entry ContentProtection is an object not an array. This is one of the reasons we are replacing the XML parser in v.5
There was a problem hiding this comment.
ok understood. Was just wondering the duplication I have always seen :)
Yes, will clarify internally this point. |
Pull request targets issue 4197: #4197