-
Notifications
You must be signed in to change notification settings - Fork 139
[Proposal] types: simplify types for PlaybackObserver #1589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 5 commits
48bbe04
ab07a1b
e630bb5
6791b75
0e1208a
ebf2d46
6c8ccdd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,15 +6,11 @@ import type { | |
| IRepresentation, | ||
| ISegment, | ||
| } from "../../manifest"; | ||
| import type { | ||
| IReadOnlyPlaybackObserver, | ||
| IRebufferingStatus, | ||
| ObservationPosition, | ||
| } from "../../playback_observer"; | ||
| import type { IReadOnlyPlaybackObserver } from "../../playback_observer"; | ||
| import type { ICorePlaybackObservation } from "../../playback_observer/types"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could maybe just re-export it just from the |
||
| import type { ICmcdOptions, ICmcdPayload, ITrackType } from "../../public_types"; | ||
| import createUuid from "../../utils/create_uuid"; | ||
| import isNullOrUndefined from "../../utils/is_null_or_undefined"; | ||
| import type { IRange } from "../../utils/ranges"; | ||
| import TaskCanceller from "../../utils/task_canceller"; | ||
| import { getRelativeUrl } from "../../utils/url-utils"; | ||
|
|
||
|
|
@@ -59,34 +55,6 @@ export interface ICmcdSegmentInfo { | |
| nextSegment: ISegment | null | undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Media playback observation's properties the `CmcdDataBuilder` wants to have | ||
| * access to. | ||
| */ | ||
| export interface ICmcdDataBuilderPlaybackObservation { | ||
| /** | ||
| * Ranges of buffered data per type of media. | ||
| * `null` if no buffer exists for that type of media. | ||
| */ | ||
| buffered: Record<ITrackType, IRange[] | null>; | ||
| /** | ||
| * Information on the current media position in seconds at the time of the | ||
| * Observation. | ||
| */ | ||
| position: ObservationPosition; | ||
| /** Target playback rate at which we want to play the content. */ | ||
| speed: number; | ||
| /** | ||
| * Describes when the player is "rebuffering" and what event started that | ||
| * status. | ||
| * "Rebuffering" is a status where the player has not enough buffer ahead to | ||
| * play reliably. | ||
| * The RxPlayer should pause playback when a playback observation indicates the | ||
| * rebuffering status. | ||
| */ | ||
| rebuffering: IRebufferingStatus | null; | ||
| } | ||
|
|
||
| /** | ||
| * Class allowing to easily obtain "Common Media Client Data" (CMCD) properties | ||
| * that may be relied on while performing HTTP(S) requests on a CDN. | ||
|
|
@@ -98,7 +66,7 @@ export default class CmcdDataBuilder { | |
| private _contentId: string; | ||
| private _typePreference: TypePreference; | ||
| private _lastThroughput: Partial<Record<ITrackType, number | undefined>>; | ||
| private _playbackObserver: IReadOnlyPlaybackObserver<ICmcdDataBuilderPlaybackObservation> | null; | ||
| private _playbackObserver: IReadOnlyPlaybackObserver<ICorePlaybackObservation> | null; | ||
| private _bufferStarvationToggle: boolean; | ||
| private _canceller: TaskCanceller | null; | ||
|
|
||
|
|
@@ -131,7 +99,7 @@ export default class CmcdDataBuilder { | |
| * @param {Object} playbackObserver | ||
| */ | ||
| public startMonitoringPlayback( | ||
| playbackObserver: IReadOnlyPlaybackObserver<ICmcdDataBuilderPlaybackObservation>, | ||
| playbackObserver: IReadOnlyPlaybackObserver<ICorePlaybackObservation>, | ||
| ): void { | ||
| this._canceller?.cancel(); | ||
| this._canceller = new TaskCanceller(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,4 @@ | ||
| import CmcdDataBuilder from "./cmcd_data_builder"; | ||
|
|
||
| export type { | ||
| ICmcdSegmentInfo, | ||
| ICmcdDataBuilderPlaybackObservation, | ||
| } from "./cmcd_data_builder"; | ||
| export type { ICmcdSegmentInfo } from "./cmcd_data_builder"; | ||
| export default CmcdDataBuilder; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,14 +4,15 @@ import type { | |
| IAudioTrackSwitchingMode, | ||
| IVideoTrackSwitchingMode, | ||
| } from "../../../public_types"; | ||
| import type { IRange } from "../../../utils/ranges"; | ||
| import type { IReadOnlySharedReference } from "../../../utils/reference"; | ||
| import type { IRepresentationEstimator } from "../../adaptive"; | ||
| import type { SegmentQueueCreator } from "../../fetchers"; | ||
| import type { IBufferType, SegmentSink } from "../../segment_sinks"; | ||
| import type { IPeriodStreamPlaybackObservation } from "../period"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To me the PeriodStream depends on the AdaptationStream, not the other way around
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The thing is that they don't overlap well, I will just define them as both types with no
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could also use different property names to explicit that difference |
||
| import type { | ||
| IRepresentationsChoice, | ||
| IRepresentationStreamCallbacks, | ||
| IRepresentationStreamPlaybackObservation, | ||
| } from "../representation"; | ||
|
|
||
| /** Callbacks called by the `AdaptationStream` on various events. */ | ||
|
|
@@ -112,16 +113,17 @@ export interface INeedsBufferFlushPayload { | |
|
|
||
| /** Regular playback information needed by the AdaptationStream. */ | ||
| export interface IAdaptationStreamPlaybackObservation | ||
| extends IRepresentationStreamPlaybackObservation { | ||
| extends Omit<IPeriodStreamPlaybackObservation, "buffered"> { | ||
| /** | ||
| * For the current SegmentSink, difference in seconds between the next position | ||
| * where no segment data is available and the current position. | ||
| */ | ||
| bufferGap: number; | ||
| /** `duration` property of the HTMLMediaElement on which the content plays. */ | ||
| duration: number; | ||
| /** Theoretical maximum position on the content that can currently be played. */ | ||
| maximumPosition: number; | ||
| /** | ||
| * Ranges of buffered data for the adaptation media type. | ||
| * `null` if no buffer exists for the media type. | ||
| */ | ||
| buffered: IRange[] | null; | ||
| } | ||
|
|
||
| /** Arguments given when creating a new `AdaptationStream`. */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,10 +7,7 @@ import type { | |
| IRepresentation, | ||
| } from "../../../manifest"; | ||
| import type { IEMSG } from "../../../parsers/containers/isobmff"; | ||
| import type { | ||
| ObservationPosition, | ||
| IReadOnlyPlaybackObserver, | ||
| } from "../../../playback_observer"; | ||
| import type { IReadOnlyPlaybackObserver } from "../../../playback_observer"; | ||
| import type { | ||
| IAudioRepresentationsSwitchingMode, | ||
| IPlayerError, | ||
|
|
@@ -20,6 +17,7 @@ import type { IRange } from "../../../utils/ranges"; | |
| import type { IReadOnlySharedReference } from "../../../utils/reference"; | ||
| import type { SegmentQueue } from "../../fetchers"; | ||
| import type { IBufferType, SegmentSink } from "../../segment_sinks"; | ||
| import type { IAdaptationStreamPlaybackObservation } from "../adaptation"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wrote it as a comment but you may have not seen it:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| /** Callbacks called by the `RepresentationStream` on various events. */ | ||
| export interface IRepresentationStreamCallbacks { | ||
|
|
@@ -178,22 +176,6 @@ export interface IBufferDiscontinuity { | |
| end: number | null; | ||
| } | ||
|
|
||
| /** Object that should be emitted by the given `IReadOnlyPlaybackObserver`. */ | ||
| export interface IRepresentationStreamPlaybackObservation { | ||
| /** | ||
| * Information on the current media position in seconds at the time of a | ||
| * Playback Observation. | ||
| */ | ||
| position: ObservationPosition; | ||
| /** | ||
| * Information on whether the media element was paused at the time of the | ||
| * Observation. | ||
| */ | ||
| paused: IPausedPlaybackObservation; | ||
| /** Last "playback rate" asked by the user. */ | ||
| speed: number; | ||
| } | ||
|
|
||
| /** Pause-related information linked to an emitted Playback observation. */ | ||
| export interface IPausedPlaybackObservation { | ||
| /** | ||
|
|
@@ -269,7 +251,7 @@ export interface IRepresentationStreamArguments<TSegmentDataType> { | |
| */ | ||
| terminate: IReadOnlySharedReference<null | ITerminationOrder>; | ||
| /** Periodically emits the current playback conditions. */ | ||
| playbackObserver: IReadOnlyPlaybackObserver<IRepresentationStreamPlaybackObservation>; | ||
| playbackObserver: IReadOnlyPlaybackObserver<IAdaptationStreamPlaybackObservation>; | ||
| /** Supplementary arguments which configure the RepresentationStream's behavior. */ | ||
| options: IRepresentationStreamOptions; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ import type { | |
| IPushChunkInfos, | ||
| SegmentSink, | ||
| } from "../../../segment_sinks"; | ||
| import type { IRepresentationStreamPlaybackObservation } from "../types"; | ||
| import type { IAdaptationStreamPlaybackObservation } from "../../adaptation"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
|
|
||
| /** | ||
| * Append a segment to the given segmentSink. | ||
|
|
@@ -46,7 +46,7 @@ import type { IRepresentationStreamPlaybackObservation } from "../types"; | |
| * @returns {Promise} | ||
| */ | ||
| export default async function appendSegmentToBuffer<T>( | ||
| playbackObserver: IReadOnlyPlaybackObserver<IRepresentationStreamPlaybackObservation>, | ||
| playbackObserver: IReadOnlyPlaybackObserver<IAdaptationStreamPlaybackObservation>, | ||
| segmentSink: SegmentSink, | ||
| dataInfos: IPushChunkInfos<T> & { inventoryInfos: IInsertedChunkInfos }, | ||
| bufferGoal: IReadOnlySharedReference<number>, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,11 +27,8 @@ import type { | |
| SegmentSink, | ||
| } from "../../../segment_sinks"; | ||
| import SegmentSinksStore, { SegmentSinkOperation } from "../../../segment_sinks"; | ||
| import type { | ||
| IBufferDiscontinuity, | ||
| IRepresentationStreamPlaybackObservation, | ||
| IQueuedSegment, | ||
| } from "../types"; | ||
| import type { IAdaptationStreamPlaybackObservation } from "../../adaptation"; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same |
||
| import type { IBufferDiscontinuity, IQueuedSegment } from "../types"; | ||
| import checkForDiscontinuity from "./check_for_discontinuity"; | ||
| import getNeededSegments from "./get_needed_segments"; | ||
| import getSegmentPriority from "./get_segment_priority"; | ||
|
|
@@ -92,7 +89,7 @@ export default function getBufferStatus( | |
| representation: IRepresentation; | ||
| }, | ||
| initialWantedTime: number, | ||
| playbackObserver: IReadOnlyPlaybackObserver<IRepresentationStreamPlaybackObservation>, | ||
| playbackObserver: IReadOnlyPlaybackObserver<IAdaptationStreamPlaybackObservation>, | ||
| fastSwitchThreshold: number | undefined, | ||
| bufferGoal: number, | ||
| maxBufferSize: number, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it weird that the ABR rely on a type defined inside another module it does not care about?
Maybe we should put that type in a more neutral place