Support for catalog in common catalog format #103

TilsonJoji · 2024-07-09T09:05:54Z

Hello Luke @kixelated , thank you for your work, we ( Gwendal Simon , Tilson Joji ) from Synamedia would like to convey our interest in contributing.

With this PR have attempted to support formatting the catalog as per common catalog format defined in spec https://datatracker.ietf.org/doc/html/draft-wilaw-moq-catalogformat-02

A PR has been requested in moq-rs as well to support this change, here is the link to the PR kixelated/moq-rs#174

Kindly review and let us know your thoughts.

commit to support common catalog format for moqtransport

englishm · 2024-07-09T13:44:39Z

Thanks for this, @TilsonJoji (and @gwendalsimon) - the catalog support we have right now is still mostly a placeholder and the format is left over from before the current common catalog spec existed, so I see no reason not to update it both here and in moq-pub!

kixelated

The catalog draft is pretty rough and I need to file a lot of issues.

The catalog draft was adopted by the WG so we should be implementing the latest version. Unfortunately I just noticed that so my review was for the 02 draft you linked.

kixelated · 2024-07-09T13:49:26Z

lib/media/catalog/index.ts

@@ -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")


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

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.

kixelated · 2024-07-09T13:59:51Z

lib/media/catalog/index.ts

 }

 export interface Mp4Track extends Track {
-	container: "mp4"
 	init_track: string


Suggested change

init_track: string

initTrack: string

kixelated · 2024-07-09T14:05:42Z

lib/media/catalog/index.ts

@@ -60,63 +61,68 @@ export function isCatalog(catalog: any): catalog is Catalog {
 }

 export interface Track {
-	kind: string
-	container: string
+	name: string


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.

kixelated · 2024-07-09T14:06:12Z

lib/media/catalog/index.ts

 	bit_rate?: number
 }

+export interface SelectionParamsAudio {
+	bit_rate: number;


Suggested change

bit_rate: number;

bitrate?: number;

kixelated · 2024-07-09T14:06:18Z

lib/media/catalog/index.ts

+	codec: string;
+	height: number;
+	width: number;
+	frame_rate: number


Suggested change

frame_rate: number

framerate?: number

kixelated · 2024-07-09T14:12:06Z

lib/media/catalog/index.ts

+export interface SelectionParamsAudio {
+	bit_rate: number;
+	channel_count: number;
+	codec: string;


Suggested change

codec: string;

codec?: string;

kixelated · 2024-07-09T14:12:16Z

lib/media/catalog/index.ts

+	bit_rate: number;
+	channel_count: number;
+	codec: string;
+	sample_rate: number;


Suggested change

sample_rate: number;

samplerate?: number;

kixelated · 2024-07-09T14:12:49Z

lib/media/catalog/index.ts

+	channel_count: number;
+	codec: string;
+	sample_rate: number;
+	sample_size: number;


This doesn't exist?

kixelated · 2024-07-09T14:14:04Z

lib/media/catalog/index.ts

-	height: number
-	frame_rate: number
-	bit_rate?: number
+	name: "video"


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.

kixelated · 2024-07-09T14:18:23Z

lib/playback/index.ts

@@ -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"))) {


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

TilsonJoji · 2024-07-10T06:48:23Z

Thank you @englishm.

Thank you @kixelated for the detailed review and the feedback . I will address the comments and update the PR.

This is related to previous PR : #1 Addressing review comments received on kixelated#103 and making the catalog compliant with https://www.ietf.org/archive/id/draft-ietf-moq-catalogformat-01.html Related PR in moq-js : TilsonJoji/kixelated-moq-rs#2

TilsonJoji · 2024-07-11T14:41:24Z

Hello Luke @kixelated , Mike @englishm , i have attempted to address the comments and followed the latest version of the spec , kindly review and let me know your thoughts.

kixelated · 2024-07-16T20:07:38Z

Thanks @TilsonJoji , I'll take a look hopefully tomorrow.

Untested because I have to get back to work. :x

kixelated · 2024-07-18T17:29:26Z

@TilsonJoji I pushed some changes inline. It passes the typescript checks (npm run build) but I haven't actually tested it yet, nor updated the Rust side.

kixelated · 2024-07-18T19:11:27Z

I made some issues on the catalog Github if you want to voice any opinions after having implemented this: https://github.com/moq-wg/catalog-format

Commit to address an glitch observed in UT with commit kixelated/moq-js@7baad29 part of PR kixelated#103

TilsonJoji · 2024-07-19T17:17:14Z

Hello Luke @kixelated , thank you for the refactoring the code with commit 7baad29.

Regarding moq-rs , I performed a test and have updated the RUST PR .
The commits in moq-rs PR are [commit to address PR comments in moq-rs , commit to address cargo build warning , and commit to update RS to work with your refactoring commit .

The commits in moq-rs have passed the PR build check. Kindly have a look at the moq-rs PR as time allows and let me know your thoughts.

Regarding moq-js , Have done a minor tweak to your commit to fix issue observed in UT and have pushed the changes in this UT1 commit and this UT2 commit

UT1 performed:

./dev/relay
./dev/pub
Access stream at URL https://IP:4321/watch/bbb?server=IP:4443 ( chrome browser )

UT2

Access URL https://ip:4321/publish?server=IP:4443
Click on Camera
Click on Go live and then on share
Watch the stream at https://ip:4321/watch/82dd0119-de7e-4f5c-8fb1-a4768a29fb5b?server=IP:4443

…roadcast

Commit to address an glitch observed in UT ( broadcast and watch ) with commit kixelated/moq-js@7baad29 part of PR kixelated#103

…05c00e

Commit to make namespace fetch compatible with RS commit kixelated/moq-rs@73bad56 and kixelated/moq-rs@f05c00e in PR : kixelated/moq-rs#174

TilsonJoji · 2024-07-22T10:51:17Z

Hello Luke @kixelated , pushed a commit b8d5a80/511ad99 to update moq-js with regards to namespace access, to make it compatible with commit 73bad56 f05c00e in PR : kixelated/moq-rs#174

…05c00e-2

committosupport-commoncatalogformat-fix-to-workwithRScommit-73bad56-f…

root and others added 2 commits July 8, 2024 14:04

committosupport-commoncatalogformat-for-moqtransport

0215993

Merge pull request #1 from TilsonJoji/catalog-change-jstempBranchTJ

a35dc11

commit to support common catalog format for moqtransport

TilsonJoji mentioned this pull request Jul 9, 2024

Formatting the catalog as per common catalog format kixelated/moq-rs#174

Merged

TilsonJoji marked this pull request as ready for review July 9, 2024 09:07

kixelated requested changes Jul 9, 2024

View reviewed changes

committosupport-commoncatalogformat-addlukereviewcomments-moqjs

f20a5fc

TilsonJoji mentioned this pull request Jul 11, 2024

Addressing review comments from Luke - moqjs TilsonJoji/kixelated-moq-js#2

Merged

Inline PR feedback/refactoring.

7baad29

Untested because I have to get back to work. :x

TilsonJoji mentioned this pull request Jul 19, 2024

committosupport-commoncatalogformat-moqjscommit7baad29-refacfix-chann… TilsonJoji/kixelated-moq-rs#4

Merged

committosupport-commoncatalogformat-minortweak-to-commit-7baad29

33b3834

TilsonJoji mentioned this pull request Jul 19, 2024

committosupport-commoncatalogformat-minortweak-to-commit-7baad29 TilsonJoji/kixelated-moq-js#3

Merged

Merge pull request #3 from TilsonJoji/catalog-change-jstempBranchTJ

3f9e9d7

Commit to address an glitch observed in UT with commit kixelated/moq-js@7baad29 part of PR kixelated#103

committosupport-commoncatalogformat-minortweak-to-commit-7baad29-fixb…

d536d3b

…roadcast

TilsonJoji mentioned this pull request Jul 19, 2024

committosupport-commoncatalogformat-minortweak-to-commit-7baad29-fixb… TilsonJoji/kixelated-moq-js#4

Merged

TilsonJoji and others added 3 commits July 20, 2024 02:11

Merge pull request #4 from TilsonJoji/catalog-change-jstempBranchTJ

d678d5d

Commit to address an glitch observed in UT ( broadcast and watch ) with commit kixelated/moq-js@7baad29 part of PR kixelated#103

committosupport-commoncatalogformat-fix-to-workwithRScommit-73bad56-f…

485a1d6

…05c00e

Merge pull request #5 from TilsonJoji/catalog-change-jstempBranchTJ

b8d5a80

Commit to make namespace fetch compatible with RS commit kixelated/moq-rs@73bad56 and kixelated/moq-rs@f05c00e in PR : kixelated/moq-rs#174

root and others added 3 commits July 22, 2024 13:02

committosupport-commoncatalogformat-fix-to-workwithRScommit-73bad56-f…

5d7a726

…05c00e-2

Merge pull request #6 from TilsonJoji/catalog-change-jstempBranchTJ

511ad99

committosupport-commoncatalogformat-fix-to-workwithRScommit-73bad56-f…

Properly support commonTrackFields.

eb1536a

kixelated merged commit 5158564 into kixelated:main Jul 24, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for catalog in common catalog format #103

Support for catalog in common catalog format #103

TilsonJoji commented Jul 9, 2024 •

edited

Loading

englishm commented Jul 9, 2024

kixelated left a comment

kixelated Jul 9, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

kixelated Jul 9, 2024

kixelated Jul 9, 2024

TilsonJoji Jul 11, 2024

TilsonJoji commented Jul 10, 2024

TilsonJoji commented Jul 11, 2024 •

edited

Loading

kixelated commented Jul 16, 2024

kixelated commented Jul 18, 2024

kixelated commented Jul 18, 2024 •

edited

Loading

TilsonJoji commented Jul 19, 2024 •

edited

Loading

TilsonJoji commented Jul 22, 2024 •

edited

Loading

Support for catalog in common catalog format #103

Support for catalog in common catalog format #103

Conversation

TilsonJoji commented Jul 9, 2024 • edited Loading

englishm commented Jul 9, 2024

kixelated left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TilsonJoji commented Jul 10, 2024

TilsonJoji commented Jul 11, 2024 • edited Loading

kixelated commented Jul 16, 2024

kixelated commented Jul 18, 2024

kixelated commented Jul 18, 2024 • edited Loading

TilsonJoji commented Jul 19, 2024 • edited Loading

TilsonJoji commented Jul 22, 2024 • edited Loading

TilsonJoji commented Jul 9, 2024 •

edited

Loading

TilsonJoji commented Jul 11, 2024 •

edited

Loading

kixelated commented Jul 18, 2024 •

edited

Loading

TilsonJoji commented Jul 19, 2024 •

edited

Loading

TilsonJoji commented Jul 22, 2024 •

edited

Loading