Skip to content

nginxInfo: report dashName alongside ffmpegFlags - #84

Merged
mormegil6 merged 2 commits into
EnvelopSound:masterfrom
mormegil6:nginxinfo-dashname
Aug 22, 2026
Merged

nginxInfo: report dashName alongside ffmpegFlags#84
mormegil6 merged 2 commits into
EnvelopSound:masterfrom
mormegil6:nginxinfo-dashname

Conversation

@mormegil6

Copy link
Copy Markdown
Collaborator

/nginxInfo exists to tell a client how this server is configured, and currently returns only ffmpegFlags.

The manifest is written to ${DASH_NAME}.mpd. That name is configurable and need not match the publish name, so a client has no reliable way to work out which URL to request. This adds the field it would need:

{"ffmpegFlags":"...","dashName":"..."}

One field, added to both nginx.conf and nginx-no-ssl.conf so the two stay identical.

The manifest is written to ${DASH_NAME}.mpd, which is configurable and
need not match the publish name, so a client cannot reliably guess the
URL it should request. /nginxInfo already exists to tell a client how
this server is configured; it just never reported this part.

One field, both config variants kept identical.
Copilot AI lite review requested due to automatic review settings August 22, 2026 20:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the /nginxInfo endpoint so clients can discover the configured DASH manifest base name (dashName) alongside the existing ffmpegFlags, enabling clients to reliably construct the correct ${DASH_NAME}.mpd URL.

Changes:

  • Add "dashName":"${DASH_NAME}" to the JSON returned by /nginxInfo in the SSL nginx config.
  • Mirror the same /nginxInfo change in the non-SSL nginx config to keep the two configurations aligned.

Reviewed changes

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

File Description
nginx-transcoder/nginx.conf Extends /nginxInfo JSON payload to include dashName.
nginx-transcoder/nginx-no-ssl.conf Applies the same /nginxInfo JSON payload change for non-SSL mode.
Suppressed comments (2)

nginx-transcoder/nginx.conf:163

  • This JSON is built by string concatenation with ${FFMPEG_FLAGS}. The repo’s own docker-compose example for FFMPEG_FLAGS includes double quotes (e.g. -filter:v:0 "scale=320:-1"), which would make /nginxInfo emit invalid JSON and potentially allow response-shaping if quotes/newlines appear. If /nginxInfo is intended to be machine-readable, consider encoding/escaping FFMPEG_FLAGS (e.g., URL/base64 encoding) or changing the response shape so it can’t be broken by quotes.
            return 200 '{"ffmpegFlags":"${FFMPEG_FLAGS}","dashName":"${DASH_NAME}"}';

nginx-transcoder/nginx-no-ssl.conf:146

  • This JSON is built by string concatenation with ${FFMPEG_FLAGS}. The repo’s own docker-compose example for FFMPEG_FLAGS includes double quotes (e.g. -filter:v:0 "scale=320:-1"), which would make /nginxInfo emit invalid JSON and potentially allow response-shaping if quotes/newlines appear. If /nginxInfo is intended to be machine-readable, consider encoding/escaping FFMPEG_FLAGS (e.g., URL/base64 encoding) or changing the response shape so it can’t be broken by quotes.
            return 200 '{"ffmpegFlags":"${FFMPEG_FLAGS}","dashName":"${DASH_NAME}"}';

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 144 to 147
# static nginx runtime info served to webtools
location /nginxInfo {
return 200 '{"ffmpegFlags":"${FFMPEG_FLAGS}"}';
return 200 '{"ffmpegFlags":"${FFMPEG_FLAGS}","dashName":"${DASH_NAME}"}';
}
Comment on lines 161 to 164
# static nginx runtime info served to webtools
location /nginxInfo {
return 200 '{"ffmpegFlags":"${FFMPEG_FLAGS}"}';
return 200 '{"ffmpegFlags":"${FFMPEG_FLAGS}","dashName":"${DASH_NAME}"}';
}
The endpoint returns JSON but the location set no type, so it inherited
the default octet-stream and browsers offered it as a download. Predates
this change, but this PR already edits both blocks.
@mormegil6

Copy link
Copy Markdown
Collaborator Author

Fair point, and taken. It predates this change, but the PR is already editing both /nginxInfo blocks so it belongs here rather than in a follow-up.

default_type application/json; added to nginx.conf and nginx-no-ssl.conf, keeping the two identical.

@mormegil6
mormegil6 merged commit a574038 into EnvelopSound:master Aug 22, 2026
4 checks passed
@mormegil6
mormegil6 deleted the nginxinfo-dashname branch August 22, 2026 20:28
mormegil6 added a commit to mormegil6/Earshot that referenced this pull request Aug 22, 2026
Both PRs were cut from this fork, so their changes arrive as conflicts
against the local copies they were taken from. Upstream's side wins in
all three: it is the same change plus the content-type line that review
added. The deployment-only patches in these files are untouched.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants