Skip to content

#7109: Use segment or audio samples duration in case of single sample video chunk #7112

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#7109: Use segment or audio samples duration in case of single sample video chunk #7112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion api-extractor/report/hls.js.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,12 +860,14 @@ export class CaptionScreen {
//
// @public (undocumented)
export class ChunkMetadata {
constructor(level: number, sn: number, id: number, size?: number, part?: number, partial?: boolean);
constructor(level: number, sn: number, id: number, size?: number, part?: number, partial?: boolean, duration?: number | null);
// (undocumented)
readonly buffering: {
[key in SourceBufferName]: HlsChunkPerformanceTiming;
};
// (undocumented)
readonly duration: number | null;
// (undocumented)
readonly id: number;
// (undocumented)
readonly level: number;
Expand Down
1 change: 1 addition & 0 deletions src/controller/audio-stream-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ class AudioStreamController
payload.byteLength,
partIndex,
partial,
!partial ? frag.duration : null,
);
transmuxer.push(
payload,
Expand Down
1 change: 1 addition & 0 deletions src/controller/base-stream-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ export default class BaseStreamController
0,
part ? part.index : -1,
!complete,
complete ? fragLoadedEndData.frag.duration : null,
);
transmuxer.flush(chunkMeta);
}
Expand Down
1 change: 1 addition & 0 deletions src/controller/stream-controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,7 @@ export default class StreamController
payload.byteLength,
partIndex,
partial,
!partial ? frag.duration : null,
);
const initPTS = this.initPTS[frag.cc];

Expand Down
3 changes: 3 additions & 0 deletions src/demux/transmuxer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ export default class Transmuxer {
accurateTimeOffset,
true,
this.id,
chunkMeta.duration,
);
transmuxResults.push({
remuxResult,
Expand Down Expand Up @@ -411,6 +412,7 @@ export default class Transmuxer {
accurateTimeOffset,
false,
this.id,
chunkMeta.duration,
);
return {
remuxResult,
Expand All @@ -437,6 +439,7 @@ export default class Transmuxer {
accurateTimeOffset,
false,
this.id,
chunkMeta.duration,
);
return {
remuxResult,
Expand Down
15 changes: 14 additions & 1 deletion src/remux/mp4-remuxer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export default class MP4Remuxer implements Remuxer {
accurateTimeOffset: boolean,
flush: boolean,
playlistType: PlaylistLevelType,
fragmentDuration: number,
): RemuxerResult {
let video: RemuxedTrack | undefined;
let audio: RemuxedTrack | undefined;
Expand Down Expand Up @@ -290,6 +291,7 @@ export default class MP4Remuxer implements Remuxer {
videoTimeOffset,
isVideoContiguous,
audioTrackLength,
fragmentDuration,
);
}
} else if (enoughVideoSamples) {
Expand All @@ -298,6 +300,7 @@ export default class MP4Remuxer implements Remuxer {
videoTimeOffset,
isVideoContiguous,
0,
fragmentDuration,
);
}
if (video) {
Expand Down Expand Up @@ -468,6 +471,7 @@ export default class MP4Remuxer implements Remuxer {
timeOffset: number,
contiguous: boolean,
audioTrackLength: number,
fragmentDuration: number,
): RemuxedTrack | undefined {
const timeScale: number = track.inputTimeScale;
const inputSamples: Array<VideoSample> = track.samples;
Expand Down Expand Up @@ -530,9 +534,15 @@ export default class MP4Remuxer implements Remuxer {
// Sample duration (as expected by trun MP4 boxes), should be the delta between sample DTS
// set this constant duration as being the avg delta between consecutive DTS.
const inputDuration = lastDTS - firstDTS;
const audioLengthBasedSampleDuration = audioTrackLength
? Math.round(audioTrackLength * track.inputTimeScale)
: track.inputTimeScale;
const fragmentLengthOrAudioLengthBasedSampleDuration = fragmentDuration
Copy link
Collaborator

@robwalch robwalch Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be incorrect to use fragment duration in the case where an LL-HLS part is being processed, or a partial segment is being processed (with progressive: true config option).

We'll need to test this against a variety of media with and without that config to confirm it is safe.

This makes sense to include in v1.7 which will focus on I-frame support since that will involve supporting single sample video segments that ideally span full segment durations.

Copy link
Contributor Author

@mstyura mstyura Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be incorrect to use fragment duration in the case where an LL-HLS part is being processed, or a partial segment is being processed (with progressive: true config option).

I agree, I've tried to "workaround" this by not passing duration of incomplete chunks, but not sure all the cases handled correctly. For some reason I assumed it is not that important because I thought LLHLS is defined only over mp4 chunks, but right now I can't find this in latest rfc.

This makes sense to include in v1.7 which will focus on I-frame support since that will involve supporting single sample video segments that ideally span full segment durations.

I have nothing against this decision, the PR is just an example of solving the issue for the example stream. I hope during development of version 1.7 more robust solution will be considered and hopefully implemented.

? Math.round(fragmentDuration * track.inputTimeScale)
: audioLengthBasedSampleDuration;
const averageSampleDuration = inputDuration
? Math.round(inputDuration / (nbSamples - 1))
: mp4SampleDuration || track.inputTimeScale / 30;
: fragmentLengthOrAudioLengthBasedSampleDuration;

// if fragment are contiguous, detect hole/overlapping between fragments
if (contiguous) {
Expand Down Expand Up @@ -737,6 +747,9 @@ export default class MP4Remuxer implements Remuxer {
maxDtsDelta = Math.max(maxDtsDelta, mp4SampleDuration);
minPtsDelta = Math.min(minPtsDelta, ptsDelta);
maxPtsDelta = Math.max(maxPtsDelta, ptsDelta);
this.logger.trace(
`[mp4-remuxer]: audioTrackLength = ${audioTrackLength}, fragmentDuration=${fragmentDuration}, fragmentLengthBasedSampleDuration=${fragmentLengthOrAudioLengthBasedSampleDuration}, audioLengthBasedSampleDuration=${audioLengthBasedSampleDuration}, averageSampleDuration=${averageSampleDuration}, mp4SampleDuration=${mp4SampleDuration}`,
);

outputSamples.push(
createMp4Sample(
Expand Down
1 change: 1 addition & 0 deletions src/types/remuxer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export interface Remuxer {
accurateTimeOffset: boolean,
flush: boolean,
playlistType: PlaylistLevelType,
fragmentDuration: number | null,
): RemuxerResult;
resetInitSegment(
initSegment: Uint8Array | undefined,
Expand Down
3 changes: 3 additions & 0 deletions src/types/transmuxer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export class ChunkMetadata {
public readonly id: number;
public readonly size: number;
public readonly partial: boolean;
public readonly duration: number | null;
public readonly transmuxing: HlsChunkPerformanceTiming =
getNewPerformanceTiming();
public readonly buffering: {
Expand All @@ -31,13 +32,15 @@ export class ChunkMetadata {
size = 0,
part = -1,
partial = false,
duration: number | null = null,
) {
this.level = level;
this.sn = sn;
this.id = id;
this.size = size;
this.part = part;
this.partial = partial;
this.duration = duration;
}
}

Expand Down
14 changes: 12 additions & 2 deletions tests/unit/demuxer/transmuxer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,12 @@ describe('TransmuxerInterface tests', function () {
const videoCodec = '';
const duration = 0;
const accurateTimeOffset = true;
let chunkMeta = new ChunkMetadata(currentFrag.level, currentFrag.sn + 1, 0);
let chunkMeta = new ChunkMetadata(
currentFrag.level,
currentFrag.sn + 1,
0,
currentFrag.duration,
);
let state = new TransmuxState(false, true, true, false, 0, false);
transmuxerInterface.push(
data,
Expand Down Expand Up @@ -224,7 +229,12 @@ describe('TransmuxerInterface tests', function () {
const videoCodec = '';
const duration = 0;
const accurateTimeOffset = true;
const chunkMeta = new ChunkMetadata(newFrag.level, newFrag.sn, 0);
const chunkMeta = new ChunkMetadata(
newFrag.level,
newFrag.sn,
0,
newFrag.duration,
);

const configureStub = sinon.stub(
transmuxerInterfacePrivates.transmuxer,
Expand Down
Loading