Skip to content

Conversation

@Noarkhh
Copy link
Contributor

@Noarkhh Noarkhh commented Oct 22, 2025

No description provided.

@Noarkhh Noarkhh self-assigned this Oct 22, 2025
@Noarkhh Noarkhh added this to Smackore Oct 22, 2025
@Noarkhh Noarkhh moved this to In Progress in Smackore Oct 22, 2025
@Noarkhh Noarkhh requested a review from varsill October 22, 2025 12:33
@Noarkhh Noarkhh assigned Noarkhh and unassigned Noarkhh Oct 22, 2025
@Noarkhh Noarkhh requested a review from mat-hek October 22, 2025 12:33
@Noarkhh Noarkhh force-pushed the update-rtsp-tutorial branch from 394cdc4 to 441efee Compare October 22, 2025 12:41
@Noarkhh Noarkhh force-pushed the update-rtsp-tutorial branch from 441efee to 0325c72 Compare October 23, 2025 13:54
@Noarkhh Noarkhh moved this from In Progress to In Review in Smackore Oct 23, 2025
Comment on lines 1 to 3
As explained in the [Architecture chapter](08_RTSP_Architecture.md), the pipeline will consist of a RTSP Source and a HLS Sink.

The initial pipeline will consist of the `RTSP Source`, which will start establishing the connection with the RTSP server, and the `HLS Sink Bin`. For now we won't connect this elements in any way, since we don't have information about what tracks we'll receive from the RTSP server which we're connecting with.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think these first two sentences are almost the same

def handle_init(_context, options) do
spec = [
child(:source, %Membrane.RTSP.Source{
transport: {:udp, options.port, options.port + 5},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why "+5"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It defines a range of ports to use, +5 just to be safe if some of them turn out to be in use

Copy link
Collaborator

@varsill varsill Nov 12, 2025

Choose a reason for hiding this comment

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

let's give it some name, perhaps let's add a module's attribute like @port_range_size or sth

@mat-hek mat-hek removed their request for review November 3, 2025 13:21
@Noarkhh Noarkhh requested a review from varsill November 5, 2025 10:59
@Noarkhh Noarkhh force-pushed the update-rtsp-tutorial branch from 7831db1 to 4d848b0 Compare November 5, 2025 11:04
As explained in the [Architecture chapter](08_RTSP_Architecture.md), the pipeline will consist of a RTSP Source and a HLS Sink.

The initial pipeline will consist of the `RTSP Source`, which will start establishing the connection with the RTSP server, and the `HLS Sink Bin`. For now we won't connect this elements in any way, since we don't have information about what tracks we'll receive from the RTSP server which we're connecting with.
As explained in the [Architecture chapter](08_RTSP_Architecture.md), the pipeline will consist of an RTSP Source and an HLS Sink Bin. For now we won't connect this elements in any way, since we don't have information about what tracks we'll receive from the RTSP server which we're connecting with.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
As explained in the [Architecture chapter](08_RTSP_Architecture.md), the pipeline will consist of an RTSP Source and an HLS Sink Bin. For now we won't connect this elements in any way, since we don't have information about what tracks we'll receive from the RTSP server which we're connecting with.
As explained in the [Architecture chapter](08_RTSP_Architecture.md), the pipeline will consist of an RTSP Source and an HLS Sink Bin. For now we won't connect this elements in any way, since we don't have information about what tracks we'll receive from the RTSP server which we're connecting to.

@Noarkhh Noarkhh requested a review from varsill November 13, 2025 14:30
Copy link
Collaborator

@varsill varsill left a comment

Choose a reason for hiding this comment

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

🥇

@Noarkhh Noarkhh merged commit 37201f6 into main Nov 13, 2025
@Noarkhh Noarkhh deleted the update-rtsp-tutorial branch November 13, 2025 14:31
@github-project-automation github-project-automation bot moved this from In Review to Done in Smackore Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants