Skip to content
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

Support for catalog in common catalog format #103

Merged
merged 14 commits into from
Jul 24, 2024
Merged

Support for catalog in common catalog format #103

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
0215993
committosupport-commoncatalogformat-for-moqtransport
Jul 8, 2024
a35dc11
Merge pull request #1 from TilsonJoji/catalog-change-jstempBranchTJ
TilsonJoji Jul 9, 2024
f20a5fc
committosupport-commoncatalogformat-addlukereviewcomments-moqjs
Jul 11, 2024
90482cf
Merge pull request #2 from TilsonJoji/catalog-change-jstempBranchTJ
TilsonJoji Jul 11, 2024
7baad29
Inline PR feedback/refactoring.
kixelated Jul 18, 2024
33b3834
committosupport-commoncatalogformat-minortweak-to-commit-7baad29
Jul 19, 2024
3f9e9d7
Merge pull request #3 from TilsonJoji/catalog-change-jstempBranchTJ
TilsonJoji Jul 19, 2024
d536d3b
committosupport-commoncatalogformat-minortweak-to-commit-7baad29-fixb…
Jul 19, 2024
d678d5d
Merge pull request #4 from TilsonJoji/catalog-change-jstempBranchTJ
TilsonJoji Jul 19, 2024
485a1d6
committosupport-commoncatalogformat-fix-to-workwithRScommit-73bad56-f…
Jul 22, 2024
b8d5a80
Merge pull request #5 from TilsonJoji/catalog-change-jstempBranchTJ
TilsonJoji Jul 22, 2024
5d7a726
committosupport-commoncatalogformat-fix-to-workwithRScommit-73bad56-f…
Jul 22, 2024
511ad99
Merge pull request #6 from TilsonJoji/catalog-change-jstempBranchTJ
TilsonJoji Jul 22, 2024
eb1536a
Properly support commonTrackFields.
kixelated Jul 24, 2024
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
60 changes: 33 additions & 27 deletions lib/media/catalog/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export class Catalog {
const str = decoder.decode(raw)

try {
if (typeof JSON.parse(str).packaging !== "string") throw new Error("invalid catalog")
Copy link
Owner

Choose a reason for hiding this comment

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

This will JSON parse the catalog twice. Also the isCatalog function should perform this check.

Copy link
Owner

Choose a reason for hiding this comment

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

IMO Catalog should be an interface now that there's actually stuff in the root.

export interface Catalog {
  version: number,
  sequence: number,
  streamingFormat: number,
  streamingFormatVersion: string,
  renderGroup: number,
  packaging: string,
  // etc?
  tracks: Track[],
}

It means you'll need to move the other functions into the root but that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

this.tracks = JSON.parse(str).tracks
if (!isCatalog(this)) {
throw new Error("invalid catalog")
Expand Down Expand Up @@ -60,63 +61,68 @@ export function isCatalog(catalog: any): catalog is Catalog {
}

export interface Track {
kind: string
container: string
name: string
Copy link
Owner

Choose a reason for hiding this comment

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

There's a few other fields that should be defined here, basically anything with Location = T.

Unfortunately some of these fields will be optional and default to the value in the root. ex.

let packaging = catalog.tracks[0].packaging ?? catalog.packing;

I think we should automatically populate the track using the default values so you don't have to perform the above every time. But that can be a future PR.

}

export interface Mp4Track extends Track {
container: "mp4"
init_track: string
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
init_track: string
initTrack: string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

data_track: string
}

export interface AudioTrack extends Track {
kind: "audio"
codec: string
channel_count: number
sample_rate: number
sample_size: number
export interface SelectionParamsVideo {
codec: string;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
codec: string;
codec?: string;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

height: number;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
height: number;
height?: number;

width: number;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
width: number;
width?: number;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

frame_rate: number
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
frame_rate: number
framerate?: number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

bit_rate?: number
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
bit_rate?: number
bitrate?: number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

}

export interface SelectionParamsAudio {
Copy link
Owner

Choose a reason for hiding this comment

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

Technically, the draft doesn't distinguish between audio/video types but it definitely should.

bit_rate: number;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
bit_rate: number;
bitrate?: number;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

channel_count: number;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
channel_count: number;
channelConfig?: string;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

codec: string;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
codec: string;
codec?: string;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

sample_rate: number;
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
sample_rate: number;
samplerate?: number;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

sample_size: number;
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

}


export interface AudioTrack extends Track {
name: "audio"
selectionParams: SelectionParamsAudio;
}

export interface VideoTrack extends Track {
kind: "video"
codec: string
width: number
height: number
frame_rate: number
bit_rate?: number
name: "video"
Copy link
Owner

Choose a reason for hiding this comment

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

This is the track name, not the kind. We can't use "audio" or "video" because that would only allow one track of each type.

There's no equivalent to kind in draft-02, which I think is a big oversight. There is only supposed to be a single SelectionParams type instead of splitting them into audio/video.

selectionParams: SelectionParamsVideo;
}

export function isTrack(track: any): track is Track {
if (typeof track.kind !== "string") return false
if (typeof track.container !== "string") return false
if (typeof track.name !== "string") return false
return true
}

export function isMp4Track(track: any): track is Mp4Track {
if (track.container !== "mp4") return false
if (typeof track.init_track !== "string") return false
if (typeof track.data_track !== "string") return false
if (!isTrack(track)) return false
return true
}

export function isVideoTrack(track: any): track is VideoTrack {
if (track.kind !== "video") return false
if (typeof track.codec !== "string") return false
if (typeof track.width !== "number") return false
if (typeof track.height !== "number") return false
if (!(track.name.toLowerCase().includes("video"))) return false
if (typeof track.selectionParams.codec !== "string") return false
if (typeof track.selectionParams.width !== "number") return false
if (typeof track.selectionParams.height !== "number") return false
if (!isTrack(track)) return false
return true
}

export function isAudioTrack(track: any): track is AudioTrack {
if (track.kind !== "audio") return false
if (typeof track.codec !== "string") return false
if (typeof track.channel_count !== "number") return false
if (typeof track.sample_rate !== "number") return false
if (typeof track.sample_size !== "number") return false
if (!(track.name.toLowerCase().includes("audio"))) return false
if (typeof track.selectionParams.codec !== "string") return false
if (typeof track.selectionParams.channel_count !== "number") return false
if (typeof track.selectionParams.sample_rate !== "number") return false
if (typeof track.selectionParams.sample_size !== "number") return false
if (!isTrack(track)) return false
return true
}
6 changes: 3 additions & 3 deletions lib/playback/backend.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ export default class Backend {

for (const track of config.catalog.tracks) {
if (isAudioTrack(track)) {
if (sampleRate && track.sample_rate !== sampleRate) {
if (sampleRate && track.selectionParams.sample_rate !== sampleRate) {
throw new Error(`TODO multiple audio tracks with different sample rates`)
}

sampleRate = track.sample_rate
channels = Math.max(track.channel_count, channels ?? 0)
sampleRate = track.selectionParams.sample_rate
channels = Math.max(track.selectionParams.channel_count, channels ?? 0)
}
}

Expand Down
6 changes: 3 additions & 3 deletions lib/playback/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ export class Player {
}

async #runTrack(track: Mp4Track) {
if (track.kind !== "audio" && track.kind !== "video") {
throw new Error(`unknown track kind: ${track.kind}`)
if (!(track.name.toLowerCase().includes("audio")) && !(track.name.toLowerCase().includes("video"))) {
Copy link
Owner

Choose a reason for hiding this comment

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

isAudioTrack(track) or isVideoTrack(track) is the way you're supposed to determine the type now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed

throw new Error(`unknown track name: ${track.name}`)
}

const sub = await this.#connection.subscribe(this.#catalog.namespace, track.data_track)
Expand All @@ -118,7 +118,7 @@ export class Player {

this.#backend.segment({
init: track.init_track,
kind: track.kind,
kind: track.name,
header: segment.header,
buffer,
stream,
Expand Down
Loading