diff --git a/README.md b/README.md index d581779..09a7a52 100644 --- a/README.md +++ b/README.md @@ -117,11 +117,14 @@ const input = createInputFrom(file, ALL_FORMATS); const duration = await input.computeDuration(); // in seconds const videoTrack = await input.getPrimaryVideoTrack(); const audioTrack = await input.getPrimaryAudioTrack(); + const displayWidth = await videoTrack.getDisplayWidth(); const displayHeight = await videoTrack.getDisplayHeight(); -const { rotation } = videoTrack; +const rotation = await videoTrack.getRotation(); + const sampleRate = await audioTrack.getSampleRate(); const numberOfChannels = await audioTrack.getNumberOfChannels(); + const { title, artist, album } = await input.getMetadataTags(); ``` diff --git a/changes-and-fixes.txt b/changes-and-fixes.txt new file mode 100644 index 0000000..a7cdfe2 --- /dev/null +++ b/changes-and-fixes.txt @@ -0,0 +1,173 @@ +================================================================================ + MEDIABUNNY HLS BRANCH - BUG FIXES AND BEHAVIOR CHANGES TO EXISTING CODE + For use in release notes. Only covers changes to pre-existing code. + New HLS/CMAF features are not listed here. +================================================================================ + + +============================================================ +BUG FIXES +============================================================ + +1. SourceRef race conditions (commit aa810fc) + - Source cache entry was added BEFORE the reference was fully created, + allowing premature garbage collection of cached sources + - Cache eviction variable pointed to wrong ref (local instead of entry) + - `count > MAX_SOURCE_CACHE_SIZE` off-by-one -> `count >= MAX_SOURCE_CACHE_SIZE` + - Input.source and Input.target getters could return unused/stale instances + when using async callbacks; now properly tracked via _getRootSourceRef() + +2. Track closing race conditions in all muxers (commit 94678dd) + - ISOBMFF, Matroska, MPEG-TS, and OGG muxers used `trackData.track.source._closed` + to check if tracks were closed. This was racy because the closed state could + change between check and use. Added explicit `closed: boolean` field to all + muxer track data structures, set synchronously in onTrackClose(). + +3. Target.slice() validation bug (commit 7b70756) + - Validation used `!Number.isInteger(offset) && offset < 0` (AND), meaning + negative non-integer offsets slipped through. Fixed to use OR (`||`). + +4. MPEG-TS demuxer: off-by-one errors in reorder buffer logic + - Rewind loop changed from `reorderSize` to `reorderSize + 1` iterations, + fixing packet duration calculation near seek points + - End-of-stream rewind loop changed from `reorderSize - 1` to `reorderSize` + - Flush condition changed from `>= reorderSize` to `> reorderSize`, keeping + one extra packet in the buffer before flushing for correct presentation + order computation + +5. MPEG-TS demuxer: first chunk assertion crash (line 1031-1035) + - Old code asserted that a key frame is always found in the first chunk, + crashing on files where that assumption doesn't hold (e.g., HLS segments + starting mid-GOP). Changed to gracefully return null. + +6. MPEG-TS demuxer: video parameters only searched in first packet + - Some muxers place SPS/PPS in later packets. The demuxer now loops through + multiple packets to find AVC/HEVC decoder configuration records instead of + only checking the first one. + +7. ISOBMFF: duration calculation didn't account for sample duration + - `lastPresentedSample()` found the sample with the highest timestamp but + didn't add its duration. Replaced with `presentationSpan()` that correctly + computes `maxEndTimestamp - minTimestamp`. + +8. ISOBMFF demuxer: assertion crash on unavailable data + - Demuxer asserted that read slices are non-null. Changed to gracefully + return null when data is outside available range (important for streaming/ + progressive scenarios). + +9. ISOBMFF demuxer: multiple trun boxes per fragment rejected + - Old code logged a warning and skipped the second trun box. Now correctly + accumulates samples across multiple trun boxes per track per fragment, + maintaining cumulative offset and timestamp state. + +10. Conversion API: track count validation before fan-out consideration + (commit 2f576aa) + - Tracks were validated against the max count before considering that some + track options had `discard: true`. Now validates inside the fan-out loop. + +11. MP3 muxer: frame positions recorded at wrong offset + - Frame byte positions were recorded AFTER writing packet data. Now recorded + BEFORE writing, which is correct for Xing TOC frame offset calculations. + +12. FLAC demuxer: missing STREAMINFO validation + - Added explicit error when STREAMINFO metadata block is missing, + producing a clear "Corrupted FLAC file" message instead of undefined + behavior downstream. + + +============================================================ +BEHAVIOR CHANGES +============================================================ + +1. Default keyFrameInterval changed from 5 seconds to 2 seconds + (commit c6e505a) + Aligns with default HLS segment duration. Affects all video encoding + that doesn't explicitly set keyFrameInterval. + +2. Conversion API copies input pairability graph by default (commit 6410ea9) + When track group is not explicitly specified, conversion now auto-creates + OutputTrackGroups that mirror the input track pairing relationships. This + means converted outputs preserve which tracks are meant to play together. + +3. Matroska demuxer: removed implicit track sorting by default disposition + Tracks are no longer sorted so that default=true tracks come first. + Now all tracks have `primary: false` by default. Callers should use the + getPrimaryVideoTrack()/getPrimaryAudioTrack() methods for selection. + +4. Removed superfluous end-position seeks in muxer finalize methods + (commit 7b70756) + MP3, FLAC, and Matroska muxers no longer seek to the end of file after + writing final metadata. The seek was unnecessary since the writer is + done at that point. + +5. clampCropRectangle now returns a new object instead of mutating input + (src/sample.ts:1123) + +6. Date.now() -> performance.now() in FinalizationRegistry callback + (src/sample.ts:48) + Uses monotonic time for timing measurements. + +7. Codec string parsing: mp4a.40.34 now correctly identified as MP3 + (src/codec.ts:677-686) + Previously would have matched the aac prefix check. MP3 check now + runs first and includes this codec string. + + +============================================================ +DEPRECATIONS +============================================================ + +1. InputTrack sync property getters (commit 051578c) + All sync getters on InputTrack/InputVideoTrack/InputAudioTrack are now + deprecated in favor of async methods: + .codec -> await .getCodec() + .languageCode -> await .getLanguageCode() + .name -> await .getName() + .timeResolution -> await .getTimeResolution() + .disposition -> await .getDisposition() + .displayWidth -> await .getDisplayWidth() + .displayHeight -> await .getDisplayHeight() + .rotation -> await .getRotation() + (etc.) + The sync getters still work for non-HLS inputs but throw when the + backing requires async resolution (e.g., HLS tracks before hydration). + +2. Source.onread callback -> source.on('read', handler) + The onread setter still works but is marked @deprecated. + +3. Target.onwrite callback -> target.on('write', handler) + Same as above. + + +============================================================ +REMOVALS +============================================================ + +1. Target.onfinalized callback (commit 6410ea9) + Removed entirely. Use the 'finalized' event: target.on('finalized', ...). + +2. InputTrackDescriptor concept (commit 051578c) + The entire InputTrackDescriptor API was removed. Its functionality + (pairable tracks, primary track selection) is now directly on InputTrack. + +3. InputTrack[] option for ConversionOptions.tracks (commit 606ee87) + ConversionOptions.tracks now only accepts 'all' | 'primary', no longer + accepts an array of InputTrack instances. + +4. Unnecessary sync getter wrappers (commit 8241820) + Removed deprecated sync getters for: hasOnlyKeyPackets, bitrate, + averageBitrate, isRelativeToUnixEpoch on InputTrack. + + +============================================================ +API ADDITIONS (for context, not release-note-worthy on their own) +============================================================ + +- PacketRetrievalOptions.skipLiveWait (fixes #342) +- DiscardedTrack.trackOptions field +- TargetRequest.mimeType +- TrackDisposition.primary +- Source/Target/Output extend EventEmitter +- CanvasSink methods now async generators (getCanvas, canvases, canvasesAtTimestamps) +- EncodedPacketSink.getFirstPacket/getPacket/getNextPacket now async +- Muxer writers deferred to start() instead of constructor diff --git a/docs/guide/converting-media-files.md b/docs/guide/converting-media-files.md index 5a5235f..cb3178a 100644 --- a/docs/guide/converting-media-files.md +++ b/docs/guide/converting-media-files.md @@ -499,4 +499,4 @@ On the flip side, you can always query which input tracks made it into the outpu const conversion = await Conversion.init({ input, output }); conversion.utilizedTracks; // => InputTrack[] ``` -A track may appear multiple times in this list when [fan-out](#fan-out) produces multiple output tracks from it. +A track may appear multiple times in this list when [fan-out](#track-fan-out) produces multiple output tracks from it. diff --git a/src/conversion.ts b/src/conversion.ts index d6c05b9..665a3e3 100644 --- a/src/conversion.ts +++ b/src/conversion.ts @@ -402,7 +402,9 @@ const validateVideoOptions = (videoOptions: ConversionVideoOptions) => { || (Array.isArray(videoOptions.group) && videoOptions.group.every(x => x instanceof OutputTrackGroup)) ) ) { - throw new TypeError('options.video.group, when provided, must be a string or an array of strings.'); + throw new TypeError( + 'options.video.group, when provided, must be an OutputTrackGroup or an array of OutputTrackGroups.', + ); } }; @@ -462,7 +464,9 @@ const validateAudioOptions = (audioOptions: ConversionAudioOptions) => { || (Array.isArray(audioOptions.group) && audioOptions.group.every(x => x instanceof OutputTrackGroup)) ) ) { - throw new TypeError('options.audio.group, when provided, must be a string or an array of strings.'); + throw new TypeError( + 'options.audio.group, when provided, must be an OutputTrackGroup or an array of OutputTrackGroups.', + ); } }; @@ -612,7 +616,7 @@ export class Conversion { && options.tracks !== 'primary' ) { throw new TypeError( - 'options.tracks, when provded, must be either \'all\' or \'primary\'.', + 'options.tracks, when provided, must be either \'all\' or \'primary\'.', ); } if ( diff --git a/src/encode.ts b/src/encode.ts index ad4381d..8785f1a 100644 --- a/src/encode.ts +++ b/src/encode.ts @@ -203,7 +203,7 @@ export const validateVideoEncodingConfig = (config: VideoEncodingConfig) => { throw new TypeError('config.transform.rotate, when provided, must be 0, 90, 180 or 270.'); } if (config.transform.crop !== undefined) { - validateCropRectangle(config.transform.crop, 'config.'); + validateCropRectangle(config.transform.crop, 'config.transform.'); } if (config.transform.process !== undefined && typeof config.transform.process !== 'function') { throw new TypeError('config.transform.process, when provided, must be a function.'); @@ -219,7 +219,7 @@ export const validateVideoEncodingConfig = (config: VideoEncodingConfig) => { } } if (config.onEncodedPacket !== undefined && typeof config.onEncodedPacket !== 'function') { - throw new TypeError('config.onEncodedChunk, when provided, must be a function.'); + throw new TypeError('config.onEncodedPacket, when provided, must be a function.'); } if (config.onEncoderConfig !== undefined && typeof config.onEncoderConfig !== 'function') { throw new TypeError('config.onEncoderConfig, when provided, must be a function.'); @@ -438,7 +438,7 @@ export const validateAudioEncodingConfig = (config: AudioEncodingConfig) => { } } if (config.onEncodedPacket !== undefined && typeof config.onEncodedPacket !== 'function') { - throw new TypeError('config.onEncodedChunk, when provided, must be a function.'); + throw new TypeError('config.onEncodedPacket, when provided, must be a function.'); } if (config.onEncoderConfig !== undefined && typeof config.onEncoderConfig !== 'function') { throw new TypeError('config.onEncoderConfig, when provided, must be a function.'); diff --git a/src/hls/hls-demuxer.ts b/src/hls/hls-demuxer.ts index 3dd5c62..75ea635 100644 --- a/src/hls/hls-demuxer.ts +++ b/src/hls/hls-demuxer.ts @@ -496,7 +496,7 @@ export class HlsDemuxer extends Demuxer { const channels = mediaTag.attributes.get('channels') ?? variantStream.attributes.get('channels'); const parsedChannels = channels !== null - ? Number(channels) + ? Number(channels.split('/')[0]!) : null; result.push({ diff --git a/src/hls/hls-muxer.ts b/src/hls/hls-muxer.ts index 4c0122f..4012837 100644 --- a/src/hls/hls-muxer.ts +++ b/src/hls/hls-muxer.ts @@ -294,19 +294,21 @@ export class HlsMuxer extends Muxer { } } + const getMetadataKeyForTrack = ({ metadata }: OutputTrack) => { + let key = ''; + key += `${metadata.languageCode ?? UNDETERMINED_LANGUAGE}-`; + key += `${metadata.name ?? ''}-`; + key += `${metadata.disposition?.default ?? true}-`; + key += `${metadata.disposition?.primary ?? false}-`; + key += `${metadata.disposition?.forced ?? false}-`; + + return key; + }; + // Video tracks that can't be paired with any other track always live on the top-level, the question is just if // they need to be separated into #EXT-X-MEDIA tags or not if (unpairedVideoTracks.length > 0) { - const uniqueMetadata = new Set(unpairedVideoTracks.map(({ metadata }) => { - let key = ''; - key += `${metadata.languageCode ?? UNDETERMINED_LANGUAGE}-`; - key += `${metadata.name ?? ''}-`; - key += `${metadata.disposition?.default ?? true}-`; - key += `${metadata.disposition?.primary ?? false}-`; - key += `${metadata.disposition?.forced ?? false}-`; - - return key; - })); + const uniqueMetadata = new Set(unpairedVideoTracks.map(getMetadataKeyForTrack)); if (uniqueMetadata.size > 1) { // They differ in metadata, emit as group @@ -336,16 +338,7 @@ export class HlsMuxer extends Muxer { // Audio tracks that can't be paired with any other track always live on the top-level, the question is just if // they need to be separated into #EXT-X-MEDIA tags or not if (unpairedAudioTracks.length > 0) { - const uniqueMetadata = new Set(unpairedAudioTracks.map(({ metadata }) => { - let key = ''; - key += `${metadata.languageCode ?? UNDETERMINED_LANGUAGE}-`; - key += `${metadata.name ?? ''}-`; - key += `${metadata.disposition?.default ?? true}-`; - key += `${metadata.disposition?.primary ?? false}-`; - key += `${metadata.disposition?.forced ?? false}-`; - - return key; - })); + const uniqueMetadata = new Set(unpairedAudioTracks.map(getMetadataKeyForTrack)); if (uniqueMetadata.size > 1) { // They differ in metadata, emit as group @@ -686,6 +679,17 @@ export class HlsMuxer extends Muxer { // Loop in case we can finalize multiple segments while (true) { + // This here is the core segmentation logic. The segmentation logic figures out which packets are to be + // written into the next segment, and if we can write a segment at all. If tracks are still open and have + // not provided sufficient media data, no segment will be written. The packets will be added to the segment + // to maximize its duration AND keep it from exceeding the target duration. This condition is extended with + // a key frame rule for video, meaning the algorithm must guarantee that every segment with video data + // begins with a video key frame. + // + // The logic is quite complex but is solved in a straight-forward way: all possible permutations of the + // problem are checked in a nested if-else structure, making sure all cases behave correctly. This was the + // easiest, least error-prone way I found to express this behavior. + const currentSegmentEndTimestamp = playlist.currentSegmentStartTimestamp + this.targetSegmentDuration; // These store the index (exclusive) until when packets can be added to the next segment @@ -816,6 +820,9 @@ export class HlsMuxer extends Muxer { if (this.singleFilePerPlaylist) { if (playlist.singleFile === null) { + // INTENTIONALLY shadow the outside `segmentInfo` because we don't want to set it. + // In single-file mode, onSegment is called once in onPlaylistDone instead of per-segment, + // so the outer `segmentInfo` intentionally stays null in this case. const segmentInfo: HlsOutputSegmentInfo = { n: playlist.nextSegmentId, format: playlist.segmentFormat, @@ -1028,6 +1035,7 @@ export class HlsMuxer extends Muxer { assert(Number.isFinite(nextSegmentStartTimestamp)); const segmentDuration = nextSegmentStartTimestamp - playlist.currentSegmentStartTimestamp; + assert(segmentDuration >= 0); playlist.writtenSegments.push({ path: relativeSegmentPath, @@ -1119,7 +1127,8 @@ export class HlsMuxer extends Muxer { // Fallback: if no contiguous set falls within the range, use per-segment max if (peakBitrate === 0) { for (const segment of segments) { - peakBitrate = Math.max(peakBitrate, 8 * segment.byteSize / segment.duration); + const segmentDuration = segment.duration || 1; // To catch 0-duration segments which can happen + peakBitrate = Math.max(peakBitrate, 8 * segment.byteSize / segmentDuration); } } @@ -1166,7 +1175,7 @@ export class HlsMuxer extends Muxer { + (!this.isLive ? '#EXT-X-PLAYLIST-TYPE:VOD\n' : '') + `#EXT-X-TARGETDURATION:${Math.ceil(targetDuration)}\n` // Must be a "decimal-integer" + (Number.isFinite(this.maxLiveSegmentCount) ? `#EXT-X-MEDIA-SEQUENCE:${playlist.mediaSequence}\n` : '') - + '#EXT-X-INDEPENDENT-SEGMENTS\n' // Todo not for live? + + '#EXT-X-INDEPENDENT-SEGMENTS\n' + (isKeyPacketsOnly ? '#EXT-X-I-FRAMES-ONLY\n' : '') + (playlist.initSegment ? (`#EXT-X-MAP:URI="${playlist.initSegment.path}"` diff --git a/src/hls/hls-segmented-input.ts b/src/hls/hls-segmented-input.ts index 977acc8..42e5bbf 100644 --- a/src/hls/hls-segmented-input.ts +++ b/src/hls/hls-segmented-input.ts @@ -187,7 +187,7 @@ export class HlsSegmentedInput extends SegmentedInput { if (!line.startsWith('#')) { if (!prevLastSegment) { if (nextSegmentDuration === null) { - throw new Error('Invalid M3U8 file; a segment must be preceeded by a #EXTINF tag.'); + throw new Error('Invalid M3U8 file; a segment must be preceded by an #EXTINF tag.'); } let key = currentKey; diff --git a/src/mp3/mp3-reader.ts b/src/mp3/mp3-reader.ts index 0226fdf..cd7dd96 100644 --- a/src/mp3/mp3-reader.ts +++ b/src/mp3/mp3-reader.ts @@ -13,23 +13,33 @@ export const readNextMp3FrameHeader = async (reader: Reader, startPos: number, u header: Mp3FrameHeader; startPos: number; } | null> => { + const CHUNK_SIZE = 2 ** 16; // So we don't need to grab thousands of slices let currentPos = startPos; - // todo optimize this shit wtf is this - while (until === null || currentPos < until) { - let slice = reader.requestSlice(currentPos, FRAME_HEADER_SIZE); + const maxLength = until !== null + ? Math.min(CHUNK_SIZE, until - currentPos) + : CHUNK_SIZE; + + let slice = reader.requestSliceRange(currentPos, FRAME_HEADER_SIZE, maxLength); if (slice instanceof Promise) slice = await slice; - if (!slice) break; + if (!slice || slice.length < FRAME_HEADER_SIZE) break; - const word = readU32Be(slice); + while (slice.remainingLength >= FRAME_HEADER_SIZE) { + const posBeforeRead = slice.filePos; + const word = readU32Be(slice); + const remainingBytes = reader.fileSize !== null + ? reader.fileSize - currentPos + : null; - const result = readMp3FrameHeader(word, reader.fileSize !== null ? reader.fileSize - currentPos : null); - if (result.header) { - return { header: result.header, startPos: currentPos }; + const result = readMp3FrameHeader(word, remainingBytes); + if (result.header) { + return { header: result.header, startPos: currentPos }; + } + + slice.filePos = posBeforeRead + result.bytesAdvanced; + currentPos = slice.filePos; } - - currentPos += result.bytesAdvanced; } return null; diff --git a/src/output-format.ts b/src/output-format.ts index b3b040a..f759896 100644 --- a/src/output-format.ts +++ b/src/output-format.ts @@ -1342,6 +1342,9 @@ export class HlsOutputFormat extends OutputFormat { if (options.getSegmentPath !== undefined && typeof options.getSegmentPath !== 'function') { throw new TypeError('options.getSegmentPath, when provided, must be a function.'); } + if (options.getInitPath !== undefined && typeof options.getInitPath !== 'function') { + throw new TypeError('options.getInitPath, when provided, must be a function.'); + } if (options.onMaster !== undefined && typeof options.onMaster !== 'function') { throw new TypeError('options.onMaster, when provided, must be a function.'); } diff --git a/src/output.ts b/src/output.ts index 93487ae..a38a0b5 100644 --- a/src/output.ts +++ b/src/output.ts @@ -445,7 +445,7 @@ export class Output< && typeof options.initTarget !== 'function' ) { throw new Error( - 'options.getInitTarget, when provided, must be a Target or a function that returns or resolves to' + 'options.initTarget, when provided, must be a Target or a function that returns or resolves to' + ' a Target.', ); } @@ -550,6 +550,7 @@ export class Output< target._output = this; if (this.state === 'canceled') { + // Promise thrown away here, but no way to surface it to the user really void target._close(); } else { this._targets.add(target); diff --git a/src/target.ts b/src/target.ts index f250c54..ce54b8c 100644 --- a/src/target.ts +++ b/src/target.ts @@ -594,7 +594,6 @@ export class FilePathTarget extends Target { chunked: true, ...options, }); - this._streamTarget._output = this._output; } /** @internal */ diff --git a/test/node/hls-output.test.ts b/test/node/hls-output.test.ts index cd4e056..3454537 100644 --- a/test/node/hls-output.test.ts +++ b/test/node/hls-output.test.ts @@ -1945,10 +1945,13 @@ test('Single-file mode', async () => { let playlistText: string | null = null; const segmentPaths = new Set(); + const onSegment = vi.fn(); + const output = new Output({ format: new HlsOutputFormat({ segmentFormat: new MpegTsOutputFormat(), singleFilePerPlaylist: true, + onSegment, }), target: new PathedTarget('', (request) => { const target = new BufferTarget(); @@ -1979,6 +1982,8 @@ test('Single-file mode', async () => { await source.add(new EncodedPacket(avcPacketData, 'delta', 3, 0), avcMetadata); await source.add(new EncodedPacket(avcPacketData, 'delta', 3.5, 0), avcMetadata); + expect(onSegment).toHaveBeenCalledTimes(0); + await output.finalize(); // Only one segment file should have been created @@ -1987,6 +1992,8 @@ test('Single-file mode', async () => { expect(playlistText).not.toBeNull(); expect(playlistText!.match(/#EXT-X-BYTERANGE/g)).toHaveLength(2); expect(playlistText).toContain('#EXT-X-VERSION:4'); + + expect(onSegment).toHaveBeenCalledTimes(1); }); test('StreamTarget, write is called for each target', async () => {