-
Notifications
You must be signed in to change notification settings - Fork 134
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
[Proposal] types: simplify types for PlaybackObserver #1589
base: dev
Are you sure you want to change the base?
Changes from 4 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 |
---|---|---|
@@ -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"; | ||
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 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 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"; | ||
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:
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"; | ||
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"; | ||
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