Skip to content

Conversation

@prk0ghy
Copy link

@prk0ghy prk0ghy commented Apr 27, 2025

Hello caiocotts, Hello deluan,
thank you for that amazing project, wrote some documentation that I hope you can use.
Cheers
prk0ghy


Important

Adds documentation for transcoding configuration and updates configuration options in Navidrome.

  • New Documentation:
    • Adds transcoding-configuration/index.md for configuring transcoding in Navidrome.
    • Includes instructions for setting transcoding per player via web UI.
    • Lists file formats that might need transcoding.
  • Configuration Options:
    • Adds AutoTranscodeDownload and EnableTranscodingConfig to configuration-options.md.
    • Corrects entries for ArtistArtPriority and AutoTranscodeDownload in configuration-options.md.
    • Adds Scanner.ArtistJoiner, Scanner.ScanOnStartup, and Scanner.Extractor to configuration-options.md.

This description was created by Ellipsis for 731e121. You can customize this summary. It will automatically update as commits are pushed.

Copy link
Member

@deluan deluan left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. There's some syntax error:

Error: error building site: process: readAndProcessContent: "/opt/build/repo/content/en/docs/usage/transcoding-configuration/index.md:46:46": unrecognized character in shortcode action: U+003E '>'. Note: Parameters with non-alphanumeric args must be quoted

#### Advanced Tips

Most options can be seen directly in the [conf/configuration.go](https://github.com/navidrome/navidrome/blob/master/conf/configuration.go) source.
But beware, even when setting them in your `navidrome.toml` they can be ignored.
Copy link
Member

Choose a reason for hiding this comment

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

In which situations?

@deluan
Copy link
Member

deluan commented Apr 28, 2025

@ellipsis-dev review this PR

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 731e121 in 3 minutes and 55 seconds. Click for details.
  • Reviewed 218 lines of code in 2 files
  • Skipped 3 files when reviewing.
  • Skipped posting 13 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. content/en/docs/usage/configuration-options.md:147
  • Draft comment:
    Duplicate entry found: 'DefaultPlaylistPublicVisibility' appears twice. Remove duplication.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. content/en/docs/usage/transcoding-configuration/index.md:46
  • Draft comment:
    Image shortcode syntax might be missing a closing delimiter. Verify proper usage of '{{< imgproc ... >}}'.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 40% <= threshold 50% The comment is asking the author to verify the usage of a shortcode, which is not allowed as it asks for verification. However, it does point out a potential syntax issue, which could be useful if rephrased as a suggestion or observation.
3. content/en/docs/usage/configuration-options.md:135
  • Draft comment:
    The environment variable names seem swapped. 'ArtistArtPriority' should use ND_ARTISTARTPRIORITY with a description about artist images, and 'AutoTranscodeDownload' should use ND_AUTOTRANSCODEDOWNLOAD for enabling transcoding download.
  • Reason this comment was not posted:
    Marked as duplicate.
4. content/en/docs/usage/configuration-options.md:194
  • Draft comment:
    Typo in 'Scanner.Extractor': 'extractrion' should be 'extraction'.
  • Reason this comment was not posted:
    Marked as duplicate.
5. content/en/docs/usage/transcoding-configuration/index.md:43
  • Draft comment:
    Typo: 'Trough' should be 'Through' and 'schould' should be 'should'.
  • Reason this comment was not posted:
    Marked as duplicate.
6. content/en/docs/usage/transcoding-configuration/index.md:83
  • Draft comment:
    Typo detected: 'wether' should be 'whether' and 'firt' should be 'first' in the description of file format support.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While typos are real, our rules say to only keep comments that require code changes. Documentation typos are more like editorial feedback. The typos don't impact functionality and could be caught in documentation review. However, since this is a new file being added, fixing basic spelling errors before merging could be valuable. Documentation quality does matter for user experience. Poor spelling can make docs look unprofessional and harder to read. While documentation quality matters, typo fixes are minor editorial changes that don't require deep technical review. This kind of feedback could be handled through other channels. Following our strict rules about only keeping comments that require clear code changes, we should remove this comment about documentation typos.
7. content/en/docs/usage/transcoding-configuration/index.md:129
  • Draft comment:
    Consider revising the informal language ('yes i asked a [llm] for this list') to maintain a professional tone.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
8. content/en/docs/usage/configuration-options.md:135
  • Draft comment:
    The environment variable names for 'ArtistArtPriority[][artistcoverart]' and 'AutoTranscodeDownload' appear to be swapped. It seems that 'ArtistArtPriority[][artistcoverart]' is mapped to ND_AUTOTRANSCODEDOWNLOAD while 'AutoTranscodeDownload' is mapped to ND_ARTISTARTPRIORITY. Please verify and correct these to match their intended functionality.
  • Reason this comment was not posted:
    Marked as duplicate.
9. content/en/docs/usage/configuration-options.md:194
  • Draft comment:
    In the description for 'Scanner.Extractor', "tag extractrion" should be corrected to "tag extraction".
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is purely informative as it points out a typo in the description. It doesn't suggest a code change or improvement.
10. content/en/docs/usage/configuration-options.md:76
  • Draft comment:
    The tab header 'macOs' should be capitalized as 'macOS' to match the proper naming convention.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. content/en/docs/usage/transcoding-configuration/index.md:43
  • Draft comment:
    Typo: 'Trough' should be 'Through', and 'schould' should be 'should' in the sentence 'Trough the webui you can specify if one of your clients schould use transcoding:'.
  • Reason this comment was not posted:
    Marked as duplicate.
12. content/en/docs/usage/transcoding-configuration/index.md:84
  • Draft comment:
    Typo: 'firt' should be 'first' in the phrase 'from where the firt list stems.'
  • Reason this comment was not posted:
    Marked as duplicate.
13. content/en/docs/usage/transcoding-configuration/index.md:129
  • Draft comment:
    Typo: Consider capitalizing the pronoun 'i' to 'I' and 'llm' to 'LLM' in 'yes i asked a llm for this list'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a very minor style issue in an informal note. The text is in italics and clearly meant to be a casual aside. The meaning is perfectly clear regardless of capitalization. This kind of nitpicky comment about capitalization doesn't improve the technical accuracy or usability of the documentation. Perhaps maintaining consistent professional style throughout documentation, even in casual notes, helps with overall documentation quality and readability. While consistency is good, this comment is about an intentionally informal aside that's marked as such with italics. The informality actually serves a purpose in distinguishing this personal note from the technical content. This comment should be removed as it addresses a trivial capitalization issue in an intentionally informal note that doesn't impact the documentation's technical accuracy or usefulness.

Workflow ID: wflow_XGfHlk49dNkM9XZH

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

| In config file | As an environment variable | Description | Default Value |
|------------------------------------------------|--------------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------|
| AlbumPlayCountMode | `ND_ALBUMPLAYCOUNTMODE` | Change how album play count is computed. When set to `"normalized"`, album play count will be divided by the number of album tracks | `"absolute"` |
| ArtistArtPriority[\*][artistcoverart] | `ND_AUTOTRANSCODEDOWNLOAD` | Enable toggle switch in the webui to download a transcoded file. | `"artist.*, album/artist.*, external"` |
Copy link

Choose a reason for hiding this comment

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

Potential swap: 'ArtistArtPriority' is mapped to ND_AUTOTRANSCODEDOWNLOAD and 'AutoTranscodeDownload' to ND_ARTISTARTPRIORITY. Check variable names.

| Scanner.Enabled | `ND_SCANNER_ENABLED` | Enable/disable the scanner. Set to `false` to disable automatic scanning of the music library. | `true` |
| Scanner.ScanOnStartup | `ND_SCANNER_SCANONSTARTUP` | Enable/disable scanning the music library on startup. Set to `false` to disable | `true` |
| Scanner.Schedule | `ND_SCANNER_SCHEDULE` | Schedule for automatic scans. Use [Cron syntax][cronspec] | `0` (disabled) |
| Scanner.Extractor | `ND_SCANNER_EXTRACTOR` | Method of tag extractrion Options: `ffmpeg`,`taglib` | `taglib` |
Copy link

Choose a reason for hiding this comment

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

Typo in 'tag extractrion' — should be 'tag extraction'.

Suggested change
| Scanner.Extractor | `ND_SCANNER_EXTRACTOR` | Method of tag extractrion Options: `ffmpeg`,`taglib` | `taglib` |
| Scanner.Extractor | `ND_SCANNER_EXTRACTOR` | Method of tag extraction Options: `ffmpeg`,`taglib` | `taglib` |



## Enable transcoding for a player
Trough the webui you can specify if one of your clients schould use transcoding:
Copy link

Choose a reason for hiding this comment

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

Typo: 'Trough' should be 'Through' and 'schould' should be 'should'.

Suggested change
Trough the webui you can specify if one of your clients schould use transcoding:
Through the webui you can specify if one of your clients should use transcoding:

## File Formats

In order to help you decide wether you need transcoding, here are two lists of file extensions.
Navidrome uses a patched version of [lijinke666/react-music-player](https://github.com/lijinke666/react-music-player) from where the firt list stems. [Fork](https://github.com/navidrome/react-music-player)
Copy link

Choose a reason for hiding this comment

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

Typo: 'firt list' should be 'first list'.

Suggested change
Navidrome uses a patched version of [lijinke666/react-music-player](https://github.com/lijinke666/react-music-player) from where the firt list stems. [Fork](https://github.com/navidrome/react-music-player)
Navidrome uses a patched version of [lijinke666/react-music-player](https://github.com/lijinke666/react-music-player) from where the first list stems. [Fork](https://github.com/navidrome/react-music-player)

Trough the webui you can specify if one of your clients schould use transcoding:
- Navigate to the player overview

{{< imgproc player_overview Fit "2000x2000" />
Copy link

Choose a reason for hiding this comment

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

The shortcode for the player overview image is missing closing braces. It likely should end with '>}}'.

Suggested change
{{< imgproc player_overview Fit "2000x2000" />
{{< imgproc player_overview Fit "2000x2000" />}}

Copy link
Member

Choose a reason for hiding this comment

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

@prk0ghy This is the error that is breaking the build.


## File Formats

In order to help you decide wether you need transcoding, here are two lists of file extensions.
Copy link

Choose a reason for hiding this comment

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

Typo: 'wether' should be 'whether' in the sentence 'In order to help you decide wether you need transcoding, here are two lists of file extensions.'

Suggested change
In order to help you decide wether you need transcoding, here are two lists of file extensions.
In order to help you decide whether you need transcoding, here are two lists of file extensions.

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