-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: support media, html sources and websocket #20
base: main
Are you sure you want to change the base?
Conversation
Bumps [find-my-way](https://github.com/delvedor/find-my-way) to 8.2.2 and updates ancestor dependency [fastify](https://github.com/fastify/fastify). These dependencies need to be updated together. Updates `find-my-way` from 7.6.2 to 8.2.2 - [Release notes](https://github.com/delvedor/find-my-way/releases) - [Commits](delvedor/find-my-way@v7.6.2...v8.2.2) Updates `fastify` from 4.17.0 to 4.28.1 - [Release notes](https://github.com/fastify/fastify/releases) - [Commits](fastify/fastify@4.17.0...v4.28.1) --- updated-dependencies: - dependency-name: find-my-way dependency-type: indirect - dependency-name: fastify dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [semver](https://github.com/npm/node-semver) to 7.5.4 and updates ancestor dependency [nodemon](https://github.com/remy/nodemon). These dependencies need to be updated together. Updates `semver` from 7.0.0 to 7.5.4 - [Release notes](https://github.com/npm/node-semver/releases) - [Changelog](https://github.com/npm/node-semver/blob/main/CHANGELOG.md) - [Commits](npm/node-semver@v7.0.0...v7.5.4) Updates `nodemon` from 2.0.22 to 3.1.7 - [Release notes](https://github.com/remy/nodemon/releases) - [Commits](remy/nodemon@v2.0.22...v3.1.7) --- updated-dependencies: - dependency-name: semver dependency-type: indirect - dependency-name: nodemon dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
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.
Some initial code comments.
In general, try to avoid reformatting existing code to much.
src/api/ateliereLive/websocket.ts
Outdated
// const send = ws.send.bind(ws); | ||
// ws.send = (message) => { | ||
// console.debug(`[websocket] sending message: ${message}`); | ||
// send(message); | ||
// }; |
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.
Remove this
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.
resolved
// TODO: When possible to edit layout, uncomment the following code | ||
// const [modalOpen, setModalOpen] = useState(false); | ||
// const toggleConfigModal = () => { | ||
// setModalOpen((state) => !state); | ||
// }; | ||
|
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.
Remove this
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.
resolved
// <> | ||
// <button | ||
// onClick={toggleConfigModal} | ||
// title={t('preset.configure_layout')} | ||
// className={`absolute top-0 right-[-10%] min-w-fit`} | ||
// > | ||
// <IconSettings className="text-p" /> | ||
// </button> | ||
// {modalOpen && ( | ||
// <div className="absolute top-5 right-[-65%] flex flex-col"> | ||
// <button | ||
// type="button" | ||
// className={`min-w-fit bg-zinc-700 rounded-t-sm p-1 border-b-[1px] border-b-zinc-600 hover:bg-zinc-600`} | ||
// onClick={() => openConfigModal('create')} | ||
// > | ||
// {t('preset.create_layout')} | ||
// </button> | ||
// <button | ||
// type="button" | ||
// className={`min-w-fit bg-zinc-700 rounded-b-sm p-1 hover:bg-zinc-600`} | ||
// onClick={() => openConfigModal('edit')} | ||
// > | ||
// {t('preset.update_layout')} | ||
// </button> | ||
// </div> | ||
// )} | ||
// </> |
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.
Remove the comments
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.
resolved
|
||
const stream: PipelineStreamSettings = { | ||
ingest_id: ingestUuid, | ||
source_id: sourceId, | ||
pipeline_id: pipeline.pipeline_id!, | ||
input_slot: input_slot, | ||
alignment_ms: pipeline.alignment_ms, | ||
audio_format: pipeline.audio_format, | ||
audio_sampling_frequency: pipeline.audio_sampling_frequency, | ||
bit_depth: pipeline.bit_depth, | ||
convert_color_range: pipeline.convert_color_range, | ||
encoder: pipeline.encoder, | ||
encoder_device: pipeline.encoder_device, | ||
format: pipeline.format, | ||
max_network_latency_ms: pipeline.max_network_latency_ms, | ||
width: pipeline.width, | ||
height: pipeline.height, | ||
frame_rate_d: pipeline.frame_rate_d, | ||
frame_rate_n: pipeline.frame_rate_n, | ||
format: pipeline.format, | ||
encoder: pipeline.encoder, | ||
encoder_device: pipeline.encoder_device, | ||
gop_length: pipeline.gop_length, | ||
height: pipeline.height, | ||
max_network_latency_ms: pipeline.max_network_latency_ms, | ||
pic_mode: pipeline.pic_mode, | ||
speed_quality_balance: pipeline.speed_quality_balance, | ||
video_kilobit_rate: pipeline.video_kilobit_rate, | ||
width: pipeline.width, | ||
ingest_id: ingestUuid, | ||
source_id: sourceId, | ||
input_slot, | ||
bit_depth: pipeline.bit_depth, | ||
speed_quality_balance: pipeline.speed_quality_balance, | ||
convert_color_range: pipeline.convert_color_range, | ||
audio_sampling_frequency: pipeline.audio_sampling_frequency, | ||
audio_format: pipeline.audio_format, |
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.
Avoid moving around the order of lines of code, is there a difference here? Can we revert this to old order (which I guess didn't make sense, but new order doesn't make it more clear either?)
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.
The idea was to move it around so that it matched the order in the PipelineStreamSettings type declaration, but I can change it back
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.
resolved
@@ -20,7 +20,6 @@ export async function GET( | |||
status: 403 | |||
}); | |||
} | |||
|
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.
it's better with a newline here
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.
resolved
@@ -15,7 +15,6 @@ export async function POST(request: NextRequest): Promise<NextResponse> { | |||
status: 403 | |||
}); | |||
} | |||
|
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.
it's better with a newline here
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.
resolved
@@ -63,14 +64,17 @@ export const en = { | |||
}, | |||
production_configuration: 'Production Configuration', | |||
production: { | |||
add_source: 'Add Source', | |||
add_source: 'Add ingest', |
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.
We're not to sure about this rephrasing from source to ingest.. we'll need to come back to that
@@ -65,14 +66,17 @@ export const sv = { | |||
}, | |||
production_configuration: 'Produktionskonfiguration', | |||
production: { | |||
add_source: 'Lägg till källa', | |||
add_source: 'Lägg till ingång', |
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.
same as above with rephrasing
pic_mode: 'pic_mode_ip' | ||
{ | ||
_id: new ObjectId('65cb266c00fecda4a1faf977'), | ||
name: '12 inputs HD', |
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.
Aren't there 13 inputs here?
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.
resolved
ws.onopen = () => { | ||
if (action === 'closeHtml') { | ||
ws.send(`html close ${inputSlot}`); | ||
ws.send('html reset'); |
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.
Why is "reset" here? And 3 rows down. Doesn't look necessary.
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.
resolved
src/api/ateliereLive/websocket.ts
Outdated
}, | ||
closeHtml: (input: number) => { | ||
ws.send(`html close ${input}`); | ||
ws.send('html reset'); |
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.
reset not necessary?
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.
resolved
src/api/ateliereLive/websocket.ts
Outdated
}, | ||
closeMediaplayer: (input: number) => { | ||
ws.send(`media close ${input}`); | ||
ws.send('media reset'); |
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.
reset not necessary?
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.
resolved
Added features: