Skip to content

Refactor NDI name tokens: replace <filter>/<source> with ${filter}/${source}#1445

Merged
Trouffman merged 5 commits into
masterfrom
copilot/refactor-filter-source-tokens
Apr 22, 2026
Merged

Refactor NDI name tokens: replace <filter>/<source> with ${filter}/${source}#1445
Trouffman merged 5 commits into
masterfrom
copilot/refactor-filter-source-tokens

Conversation

Copilot AI commented Apr 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

NDI filter name replacement tokens used angle-bracket syntax (<filter>, <source>). This modernizes them exclusively to the brace-style ${filter}/${source} syntax. Legacy forms (<filter>, <source>) and bare $filter/$source are no longer supported.

Changes

  • src/ndi-filter.cpp: Updated ndi_sender_create to recognize only ${source} and ${filter} tokens. All legacy forms have been removed. Token presence is now checked against the original template string before any replacements are applied, preventing an injected source or filter name that contains a token string from triggering unintended expansion:

    // Before
if (ndi_name.contains("<source>")) {
    ndi_name.replace("<source>", QString(parent_name));
}

// After
const bool has_source_token = ndi_name.contains("${source}");
const bool has_filter_token = ndi_name.contains("${filter}");

if (has_source_token) {
    ndi_name.replace("${source}", QString(parent_name));
}
if (has_filter_token) {
    ndi_name.replace("${filter}", QString(filter_name));
}

  • data/locale/en-US.ini: Updated the default NDI name value to use the new syntax:

    # Before
NDIPlugin.FilterProps.NDIName.Default="<filter> (<source>)"

# After
NDIPlugin.FilterProps.NDIName.Default="${filter} (${source})"

Copilot AI linked an issue Apr 21, 2026 that may be closed by this pull request
Refactor <filter>/<source> token replacements to $filter/$source and support ${filter}/${source} #1444
Closed
Copilot AI changed the title [WIP] Refactor <filter>/<source> token replacements to $filter/$source Refactor NDI name tokens: replace <filter>/<source> with $filter/$source Apr 21, 2026
Copilot AI requested a review from paulpv April 21, 2026 18:37
@Trouffman

Trouffman commented Apr 21, 2026

Copy link
Copy Markdown
Collaborator

Can we keep the brace option only?

I could see problem if someone want to use the $ character in their naming.

paulpv
paulpv requested changes Apr 21, 2026
View reviewed changes
Comment thread src/ndi-filter.cpp Outdated
Comment thread src/ndi-filter.cpp Outdated

Copilot AI commented Apr 21, 2026

Copy link
Copy Markdown
Contributor Author

Can we keep the brace option only?

I could see problem if someone want to use the $ character in their naming.
...

Done in acdc846. Dropped bare $filter/$source support — only ${filter}/${source} (and the legacy <filter>/<source> for backward compat) are now recognized.

Copilot AI changed the title Refactor NDI name tokens: replace <filter>/<source> with $filter/$source Refactor NDI name tokens: replace <filter>/<source> with ${filter}/${source} Apr 21, 2026
Copilot AI requested a review from paulpv April 21, 2026 19:06
@paulpv

paulpv commented Apr 21, 2026

Copy link
Copy Markdown
Member

Can we keep the brace option only?

I could see problem if someone want to use the $ character in their naming.

@Trouffman A single $ anywhere shouldn't cause any problem.
Are you saying to not support $filter or $source at all, and only support ${filter} and ${source}?
Can we completely remove <filter> and <source>?

@paulpv
paulpv marked this pull request as ready for review April 21, 2026 19:12
Copilot AI review requested due to automatic review settings April 21, 2026 19:12
Copilot AI reviewed Apr 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Modernizes NDI filter name template tokens from legacy angle-bracket placeholders (<filter>, <source>) to brace-style placeholders (${filter}, ${source}), while retaining backward compatibility with the legacy forms.

Changes:

  • Updated ndi_sender_create to recognize and replace both ${source}/${filter} and legacy <source>/<filter>.
  • Updated the default localized filter NDI name template to use the new ${...} syntax.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ndi-filter.cpp Adds ${source}/${filter} token detection and replacement while keeping <source>/<filter> working.
data/locale/en-US.ini Updates the default filter NDI name template string to the new ${filter} (${source}) format.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ndi-filter.cpp Outdated
@Trouffman

Trouffman commented Apr 21, 2026

Copy link
Copy Markdown
Collaborator

I'm okay to only support the brace.

And disable the <> support too. We will release a temp update very quickly

paulpv
paulpv previously requested changes Apr 21, 2026
View reviewed changes

@paulpv paulpv left a comment

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.

@copilot Only support ${filter} and ${source}; Remove legacy support for $filter, <filter>, $source, and <source>.

Copilot AI commented Apr 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@copilot Only support ${filter} and ${source}; Remove legacy support for $filter, <filter>, $source, and <source>.

Done in 4be9348. Only ${filter} and ${source} are now supported — all legacy forms (<filter>, <source>, $filter, $source) have been removed. Also fixed the mutation bug flagged in the review: token presence is now checked on the original template string before any replacements run, so an injected name containing ${filter} or ${source} can't trigger unintended expansion.

I'm okay to only support the brace.

And disable the <> support too. We will release a temp update very quickly

Done in 4be9348. <filter> and <source> legacy support has been removed — only ${filter} and ${source} are recognized now.

Copilot AI requested a review from paulpv April 21, 2026 21:02
@Trouffman

Trouffman commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

@paulpv - can you confirm when this is tested ? :D

@paulpv

paulpv commented Apr 22, 2026

Copy link
Copy Markdown
Member

@paulpv - can you confirm when this is tested ? :D

Sorry, I had/have no plans to do any [focused?] testing on this. :/

@Trouffman

Trouffman commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

Tested on Win 11
OBS 32.1.1

It does work and does not use the previous system also tested with additional fudging on the name

image

@paulpv
paulpv dismissed their stale review April 22, 2026 20:46

Dismissing because GitHub Copilot was messing up today (confirmed by https://www.githubstatus.com) and I might have been clicking buttons too fast.

@Trouffman
Trouffman self-requested a review April 22, 2026 20:47
@Trouffman
Trouffman merged commit 428d75b into master Apr 22, 2026
9 checks passed
@Trouffman
Trouffman deleted the copilot/refactor-filter-source-tokens branch April 22, 2026 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@Trouffman Trouffman Trouffman approved these changes
+1 more reviewer
@paulpv paulpv paulpv approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@paulpv paulpv

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor <filter>/<source> token replacements to $filter/$source and support ${filter}/${source}

4 participants

@Trouffman @paulpv