Skip to content

Add subscription parameters to MoQ.Source, enable integration tests on CI - #8

Merged
kidq330 merged 6 commits into
masterfrom
kidq330/add_sub_params
Oct 6, 2026
Merged

kidq330 merged 6 commits into
masterfrom
kidq330/add_sub_params

Conversation

@kidq330

@kidq330 kidq330 commented Sep 15, 2026

Copy link
Copy Markdown
Member

No description provided.

@kidq330 kidq330 self-assigned this Sep 16, 2026
@kidq330 kidq330 added this to Smackore Sep 16, 2026
@kidq330 kidq330 changed the title Add subscription parameters to MoQ.Source Add subscription parameters to MoQ.Source, enable integration tests on CI Sep 16, 2026
@kidq330
kidq330 requested a review from varsill September 16, 2026 13:52
@kidq330 kidq330 moved this to In Review in Smackore Sep 16, 2026
@kidq330
kidq330 marked this pull request as ready for review September 16, 2026 15:32
@kidq330
kidq330 requested review from FelonEkonom and removed request for varsill September 16, 2026 15:32
Comment thread test/integration_test.exs Outdated
@track "video"
@audio_track "audio"

@subscription %ExMoQ.Subscription{group_start: 0, latency_ns: Membrane.Time.seconds(5)}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You shouldn't assume that Membrane.Time is in fact integer meaning number of nanoseconds. It is a part of a private API and it may change in the future

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

good catch, thanks!

get_child(:source)
|> via_out(Pad.ref(:output, generation), options: [track: name])
|> via_out(Pad.ref(:output, generation),
options: [track: name, subscription: @subscription]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it make sense, to require specifying both track name and subscription? Maybe sbd who has some expertise in MoQ could look at it too

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think it's too important, the struct is just a bundle of parameters, we may just as well put :track inside it. It would be closer to moq-lite's structure of SUBSCRIBE, see https://datatracker.ietf.org/doc/draft-lcurley-moq-lite/ §7.7

for comparison, IETF MoQ splits the track name and parameters: https://www.ietf.org/archive/id/draft-ietf-moq-transport-21.html#section-9.6

It makes sense for me to have the current shape with the track name as the "most important" parameter, but it could just as well be a required value for the struct, your call

@kidq330
kidq330 merged commit 7bd372a into master Oct 6, 2026
4 checks passed
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.

2 participants