Compare commits
13
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
792e87298b | ||
|
|
3bf27e3fd5 | ||
|
|
48abecb830 | ||
|
|
e1c07438e9 | ||
|
|
6e563f3846 | ||
|
|
590a0d86dd | ||
|
|
36af7090d9 | ||
|
|
3e142f8c35 | ||
|
|
3d375adab1 | ||
|
|
e3d492e130 | ||
|
|
e6f30b6e43 | ||
|
|
351798fd66 | ||
|
|
40984f6086 |
@@ -3445,3 +3445,41 @@ their own output, and leave the previous snapshots in place.
|
|||||||
makes it worth having: a restored snapshot resolves to `refresh` and
|
makes it worth having: a restored snapshot resolves to `refresh` and
|
||||||
folds in the incremental listens since — minutes, against the 3–23 h a
|
folds in the incremental listens since — minutes, against the 3–23 h a
|
||||||
rebuild was estimating.
|
rebuild was estimating.
|
||||||
|
|
||||||
|
## A green release pipeline can ship an empty changelog (2026-08-18)
|
||||||
|
|
||||||
|
`conventional-changelog-conventionalcommits@10` is silently incompatible
|
||||||
|
with the writer `@semantic-release/release-notes-generator@14` depends on
|
||||||
|
(`conventional-changelog-writer@^8`). Every release note renders as a bare
|
||||||
|
`## 0.0.1 (date)` heading with **no sections and no commits under it**, no
|
||||||
|
step fails, and the release ships with an empty body.
|
||||||
|
|
||||||
|
It is pinned to `9` in `.gitea/workflows/release.yml` and in
|
||||||
|
`make release-dry`, which must stay identical. **Check the rendered notes,
|
||||||
|
never the exit code** — this is invisible to every tick in the pipeline.
|
||||||
|
|
||||||
|
## semantic-release needs push rights to the branch even when it never pushes to it (2026-08-18)
|
||||||
|
|
||||||
|
Core runs `git push --dry-run HEAD:<branch>` as a permission check, before
|
||||||
|
and independently of any plugin. With `@semantic-release/git` removed
|
||||||
|
nothing ever pushes to `main`, and the check still runs.
|
||||||
|
|
||||||
|
Two things this looked like and was not:
|
||||||
|
|
||||||
|
- **Not branch protection.** A `--dry-run` push does not reach the
|
||||||
|
pre-receive hook: pushing one to protected `main` with a write-scoped
|
||||||
|
token succeeds. So `main`'s `enable_push: false` is not what fails here.
|
||||||
|
- **A flat `403 Forbidden`, not Gitea's protection message.** That is the
|
||||||
|
tell. `PACKAGE_TOKEN` had package-write and repo-*read* — enough to
|
||||||
|
clone a private repo, so every other workflow was fine — and needed
|
||||||
|
`write:repository`.
|
||||||
|
|
||||||
|
## A tag-triggered workflow runs the workflow file at the *tagged* commit (2026-08-18)
|
||||||
|
|
||||||
|
Not the one on `main`. Moving `v0.0.0` onto a pre-merge commit ran that
|
||||||
|
commit's version of `homebrew-formula.yml`, which predated the `v0.0.0`
|
||||||
|
skip guard added in the same plan, and it pushed a `0.0.0` formula to the
|
||||||
|
public tap.
|
||||||
|
|
||||||
|
A guard added today does not protect a tag that points at yesterday. When
|
||||||
|
re-pointing a tag, check what the workflows looked like *there*.
|
||||||
|
|||||||
@@ -1,358 +0,0 @@
|
|||||||
# 017 — Releases that happen by themselves
|
|
||||||
|
|
||||||
> **Status: built, not yet run.** Phases 0–4 have landed on this branch;
|
|
||||||
> phase 5 is the merge itself and cannot be done until then. The old
|
|
||||||
> `v1.x` tags are already deleted from `origin`. Verified locally against
|
|
||||||
> a scratch remote: semantic-release computes **0.0.1** from these
|
|
||||||
> commits and renders correct sectioned notes.
|
|
||||||
>
|
|
||||||
> **One thing found by testing that no amount of reading would have
|
|
||||||
> caught.** `conventional-changelog-conventionalcommits@10` — the current
|
|
||||||
> release, and my first pin — is silently incompatible with the writer
|
|
||||||
> `release-notes-generator@14` depends on: the version is right, the tag
|
|
||||||
> is right, every step reports success, and the release body is a bare
|
|
||||||
> `## 0.0.1 (date)` heading with **nothing under it**. It is pinned to 9
|
|
||||||
> in both `release.yml` and `make release-dry`, with the reason written
|
|
||||||
> beside it. Four of my seven original pins were wrong majors besides;
|
|
||||||
> they were guesses, and `npm view` was the fix.
|
|
||||||
|
|
||||||
The goal in one sentence: **a merge to `main` computes the next version
|
|
||||||
from the commits it contains, cuts a tag and a Gitea release whose body
|
|
||||||
is the changelog, and every publishing channel builds that tag.** The
|
|
||||||
first release under this scheme is `v0.0.1`, and the five existing `v1.x`
|
|
||||||
tags go.
|
|
||||||
|
|
||||||
## What is there now
|
|
||||||
|
|
||||||
Measured, not remembered:
|
|
||||||
|
|
||||||
- **Five tags and zero releases.** `v1.3.0`, `v1.4.0`, `v1.4.1`,
|
|
||||||
`v1.5.0`, `v1.6.0` exist on `origin`;
|
|
||||||
`GET /api/v1/repos/yonlu/yellowjacket/releases` returns `[]`. So there
|
|
||||||
is no release page to preserve and nothing but the tags to remove.
|
|
||||||
- **`CHANGELOG.md` is stale and belongs to another repo.** Its newest
|
|
||||||
entry is `1.3.0` and every link in it points at
|
|
||||||
`github.com/onion-4-dinner/yellowjacket` — it was written by a
|
|
||||||
semantic-release run against a GitHub remote this project no longer
|
|
||||||
has.
|
|
||||||
- **`.releaserc.yml` is a complete semantic-release config that nothing
|
|
||||||
invokes**, which CLAUDE.md already says in as many words.
|
|
||||||
- **Root `package.json` is literally `{}`** — the stub left behind by
|
|
||||||
whatever was going to run it.
|
|
||||||
- The triggers today are: `arch-package` on **push to `main`**,
|
|
||||||
`homebrew-formula` on **`v*`**, `android-apk` on **`v*`**, `ci` on
|
|
||||||
every branch, `index-artifact` on cron/dispatch. So Arch publishes a
|
|
||||||
`git describe` version on every merge and the other two publish only
|
|
||||||
when a human remembers to push a tag.
|
|
||||||
|
|
||||||
## Decision 1 — semantic-release, with `exec` in place of the `github` plugin
|
|
||||||
|
|
||||||
**Revised: the first draft of this plan proposed a shell script and the
|
|
||||||
argument for it does not hold.** Recorded here rather than deleted,
|
|
||||||
because the reasoning is what the decision rests on.
|
|
||||||
|
|
||||||
What I said, and what checking it showed:
|
|
||||||
|
|
||||||
- *"The two plugins that would carry the work do not fit."* Half true.
|
|
||||||
`@semantic-release/github` genuinely does not speak Gitea's `/api/v1`
|
|
||||||
— but the replacement is **`@semantic-release/exec`**, which is
|
|
||||||
first-party, published 2026-06, and peer-deps `semantic-release >=24.1`.
|
|
||||||
Its `publishCmd` is one `curl` at the Gitea release endpoint with
|
|
||||||
`${nextRelease.notes}` as the body. The Gitea-shaped part of this is
|
|
||||||
five lines, and the part I proposed to hand-roll — parsing conventional
|
|
||||||
commits, ordering semver, rendering grouped notes — is the part with
|
|
||||||
the edge cases and none of it is Gitea-shaped at all.
|
|
||||||
- *"`@semantic-release/git` commits the changelog back to `main`, which
|
|
||||||
re-triggers everything."* True, and it is the one real risk — but it
|
|
||||||
is a two-line guard (skip the job when `HEAD`'s subject is
|
|
||||||
`chore(release):`), not a reason to write a version calculator. That
|
|
||||||
guard is needed under **either** design, since either one writes a
|
|
||||||
changelog commit.
|
|
||||||
- *"A Node dependency tree at the root of a Go repo."* The commitlint
|
|
||||||
precedent does not transfer. commitlint was a dependency to regex one
|
|
||||||
line; this is a dependency to do something with real complexity, it is
|
|
||||||
`npx`-only so nothing lands in the repo, and Node is already installed
|
|
||||||
in CI for the frontend.
|
|
||||||
- *"It cannot be told to produce `0.0.1`."* Wrong — that is a property
|
|
||||||
of which commits are in the range, not of the tool. Identical under
|
|
||||||
both designs. See below.
|
|
||||||
|
|
||||||
Note also that **`@saithodev/semantic-release-gitea` is a dead end** and
|
|
||||||
should not be reached for: last published 2022, depends on `got@10` and
|
|
||||||
`fs-extra@8`, and declares no peer dependency on semantic-release at all
|
|
||||||
— i.e. it is untested against anything since v19, against a core now at
|
|
||||||
v25. `exec` + `curl` is both simpler and maintained.
|
|
||||||
|
|
||||||
So `.releaserc.yml` stays, and its plugin list becomes five **first-party**
|
|
||||||
plugins, all published within the last six months:
|
|
||||||
|
|
||||||
| plugin | job |
|
|
||||||
| --- | --- |
|
|
||||||
| `commit-analyzer` | the version |
|
|
||||||
| `release-notes-generator` | the notes |
|
|
||||||
| `changelog` | writes `CHANGELOG.md` |
|
|
||||||
| `git` | commits it back |
|
|
||||||
| `exec` | `curl`s the Gitea release |
|
|
||||||
|
|
||||||
The `releaseRules` and `presetConfig` blocks already in the file are
|
|
||||||
kept verbatim — they are the same bump table `commit-check.sh` already
|
|
||||||
enforces the grammar for, and nothing about the project's commit
|
|
||||||
convention changes.
|
|
||||||
|
|
||||||
Two mechanical details that decide whether this works at all:
|
|
||||||
|
|
||||||
- **semantic-release pushes the tag itself**, as core behaviour, using
|
|
||||||
`repositoryUrl`. The remote here is `ssh://git@git.ljones.me:2222/…`,
|
|
||||||
which would need an SSH key in CI — so the run passes
|
|
||||||
`--repository-url "https://x-access-token:$PACKAGE_TOKEN@git.ljones.me/yonlu/yellowjacket.git"`
|
|
||||||
on the command line rather than committing a token to the config.
|
|
||||||
**That is also what satisfies Decision 2**: the tag push is attributed
|
|
||||||
to a real user, not to the Actions token.
|
|
||||||
- **The empty root `package.json` (`{}`) goes.** semantic-release does
|
|
||||||
not need one when `--repository-url` is explicit, and leaving a
|
|
||||||
package manifest at the root of a Go repo invites the npm plugin and
|
|
||||||
every tool that looks for one.
|
|
||||||
|
|
||||||
Invocation is pinned in the workflow, not installed into the repo:
|
|
||||||
|
|
||||||
```
|
|
||||||
npx --yes \
|
|
||||||
-p semantic-release@25 \
|
|
||||||
-p @semantic-release/commit-analyzer@14 \
|
|
||||||
-p @semantic-release/release-notes-generator@15 \
|
|
||||||
-p @semantic-release/changelog@6 \
|
|
||||||
-p @semantic-release/git@10 \
|
|
||||||
-p @semantic-release/exec@7 \
|
|
||||||
-p conventional-changelog-conventionalcommits@9 \
|
|
||||||
semantic-release --repository-url "…"
|
|
||||||
```
|
|
||||||
|
|
||||||
(Exact majors get pinned from `npm view` at implementation time;
|
|
||||||
`conventional-changelog-conventionalcommits` is in the list because both
|
|
||||||
the analyzer and the notes generator name that preset and neither
|
|
||||||
depends on it.)
|
|
||||||
|
|
||||||
## Decision 2 — how the publish workflows learn about the tag
|
|
||||||
|
|
||||||
**Gitea, like GitHub, does not start a workflow from a tag pushed by a
|
|
||||||
workflow's own token** (go-gitea#33123, and the forum thread it points
|
|
||||||
at). This is the one load-bearing unknown in the plan.
|
|
||||||
|
|
||||||
The remedy is to push the tag with a *user* PAT — `secrets.PACKAGE_TOKEN`
|
|
||||||
is already in this repo and already used by `arch-package` and
|
|
||||||
`android-apk` to clone and to publish — so the push is attributed to a
|
|
||||||
person and the `v*` triggers fire normally. That keeps the three publish
|
|
||||||
workflows completely unchanged in shape.
|
|
||||||
|
|
||||||
**It is verified in phase 5, not assumed.** The fallback, if it does not
|
|
||||||
fire, is an explicit `POST
|
|
||||||
/api/v1/repos/{owner}/{repo}/actions/workflows/{file}/dispatches` per
|
|
||||||
channel from the release job. That needs `workflow_dispatch` (with a
|
|
||||||
`version` input) added to `homebrew-formula.yml` and `arch-package.yml`;
|
|
||||||
`android-apk.yml` already has both. **Add those inputs in phase 3
|
|
||||||
regardless** — a hand-triggered rebuild of one channel is worth having
|
|
||||||
whether or not the fallback is needed.
|
|
||||||
|
|
||||||
The alternative — one `release.yml` with the three publishes as
|
|
||||||
`needs:` jobs — is rejected: it means either copying ~400 lines of
|
|
||||||
Android and Arch setup into it or relying on `workflow_call`, and it
|
|
||||||
puts every merge to `main` behind an up-to-60-minute Android build on a
|
|
||||||
runner with capacity 1.
|
|
||||||
|
|
||||||
## Decision 3 — 1.6.0 → 0.0.1 is a downgrade, and the answer is reinstall
|
|
||||||
|
|
||||||
**Decided: no version-code offset, no epoch. The version number stays
|
|
||||||
honest and existing installs are replaced by hand.** Every channel is a
|
|
||||||
downgrade and each declines differently, so what to expect:
|
|
||||||
|
|
||||||
- **Arch: no upgrade is offered, silently.** `pkgver()` derives from
|
|
||||||
`git describe`, so after the wipe it reads `0.0.1.rN.gHASH`, which
|
|
||||||
pacman orders *below* the `1.3.0.rN.*` in the registry. `pacman -R
|
|
||||||
yellowjacket && pacman -S yellowjacket` is the remedy. (`epoch=1` in
|
|
||||||
the PKGBUILD would have avoided it for one line — but an epoch can
|
|
||||||
never be removed, and it puts a permanent `1:` in front of every
|
|
||||||
version string this project will ever have.)
|
|
||||||
- **Homebrew: no upgrade is offered, silently.** Brew has no epoch at
|
|
||||||
all. `brew uninstall yellowjacket && brew install …`.
|
|
||||||
- **Android: a hard refusal.** `versionCode` is
|
|
||||||
`maj*10000 + min*100 + pat`, so `0.0.1` is **1** against the **10300**
|
|
||||||
an installed 1.3.0 carries, and the install fails with
|
|
||||||
`INSTALL_FAILED_VERSION_DOWNGRADE`. Uninstall first — **and that takes
|
|
||||||
the app's library and config with it**, which is the same data loss
|
|
||||||
`android-apk.yml`'s keystore guard exists to prevent, arrived at from
|
|
||||||
the other direction. The workflow's own `code -le 0` guard still passes
|
|
||||||
at 1, so nothing in CI stops or warns about this.
|
|
||||||
|
|
||||||
All three go in the release notes for `v0.0.1` and in
|
|
||||||
`packaging/homebrew/README.md` / `docs/android-release.md`, because a
|
|
||||||
channel that silently offers no upgrade is indistinguishable from a
|
|
||||||
broken pipeline six months from now.
|
|
||||||
|
|
||||||
## Landing exactly `v0.0.1`
|
|
||||||
|
|
||||||
Determinism comes from two things:
|
|
||||||
|
|
||||||
1. **Seed `v0.0.0` on `6fb7b5e`** (current `origin/main`) after wiping
|
|
||||||
the old tags. That is the floor, and the analyser's range starts
|
|
||||||
there.
|
|
||||||
2. **This branch carries no `feat:` commit.** Everything in it is
|
|
||||||
`ci:`/`docs:`/`chore:`/`build:`, plus at least one `fix:` — which is
|
|
||||||
honest, since wiring up release machinery that was configured and
|
|
||||||
never run *is* a fix. One patch-level commit in `v0.0.0..HEAD`
|
|
||||||
computes `0.0.1` and nothing else can.
|
|
||||||
|
|
||||||
This is a property of the commit range, not of the tool — it would have
|
|
||||||
been the same constraint under the shell script.
|
|
||||||
|
|
||||||
This is a real constraint on the branch, not an accounting trick: a
|
|
||||||
single `feat:` commit here makes the first release `v0.1.0`.
|
|
||||||
|
|
||||||
`v0.0.0` itself gets no release object — it is a floor, not a shipment.
|
|
||||||
|
|
||||||
## Decision 4 — what the release page carries
|
|
||||||
|
|
||||||
Four artifacts, and the fourth is the interesting one. Measured on this
|
|
||||||
machine rather than assumed:
|
|
||||||
|
|
||||||
| asset | built by | state |
|
|
||||||
| --- | --- | --- |
|
|
||||||
| `yellowjacket-<v>-android-arm64.apk` | `android-apk.yml` | already built, verified, signed |
|
|
||||||
| `yellowjacket-<v>-linux-amd64.tar.gz` | new job | binary + `.desktop` + icon |
|
|
||||||
| `yellowjacket-<v>-x86_64.pkg.tar.zst` | `arch-package.yml` | already built; free to attach |
|
|
||||||
| `yellowjacket-<v>-windows-amd64.zip` | new job | **compiles; has never been run** |
|
|
||||||
|
|
||||||
**macOS cannot be one of them.** `GOOS=darwin CGO_ENABLED=0` fails at
|
|
||||||
`wails/v3/pkg/mac: build constraints exclude all Go files` — the darwin
|
|
||||||
backend is Objective-C behind cgo, so a `.app` needs a macOS host and
|
|
||||||
the runner is a Linux container. That is precisely why the Homebrew
|
|
||||||
channel builds from source on the user's own Mac, and it stays the
|
|
||||||
answer for macOS.
|
|
||||||
|
|
||||||
**Windows is newly possible and should be labelled honestly.**
|
|
||||||
`GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go build -tags production`
|
|
||||||
succeeds in 2.5 s and produces a 40 MB `.exe` — nothing in the audio,
|
|
||||||
database or webview path needs cgo on Windows (oto uses WinMM through
|
|
||||||
`x/sys`, sqlite is modernc's pure-Go driver, WebView2 is COM syscalls,
|
|
||||||
and MPRIS is `linux && !android`-tagged). But **compiling is not
|
|
||||||
running**: no Windows build of this app has ever been started, no CI tier
|
|
||||||
can exercise one, and `backend/system`'s `%LOCALAPPDATA%` path has never
|
|
||||||
resolved on a real machine. It ships marked as untested in the release
|
|
||||||
notes, or it does not ship — an unlabelled Windows download is a promise
|
|
||||||
nothing here can keep.
|
|
||||||
|
|
||||||
### The race the ordering creates
|
|
||||||
|
|
||||||
semantic-release runs **prepare** (changelog commit, tag push) before
|
|
||||||
**publish** (the `exec` curl that creates the release object). The tag
|
|
||||||
push is what starts the publishing workflows — so a fast one can reach
|
|
||||||
its upload step *before the release exists*, and
|
|
||||||
`POST /releases/{id}/assets` needs an id.
|
|
||||||
|
|
||||||
The capacity-1 runner serialises things enough that this would usually
|
|
||||||
work, which is the worst kind of bug. So each upload step **polls
|
|
||||||
`GET /api/v1/repos/…/releases/tags/{tag}` with a bounded retry** before
|
|
||||||
uploading, and fails loudly on timeout rather than skipping the asset.
|
|
||||||
That is ~8 lines of shell, shared by all three publishers.
|
|
||||||
|
|
||||||
## Phases
|
|
||||||
|
|
||||||
**Phase 0 — clear the ground.**
|
|
||||||
Delete `v1.3.0`–`v1.6.0` locally and on `origin`; push `v0.0.0` at
|
|
||||||
`6fb7b5e` — this is the floor semantic-release reads, and without it the
|
|
||||||
first release is `1.0.0` by its own rule. Delete the empty root
|
|
||||||
`package.json`. Truncate `CHANGELOG.md` to a header plus a line saying
|
|
||||||
history before `0.0.1` is in `git log` — the existing content is another
|
|
||||||
repo's links and cannot be repaired, only replaced, and the `changelog`
|
|
||||||
plugin prepends to whatever it finds.
|
|
||||||
|
|
||||||
**Phase 1 — `.releaserc.yml`.**
|
|
||||||
Swap `@semantic-release/github` for `@semantic-release/exec`, whose
|
|
||||||
`publishCmd` POSTs to
|
|
||||||
`/api/v1/repos/yonlu/yellowjacket/releases` with `tag_name`, `name` and
|
|
||||||
`body` taken from `${nextRelease.*}`. Keep `commit-analyzer`,
|
|
||||||
`release-notes-generator`, `changelog` and `git` exactly as written; fix
|
|
||||||
the `git` plugin's commit message so it passes `commit-check`
|
|
||||||
(`chore(release): ${nextRelease.version}` — the existing one already
|
|
||||||
does, but the trailing `${nextRelease.notes}` in the body is worth
|
|
||||||
keeping deliberate rather than incidental). `make release-dry` wraps
|
|
||||||
`semantic-release --dry-run` so the next version is answerable without
|
|
||||||
pushing anything.
|
|
||||||
|
|
||||||
`scripts/commit-check.sh`'s header already points at `.releaserc.yml`
|
|
||||||
for the type list and stays correct — that coupling survives this plan
|
|
||||||
rather than being broken by it.
|
|
||||||
|
|
||||||
**Phase 2 — `.gitea/workflows/release.yml`.**
|
|
||||||
On `push: branches: [main]`. Node 22, the pinned `npx` line from
|
|
||||||
Decision 1, `--repository-url` carrying `PACKAGE_TOKEN`. Concurrency
|
|
||||||
group `release-main` with `cancel-in-progress: false` — cutting a tag is
|
|
||||||
not a thing to cancel halfway.
|
|
||||||
|
|
||||||
The one guard that matters: **the job exits early when `HEAD`'s subject
|
|
||||||
starts `chore(release):`**, so the changelog commit the `git` plugin
|
|
||||||
pushes cannot re-enter this workflow. That is checked in shell rather
|
|
||||||
than left to `[skip ci]`, whose handling in Gitea is one more thing that
|
|
||||||
would have to be verified.
|
|
||||||
|
|
||||||
**Phase 3 — rewire the publish workflows.**
|
|
||||||
`arch-package.yml` moves from `push: branches: [main]` to
|
|
||||||
`push: tags: ['v*']` plus `workflow_dispatch`, so a merge no longer
|
|
||||||
publishes an untagged package. `homebrew-formula.yml` gains
|
|
||||||
`workflow_dispatch` with a `version` input and takes its version from
|
|
||||||
the input when there is no tag. `android-apk.yml` needs neither.
|
|
||||||
|
|
||||||
**Phase 3b — the assets.**
|
|
||||||
`scripts/release-asset.sh` is the shared uploader: wait for the release
|
|
||||||
by tag, then `POST /releases/{id}/assets?name=…`. `android-apk.yml` and
|
|
||||||
`arch-package.yml` each call it with the artifact they already built.
|
|
||||||
A new `desktop-assets` job — `push: tags: ['v*']`, in the same
|
|
||||||
`ubuntu:24.04` container `ci.yml` uses — builds the Linux binary via
|
|
||||||
`make build-prod` and the Windows one via the `CGO_ENABLED=0`
|
|
||||||
cross-compile, and uploads both. It is a separate job from the Arch one
|
|
||||||
because that runs in an `archlinux` container as an unprivileged
|
|
||||||
`makepkg` user, and grafting two unrelated builds onto it would make one
|
|
||||||
failure look like the other.
|
|
||||||
|
|
||||||
**Phase 4 — say that the upgrade is a reinstall, and that Windows is untried.**
|
|
||||||
No code change: a note in `packaging/homebrew/README.md`, one in
|
|
||||||
`docs/android-release.md`, the three-channel downgrade warning written
|
|
||||||
into the `v0.0.1` release notes, and a standing line in the notes
|
|
||||||
template marking the Windows asset unverified until someone runs it.
|
|
||||||
|
|
||||||
**Phase 5 — cut it and watch.** *(the only phase left)*
|
|
||||||
Merge, then verify with `gitea_ci` that (a) `release.yml` ran, seeded
|
|
||||||
`v0.0.0` and produced `v0.0.1`, (b) the release exists **with a non-empty
|
|
||||||
body** — check the body, not the exit code — and (c) **all four publish
|
|
||||||
workflows started from the tag**. If (c) is empty, that is Decision 2's
|
|
||||||
fallback and the `workflow_dispatch` inputs added in phase 3 are already
|
|
||||||
there to drive it.
|
|
||||||
|
|
||||||
The expected sequence on the merge is: `release.yml` seeds `v0.0.0`
|
|
||||||
(triggering nothing), releases `0.0.1`, and pushes both the changelog
|
|
||||||
commit and the tag — at which point `release.yml` fires a second time on
|
|
||||||
the changelog commit and exits at the `chore(release):` guard, while the
|
|
||||||
four `v*` workflows start. On a capacity-1 runner they will queue behind
|
|
||||||
each other, Android last and longest.
|
|
||||||
|
|
||||||
**Phase 6 — the documentation that will otherwise be wrong.**
|
|
||||||
CLAUDE.md's *Commits* section currently explains `.releaserc.yml` and
|
|
||||||
says nothing runs it; the CI section says there are five workflows and
|
|
||||||
that only `ci.yml` gates. Both change. `docs/android-release.md`
|
|
||||||
describes tags as hand-pushed. `make skill-check` fails on a `.pi/`
|
|
||||||
reference to a make target that does not exist, so `make release-dry`
|
|
||||||
gets documented or nothing does.
|
|
||||||
|
|
||||||
## Open questions for you
|
|
||||||
|
|
||||||
1. **Ship the Windows `.exe` or not?** It builds, and it has never run.
|
|
||||||
Marked-as-untested is the assumption; say if you would rather hold it
|
|
||||||
back until someone boots it.
|
|
||||||
|
|
||||||
Resolved: semantic-release stays, with `exec` in place of the `github`
|
|
||||||
plugin (Decision 1). Reinstalls are accepted, so no epoch and no
|
|
||||||
versionCode offset (Decision 3). The release carries the APK, a Linux
|
|
||||||
tarball, the Arch package and — pending (1) — a Windows zip; macOS is
|
|
||||||
not buildable here and stays a Homebrew-from-source channel (Decision 4).
|
|
||||||
`v0.0.0` has to be a real tag under this design — semantic-release reads
|
|
||||||
git tags for its floor and has no "treat absence as 0.0.0" knob that
|
|
||||||
also stops it calling the first release `1.0.0`.
|
|
||||||
@@ -0,0 +1,87 @@
|
|||||||
|
# 017 — Releases that happen by themselves
|
||||||
|
|
||||||
|
**Shipped as `v0.0.1`.** A merge to `main` now reads the Conventional
|
||||||
|
Commits since the last tag, cuts the tag and the Gitea release whose body
|
||||||
|
is the generated changelog, and the four publishing workflows build that
|
||||||
|
tag and attach their artifacts. Nothing is released by hand.
|
||||||
|
|
||||||
|
## What it looks like now
|
||||||
|
|
||||||
|
`release.yml` on push to `main` → semantic-release → tag → four `v*`
|
||||||
|
workflows in parallel (serialised in practice by the capacity-1 runner):
|
||||||
|
|
||||||
|
| workflow | publishes | attaches |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| `arch-package` | pacman registry | `…-x86_64.pkg.tar.zst` |
|
||||||
|
| `android-apk` | generic registry (Obtainium) | `…-android-arm64.apk` |
|
||||||
|
| `desktop-assets` | — | `…-linux-amd64.tar.gz` |
|
||||||
|
| `homebrew-formula` | the public tap | — (builds from source) |
|
||||||
|
|
||||||
|
Verified on the real thing: all five green, three assets on the release,
|
||||||
|
the tap at `0.0.1`, and the Obtainium `latest` URL serving 200.
|
||||||
|
|
||||||
|
## The five decisions, and what they cost
|
||||||
|
|
||||||
|
1. **semantic-release, not a shell script.** The first draft of this plan
|
||||||
|
proposed hand-rolling it and the argument did not survive checking:
|
||||||
|
`@semantic-release/exec` is first-party and current, and the
|
||||||
|
Gitea-shaped part is one `curl`. What I would have hand-rolled —
|
||||||
|
commit parsing, semver ordering, note rendering — is the part with the
|
||||||
|
edge cases and none of it is Gitea-shaped.
|
||||||
|
2. **`@saithodev/semantic-release-gitea` is a dead end** and was offered
|
||||||
|
before it was checked: last published 2022, `got@10`, and no peer
|
||||||
|
dependency on semantic-release at all.
|
||||||
|
3. **No `@semantic-release/git`.** `main` is protected, so a changelog
|
||||||
|
commit-back is rejected by the pre-receive hook — and would be
|
||||||
|
rejected *after* the tag was pushed, leaving a tagged release the run
|
||||||
|
reports as failed. The release page is the changelog;
|
||||||
|
`.release-notes.md` is a gitignored carrier and `CHANGELOG.md` is a
|
||||||
|
signpost.
|
||||||
|
4. **Versions restart at `0.0.1`**, a downgrade on every channel. No
|
||||||
|
`epoch`, no `versionCode` offset: both are permanent, a reinstall is
|
||||||
|
once. Documented in `packaging/homebrew/README.md` and
|
||||||
|
`docs/android-release.md`.
|
||||||
|
5. **No macOS and no Windows.** `GOOS=darwin CGO_ENABLED=0` fails at
|
||||||
|
`wails/v3/pkg/mac` and there is no macOS runner, so Homebrew-from-source
|
||||||
|
stays that channel. Windows cross-compiles in ~2.5 s and is withheld
|
||||||
|
because no build of it has ever been *run*.
|
||||||
|
|
||||||
|
## Four things that only showed up by running it
|
||||||
|
|
||||||
|
- **`conventional-changelog-conventionalcommits@10` renders empty
|
||||||
|
notes.** Silently: right version, right tag, every step green, and a
|
||||||
|
release body that is a bare `## 0.0.1 (date)` heading with nothing
|
||||||
|
beneath it. Held at `9`, in `release.yml` and `make release-dry`, with
|
||||||
|
the reason beside both. **Check the rendered notes, never the exit
|
||||||
|
code.**
|
||||||
|
- **semantic-release core dry-run-pushes to the release branch** as a
|
||||||
|
permission check, independently of any plugin. `PACKAGE_TOKEN` had
|
||||||
|
package-write and repo-*read* — enough to clone, not enough for this —
|
||||||
|
and it failed with a flat `403 Forbidden` that reads exactly like
|
||||||
|
branch protection. It is not: a `--dry-run` push never reaches the
|
||||||
|
pre-receive hook, which a one-line experiment settled. The token needed
|
||||||
|
`write:repository`.
|
||||||
|
- **The floor tag must go on `HEAD^`, not `HEAD`.** Seeded on the merge
|
||||||
|
commit itself it leaves nothing between the floor and HEAD, and
|
||||||
|
semantic-release correctly reports there is nothing to release. The
|
||||||
|
first run did exactly that and cut nothing.
|
||||||
|
- **A tag-triggered workflow runs from the tagged commit's tree.**
|
||||||
|
Moving `v0.0.0` back to `6fb7b5e` ran the *pre-merge* homebrew
|
||||||
|
workflow, which predates the `v0.0.0` skip guard, and pushed a `0.0.0`
|
||||||
|
formula to the public tap. Self-corrected at `0.0.1`. The corollary is
|
||||||
|
general: a guard added today does not protect a tag pointing at
|
||||||
|
yesterday.
|
||||||
|
|
||||||
|
## Two mechanisms confirmed, having been assumptions
|
||||||
|
|
||||||
|
- **A tag pushed with a user PAT does start the `v*` workflows**; one
|
||||||
|
pushed with the Actions token does not (go-gitea#33123). Both halves
|
||||||
|
are load-bearing and both were observed: the floor seed triggered
|
||||||
|
nothing, and the release tag triggered all four.
|
||||||
|
- **Tags are not protected** on this repo, only `main` — which is what
|
||||||
|
lets semantic-release tag at all.
|
||||||
|
|
||||||
|
## Left behind deliberately
|
||||||
|
|
||||||
|
`v0.0.0` stays on `origin` as the floor. It carries no release, and all
|
||||||
|
four publishers skip it by name.
|
||||||
@@ -411,9 +411,10 @@ func (c *Config) SetDownloadPreferences(prefs download.AutoDownloadPrefs) error
|
|||||||
formats = append(formats, string(f))
|
formats = append(formats, string(f))
|
||||||
}
|
}
|
||||||
|
|
||||||
c.Downloads.MinFileSizeMB = prefs.MinSizeMB
|
c.Downloads.MinKbps = prefs.MinKbps
|
||||||
|
c.Downloads.MaxKbps = prefs.MaxKbps
|
||||||
|
c.Downloads.PreferredKbps = prefs.PreferredKbps
|
||||||
c.Downloads.MaxFileSizeMB = prefs.MaxSizeMB
|
c.Downloads.MaxFileSizeMB = prefs.MaxSizeMB
|
||||||
c.Downloads.PreferredFileSizeMB = prefs.PreferredSizeMB
|
|
||||||
c.Downloads.AllowedFormats = formats
|
c.Downloads.AllowedFormats = formats
|
||||||
|
|
||||||
if err := c.Save(); err != nil {
|
if err := c.Save(); err != nil {
|
||||||
|
|||||||
+28
-11
@@ -34,13 +34,29 @@ type UserConfig struct {
|
|||||||
// in one burst that every provider sees as a flood.
|
// in one burst that every provider sees as a flood.
|
||||||
WantedBatch int `toml:"WantedBatch"`
|
WantedBatch int `toml:"WantedBatch"`
|
||||||
|
|
||||||
// MinFileSizeMB, MaxFileSizeMB and PreferredFileSizeMB bound and
|
// MinKbps, MaxKbps and PreferredKbps bound and nudge what auto-pick
|
||||||
// nudge what auto-pick (interactive or via the request list) may
|
// (interactive or via the request list) may grab without asking.
|
||||||
// grab without asking. Zero on any of them is permissive: see
|
// Zero on any of them is permissive: see AutoDownloadPrefs.
|
||||||
// AutoDownloadPrefs.
|
//
|
||||||
MinFileSizeMB int `toml:"MinFileSizeMB"`
|
// They replaced MinFileSizeMB / MaxFileSizeMB /
|
||||||
MaxFileSizeMB int `toml:"MaxFileSizeMB"`
|
// PreferredFileSizeMB, which were megabytes and so said nothing
|
||||||
PreferredFileSizeMB int `toml:"PreferredFileSizeMB"`
|
// without knowing how long the release was. The old keys are
|
||||||
|
// deliberately *not* read back: a number that meant "300 MB" cannot
|
||||||
|
// be reinterpreted as a bitrate without knowing the album it was
|
||||||
|
// aimed at, so migrating it would be inventing an intent the user
|
||||||
|
// never expressed. An existing config falls back to no window,
|
||||||
|
// which is the permissive default and matches a fresh install —
|
||||||
|
// and MaxFileSizeMB is the one that does carry over, because a
|
||||||
|
// ceiling on total bytes still means exactly what it did.
|
||||||
|
MinKbps int `toml:"MinKbps"`
|
||||||
|
MaxKbps int `toml:"MaxKbps"`
|
||||||
|
PreferredKbps int `toml:"PreferredKbps"`
|
||||||
|
|
||||||
|
// MaxFileSizeMB is a hard ceiling on a candidate's total size, kept
|
||||||
|
// in megabytes on purpose — it is a question about disk space, not
|
||||||
|
// about quality, and it has to apply to a candidate whose bitrate
|
||||||
|
// cannot be worked out at all.
|
||||||
|
MaxFileSizeMB int `toml:"MaxFileSizeMB"`
|
||||||
|
|
||||||
// AllowedFormats restricts auto-pick to these formats. Empty means
|
// AllowedFormats restricts auto-pick to these formats. Empty means
|
||||||
// no restriction. Values are Format strings ("flac", "mp3", ...).
|
// no restriction. Values are Format strings ("flac", "mp3", ...).
|
||||||
@@ -56,10 +72,11 @@ func (c *UserConfig) AutoDownloadPrefs() AutoDownloadPrefs {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return AutoDownloadPrefs{
|
return AutoDownloadPrefs{
|
||||||
MinSizeMB: c.MinFileSizeMB,
|
MinKbps: c.MinKbps,
|
||||||
MaxSizeMB: c.MaxFileSizeMB,
|
MaxKbps: c.MaxKbps,
|
||||||
PreferredSizeMB: c.PreferredFileSizeMB,
|
PreferredKbps: c.PreferredKbps,
|
||||||
AllowedFormats: formats,
|
MaxSizeMB: c.MaxFileSizeMB,
|
||||||
|
AllowedFormats: formats,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -236,6 +236,12 @@ func (m *Manager) AutoPickable(dl Download, ranked []Candidate) bool {
|
|||||||
return AutoPickable(dl, ranked, m.preferences())
|
return AutoPickable(dl, ranked, m.preferences())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// AutoPickVeto wraps the package function the same way, and is what the
|
||||||
|
// request list quotes back to the user.
|
||||||
|
func (m *Manager) AutoPickVeto(dl Download, ranked []Candidate) string {
|
||||||
|
return AutoPickVeto(dl, ranked, m.preferences())
|
||||||
|
}
|
||||||
|
|
||||||
// Reload rebuilds every provider from stored config. Called at startup
|
// Reload rebuilds every provider from stored config. Called at startup
|
||||||
// and after any provider settings change.
|
// and after any provider settings change.
|
||||||
//
|
//
|
||||||
@@ -612,16 +618,8 @@ func (m *Manager) Attempt(
|
|||||||
return false, "", err
|
return false, "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
if !m.AutoPickable(dl, ranked) {
|
if veto := m.AutoPickVeto(dl, ranked); veto != "" {
|
||||||
best := ranked[0]
|
return false, veto, nil
|
||||||
|
|
||||||
return false, fmt.Sprintf(
|
|
||||||
"best of %d found is not a confident enough match "+
|
|
||||||
"(match %.0f%%, quality %.0f%%)",
|
|
||||||
len(ranked),
|
|
||||||
best.Match.Overall*100, //nolint:mnd // percent
|
|
||||||
best.Quality.Overall*100,
|
|
||||||
), nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := m.store.CreateDownload(ctx, dl); err != nil {
|
if err := m.store.CreateDownload(ctx, dl); err != nil {
|
||||||
|
|||||||
@@ -218,8 +218,17 @@ func TestManagerEndToEndAutoPick(t *testing.T) {
|
|||||||
}, "staging was never released, or the library was never rescanned")
|
}, "staging was never released, or the library was never rescanned")
|
||||||
}
|
}
|
||||||
|
|
||||||
// An ambiguous result set must park for the user rather than guess.
|
// Two equally good copies are not an ambiguity — they are a spare.
|
||||||
func TestManagerWaitsWhenAmbiguous(t *testing.T) {
|
//
|
||||||
|
// This asserted the opposite for as long as auto-pick required 0.08 of
|
||||||
|
// daylight over the runner-up, and that rule was wrong in exactly the
|
||||||
|
// case it fired hardest: a popular album turns up several *correct*
|
||||||
|
// copies, all matching the tracklist, differing only in format and
|
||||||
|
// seeders. There is no question there about what to fetch, only about
|
||||||
|
// which copy, and the ranking already answers that — closest to the
|
||||||
|
// preferred bitrate first. A candidate does not have to be better than
|
||||||
|
// the field, only good enough on its own terms.
|
||||||
|
func TestManagerAutoPicksAmongEquallyGoodCopies(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
f := newManagerFixture(t)
|
f := newManagerFixture(t)
|
||||||
@@ -237,11 +246,41 @@ func TestManagerWaitsWhenAmbiguous(t *testing.T) {
|
|||||||
t.Fatalf("Start: %v", err)
|
t.Fatalf("Start: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if f.manager.AutoPickable(dl, ranked) {
|
if veto := f.manager.AutoPickVeto(dl, ranked); veto != "" {
|
||||||
t.Fatal("two equivalent candidates must not auto-pick")
|
t.Fatalf("two equally good copies must auto-pick, got veto: %s", veto)
|
||||||
|
}
|
||||||
|
|
||||||
|
waitForDownloadState(t, f.store, dl.ID, StateComplete)
|
||||||
|
|
||||||
|
// Exactly one of them was fetched, not both.
|
||||||
|
if grabs := a.GrabCalls + b.GrabCalls; grabs != 1 {
|
||||||
|
t.Errorf("grabs = %d, want exactly 1", grabs)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The user can still pick explicitly when auto-pick is not what
|
||||||
|
// happened — a candidate the ranking did not choose is still grabbable.
|
||||||
|
func TestManagerPickIsExplicit(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
f := newManagerFixture(t)
|
||||||
|
|
||||||
|
a := fakeWithAlbum(1, "source-a", ".flac")
|
||||||
|
b := fakeWithAlbum(2, "source-b", ".flac")
|
||||||
|
|
||||||
|
f.manager.installProvider(Config{ID: 1, Priority: 50}, a)
|
||||||
|
f.manager.installProvider(Config{ID: 2, Priority: 50}, b)
|
||||||
|
|
||||||
|
// No tracklist: never auto-picks, so the result set parks for the
|
||||||
|
// user and Pick is the only way anything is fetched.
|
||||||
|
dl := fourTrackDownload()
|
||||||
|
dl.Expected = nil
|
||||||
|
|
||||||
|
ranked, err := f.manager.Start(context.Background(), dl)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Start: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Nothing was grabbed while waiting for the user.
|
|
||||||
if a.GrabCalls != 0 || b.GrabCalls != 0 {
|
if a.GrabCalls != 0 || b.GrabCalls != 0 {
|
||||||
t.Errorf(
|
t.Errorf(
|
||||||
"grabs happened without a pick: a=%d b=%d",
|
"grabs happened without a pick: a=%d b=%d",
|
||||||
@@ -258,7 +297,6 @@ func TestManagerWaitsWhenAmbiguous(t *testing.T) {
|
|||||||
t.Errorf("stored request id = %s, want %s", stored.ID, dl.ID)
|
t.Errorf("stored request id = %s, want %s", stored.ID, dl.ID)
|
||||||
}
|
}
|
||||||
|
|
||||||
// The user picks the second one explicitly.
|
|
||||||
if err := f.manager.Pick(
|
if err := f.manager.Pick(
|
||||||
context.Background(), dl.ID, ranked[1].ID,
|
context.Background(), dl.ID, ranked[1].ID,
|
||||||
); err != nil {
|
); err != nil {
|
||||||
|
|||||||
+313
-77
@@ -1,6 +1,7 @@
|
|||||||
package download
|
package download
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"fmt"
|
||||||
"math"
|
"math"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -34,38 +35,102 @@ const (
|
|||||||
weightArtistFit = 0.12
|
weightArtistFit = 0.12
|
||||||
)
|
)
|
||||||
|
|
||||||
// Quality sub-weights. They sum to 1.0 along with weightSizeFit below.
|
// Quality sub-weights. Each set sums to 1.0.
|
||||||
|
//
|
||||||
|
// There are two of them because a stated preference changes what the
|
||||||
|
// other numbers are *for*. `formatRank` and `bitrateScore` are the
|
||||||
|
// app guessing at how good a copy is — FLAC over MP3, 320 over 128 —
|
||||||
|
// and that guess exists precisely because the user has not said. Once
|
||||||
|
// they have, the guess should not outvote them: with the old single set
|
||||||
|
// a preference of 320 kbps moved a candidate's score by at most 0.05
|
||||||
|
// against the 0.42 riding on format, so asking for 320 and being handed
|
||||||
|
// a FLAC every time was the *designed* behaviour. That is the same
|
||||||
|
// fault the megabyte window had — a preference the user can express and
|
||||||
|
// the ranking can ignore.
|
||||||
const (
|
const (
|
||||||
weightFormat = 0.42
|
weightFormat = 0.42
|
||||||
weightBitrate = 0.23
|
weightBitrate = 0.23
|
||||||
weightHealth = 0.20
|
weightHealth = 0.20
|
||||||
weightPriority = 0.10
|
weightPriority = 0.10
|
||||||
weightSizeFit = 0.05
|
weightBitrateFit = 0.05
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// Quality sub-weights when the user has named a preferred bitrate.
|
||||||
|
// The weight comes off format and bitrate — the two proxies the
|
||||||
|
// preference replaces — and health and priority are untouched, since
|
||||||
|
// neither is a stand-in for anything the user just said.
|
||||||
|
const (
|
||||||
|
statedWeightFormat = 0.20
|
||||||
|
statedWeightBitrate = 0.10
|
||||||
|
statedWeightHealth = 0.20
|
||||||
|
statedWeightPriority = 0.10
|
||||||
|
statedWeightBitrateFit = 0.40
|
||||||
|
)
|
||||||
|
|
||||||
|
// qualityWeights picks the set, in the order scoreQuality applies them.
|
||||||
|
func qualityWeights(p AutoDownloadPrefs) (
|
||||||
|
format, bitrate, health, priority, fit float64,
|
||||||
|
) {
|
||||||
|
if p.PreferredKbps > 0 {
|
||||||
|
return statedWeightFormat,
|
||||||
|
statedWeightBitrate,
|
||||||
|
statedWeightHealth,
|
||||||
|
statedWeightPriority,
|
||||||
|
statedWeightBitrateFit
|
||||||
|
}
|
||||||
|
|
||||||
|
return weightFormat,
|
||||||
|
weightBitrate,
|
||||||
|
weightHealth,
|
||||||
|
weightPriority,
|
||||||
|
weightBitrateFit
|
||||||
|
}
|
||||||
|
|
||||||
// unanchoredCap bounds the match score of a free-text request. Without
|
// unanchoredCap bounds the match score of a free-text request. Without
|
||||||
// an MBID there is no tracklist to be right about, so a confident-
|
// an MBID there is no tracklist to be right about, so a confident-
|
||||||
// looking score would be a lie — and auto-pick keys off this.
|
// looking score would be a lie — and auto-pick keys off this.
|
||||||
const unanchoredCap = 0.65
|
const unanchoredCap = 0.65
|
||||||
|
|
||||||
// AutoDownloadPrefs gates and scores what AutoPickable may choose
|
// AutoDownloadPrefs gates and scores what AutoPickable may choose
|
||||||
// without asking. Zero values are permissive: no size window and no
|
// without asking. Zero values are permissive: no bitrate window, no
|
||||||
// format restriction.
|
// size ceiling and no format restriction.
|
||||||
|
//
|
||||||
|
// **The window is a rate, not a size.** It used to be three numbers in
|
||||||
|
// megabytes, which cannot mean anything on their own: 300 MB is a
|
||||||
|
// generous FLAC single and a suspiciously small boxset, and the user
|
||||||
|
// setting the number has no idea which release the pipeline will
|
||||||
|
// eventually apply it to. A bitrate is the same statement normalised
|
||||||
|
// by how long the music is, so one number holds across a 9-minute EP
|
||||||
|
// and a 3-hour opera — and it is the unit the thing being described is
|
||||||
|
// actually measured in. The runtime is known for every request
|
||||||
|
// auto-pick can act on (`Download.Expected` carries per-track lengths,
|
||||||
|
// and an anchored request is the only kind that reaches here), so this
|
||||||
|
// costs no extra lookup.
|
||||||
type AutoDownloadPrefs struct {
|
type AutoDownloadPrefs struct {
|
||||||
// MinSizeMB and MaxSizeMB bound what auto-pick will grab. Zero
|
// MinKbps and MaxKbps bound the average bitrate auto-pick will
|
||||||
// means no bound on that side. A candidate outside the window is
|
// grab. Zero means no bound on that side. A candidate outside the
|
||||||
// filtered out of auto-pick entirely, not merely scored down — a
|
// window is filtered out of auto-pick entirely, not merely scored
|
||||||
// tiny "sampler" torrent or a boxset ten times the expected size is
|
// down — a 96 kbps rip of the right album is not a worse copy the
|
||||||
// usually the wrong thing entirely, not a worse copy of the right
|
// user might accept, it is one they said not to take unattended.
|
||||||
// thing.
|
//
|
||||||
MinSizeMB int `json:"minSizeMb"`
|
// For reference: 320 is the top of MP3, ~500–1000 is FLAC depending
|
||||||
MaxSizeMB int `json:"maxSizeMb"`
|
// on the material, and anything under ~128 is a transcode.
|
||||||
|
MinKbps int `json:"minKbps"`
|
||||||
|
MaxKbps int `json:"maxKbps"`
|
||||||
|
|
||||||
// PreferredSizeMB nudges the score toward a target size within the
|
// PreferredKbps nudges the score toward a target rate within the
|
||||||
// min/max window (a lossless rip and a heavily-padded lossless rip
|
// window, and breaks the tie when several candidates are equally
|
||||||
// can both pass the window). Zero disables the nudge; sizeFit then
|
// good matches. Zero disables the nudge; bitrateFit then returns a
|
||||||
// returns a neutral value that does not affect ranking.
|
// neutral value that does not affect ranking.
|
||||||
PreferredSizeMB int `json:"preferredSizeMb"`
|
PreferredKbps int `json:"preferredKbps"`
|
||||||
|
|
||||||
|
// MaxSizeMB is a hard ceiling on the whole candidate, and it is
|
||||||
|
// deliberately still a size. It answers a different question from
|
||||||
|
// the window above — not "is this the quality I want" but "is this
|
||||||
|
// going to fill the disk" — and it has to hold even for a candidate
|
||||||
|
// whose bitrate cannot be worked out, which is exactly the shape a
|
||||||
|
// mislabelled boxset arrives in. Zero means no ceiling.
|
||||||
|
MaxSizeMB int `json:"maxSizeMb"`
|
||||||
|
|
||||||
// AllowedFormats restricts auto-pick to candidates whose audio
|
// AllowedFormats restricts auto-pick to candidates whose audio
|
||||||
// files are all in one of these formats. Empty means no
|
// files are all in one of these formats. Empty means no
|
||||||
@@ -74,19 +139,33 @@ type AutoDownloadPrefs struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// eligible reports whether a candidate may be auto-picked under these
|
// eligible reports whether a candidate may be auto-picked under these
|
||||||
// preferences: within the size window (when set) and, when a format
|
// preferences: inside the bitrate window and the size ceiling (when
|
||||||
// list is given, every audio file in an allowed format.
|
// set) and, when a format list is given, every audio file in an
|
||||||
func (p AutoDownloadPrefs) eligible(c Candidate) bool {
|
// allowed format.
|
||||||
|
//
|
||||||
|
// `runtimeMillis` is how long the requested release is, and 0 means
|
||||||
|
// nobody knows. An unknown runtime **passes** the bitrate window
|
||||||
|
// rather than failing it: the window is a statement about quality, and
|
||||||
|
// refusing everything the moment a tracklist is missing a length would
|
||||||
|
// turn a gap in MusicBrainz into a silent embargo. The size ceiling
|
||||||
|
// still applies, which is why it exists separately.
|
||||||
|
func (p AutoDownloadPrefs) eligible(c Candidate, runtimeMillis int64) bool {
|
||||||
const bytesPerMB = 1 << 20
|
const bytesPerMB = 1 << 20
|
||||||
|
|
||||||
if p.MinSizeMB > 0 && c.TotalSize < int64(p.MinSizeMB)*bytesPerMB {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
|
|
||||||
if p.MaxSizeMB > 0 && c.TotalSize > int64(p.MaxSizeMB)*bytesPerMB {
|
if p.MaxSizeMB > 0 && c.TotalSize > int64(p.MaxSizeMB)*bytesPerMB {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if kbps := candidateKbps(c, runtimeMillis); kbps > 0 {
|
||||||
|
if p.MinKbps > 0 && kbps < float64(p.MinKbps) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
if p.MaxKbps > 0 && kbps > float64(p.MaxKbps) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
if len(p.AllowedFormats) == 0 {
|
if len(p.AllowedFormats) == 0 {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
@@ -107,11 +186,14 @@ func (p AutoDownloadPrefs) eligible(c Candidate) bool {
|
|||||||
|
|
||||||
// filter returns only the candidates these preferences allow to be
|
// filter returns only the candidates these preferences allow to be
|
||||||
// auto-picked, in the same (already ranked) order.
|
// auto-picked, in the same (already ranked) order.
|
||||||
func (p AutoDownloadPrefs) filter(ranked []Candidate) []Candidate {
|
func (p AutoDownloadPrefs) filter(
|
||||||
|
ranked []Candidate,
|
||||||
|
runtimeMillis int64,
|
||||||
|
) []Candidate {
|
||||||
out := make([]Candidate, 0, len(ranked))
|
out := make([]Candidate, 0, len(ranked))
|
||||||
|
|
||||||
for _, c := range ranked {
|
for _, c := range ranked {
|
||||||
if p.eligible(c) {
|
if p.eligible(c, runtimeMillis) {
|
||||||
out = append(out, c)
|
out = append(out, c)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -119,32 +201,116 @@ func (p AutoDownloadPrefs) filter(ranked []Candidate) []Candidate {
|
|||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
// sizeFit scores how close totalSize is to PreferredSizeMB, 0..1,
|
// bitrateFit scores how close a candidate's average bitrate is to
|
||||||
// falling off linearly as the size doubles or halves away from it.
|
// PreferredKbps, falling off linearly as it doubles or halves away
|
||||||
// Returns a neutral 0.5 when no preference is set, so the absence of a
|
// from it.
|
||||||
// preference does not bias ranking.
|
//
|
||||||
func (p AutoDownloadPrefs) sizeFit(totalSize int64) float64 {
|
// The range is **0.5 to 1.0, not 0 to 1**, and the floor is the point.
|
||||||
|
// This carries 0.40 of the quality score once a preference is set, so a
|
||||||
|
// span down to zero would let a preference of 320 kbps push a perfectly
|
||||||
|
// good FLAC under `minQuality` and out of auto-pick altogether —
|
||||||
|
// turning "I like 320" into "never take anything else", silently. A
|
||||||
|
// preference may promote the copy that matches it; it may not
|
||||||
|
// disqualify the others. That is what `MinKbps`/`MaxKbps` are for, and
|
||||||
|
// they say so out loud.
|
||||||
|
//
|
||||||
|
// Returns the neutral floor when no preference is set or the rate
|
||||||
|
// cannot be worked out, so neither an absent preference nor an absent
|
||||||
|
// runtime biases ranking.
|
||||||
|
func (p AutoDownloadPrefs) bitrateFit(
|
||||||
|
c Candidate,
|
||||||
|
runtimeMillis int64,
|
||||||
|
) float64 {
|
||||||
const (
|
const (
|
||||||
bytesPerMB = 1 << 20
|
neutral = 0.5
|
||||||
neutral = 0.5
|
span = 0.5
|
||||||
)
|
)
|
||||||
|
|
||||||
if p.PreferredSizeMB <= 0 || totalSize <= 0 {
|
if p.PreferredKbps <= 0 {
|
||||||
return neutral
|
return neutral
|
||||||
}
|
}
|
||||||
|
|
||||||
preferred := float64(p.PreferredSizeMB) * bytesPerMB
|
kbps := candidateKbps(c, runtimeMillis)
|
||||||
ratio := float64(totalSize) / preferred
|
if kbps <= 0 {
|
||||||
|
return neutral
|
||||||
|
}
|
||||||
|
|
||||||
|
ratio := kbps / float64(p.PreferredKbps)
|
||||||
if ratio < 1 {
|
if ratio < 1 {
|
||||||
ratio = 1 / ratio
|
ratio = 1 / ratio
|
||||||
}
|
}
|
||||||
|
|
||||||
// ratio is now >= 1: 1.0 is an exact match, 2.0 is double or half
|
// ratio is now >= 1: 1.0 is an exact match, 2.0 is double or half
|
||||||
// the preferred size. Falls to 0 at 2x away and beyond.
|
// the preferred rate, where the closeness term reaches 0.
|
||||||
fit := 1 - (ratio - 1)
|
return neutral + span*clamp01(1-(ratio-1))
|
||||||
|
}
|
||||||
|
|
||||||
return clamp01(fit)
|
// candidateKbps is a candidate's average audio bitrate, or 0 when it
|
||||||
|
// cannot be worked out.
|
||||||
|
//
|
||||||
|
// Two sources, in this order, and the order matters:
|
||||||
|
//
|
||||||
|
// - **Derived from bytes over runtime**, which is the honest one. It
|
||||||
|
// covers lossless (where a stated bitrate rarely exists), it cannot
|
||||||
|
// be lied to by a filename, and it is what the user's window means.
|
||||||
|
// Only the *audio* files count: cover scans and a log file are not
|
||||||
|
// part of the bitrate, and a folder with 30 MB of artwork would
|
||||||
|
// otherwise read as a better rip than the same music without it.
|
||||||
|
// - **The mean stated bitrate**, when the runtime is unknown. Weaker
|
||||||
|
// — a provider that parses it from an MP3 header states it and one
|
||||||
|
// that guesses from the filename also "states" it — but a number
|
||||||
|
// from the file itself beats no number at all.
|
||||||
|
func candidateKbps(c Candidate, runtimeMillis int64) float64 {
|
||||||
|
const bitsPerByte = 8
|
||||||
|
|
||||||
|
audio := c.AudioFiles()
|
||||||
|
if len(audio) == 0 {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
if runtimeMillis > 0 {
|
||||||
|
var bytes int64
|
||||||
|
for _, f := range audio {
|
||||||
|
bytes += f.Size
|
||||||
|
}
|
||||||
|
|
||||||
|
if bytes > 0 {
|
||||||
|
// bytes×8 bits over seconds, expressed in kbps: the two
|
||||||
|
// factors of 1000 (millis→seconds, bits→kilobits) cancel.
|
||||||
|
return float64(bytes) * bitsPerByte /
|
||||||
|
float64(runtimeMillis)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
var (
|
||||||
|
sum int
|
||||||
|
count int
|
||||||
|
)
|
||||||
|
|
||||||
|
for _, f := range audio {
|
||||||
|
if f.Bitrate > 0 {
|
||||||
|
sum += f.Bitrate
|
||||||
|
count++
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if count == 0 {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
return float64(sum) / float64(count)
|
||||||
|
}
|
||||||
|
|
||||||
|
// runtimeMillis is how long the requested release is, summed over its
|
||||||
|
// expected tracklist. Zero when the tracklist is absent or carries no
|
||||||
|
// lengths, which is what every caller here treats as "unknown".
|
||||||
|
func (d Download) runtimeMillis() int64 {
|
||||||
|
var total int64
|
||||||
|
for _, t := range d.Expected {
|
||||||
|
total += t.LengthMillis
|
||||||
|
}
|
||||||
|
|
||||||
|
return total
|
||||||
}
|
}
|
||||||
|
|
||||||
// Score fills a candidate's Match, Quality and Score fields.
|
// Score fills a candidate's Match, Quality and Score fields.
|
||||||
@@ -160,7 +326,9 @@ func Score(dl Download, c Candidate, priority int, prefs AutoDownloadPrefs) Cand
|
|||||||
c.Files = mergeMatched(c.Files, matched)
|
c.Files = mergeMatched(c.Files, matched)
|
||||||
|
|
||||||
c.Match = scoreMatch(dl, c, audio, titleFit)
|
c.Match = scoreMatch(dl, c, audio, titleFit)
|
||||||
c.Quality = scoreQuality(c, audio, priority, prefs)
|
c.Quality = scoreQuality(
|
||||||
|
c, audio, priority, prefs, dl.runtimeMillis(),
|
||||||
|
)
|
||||||
|
|
||||||
c.Score = weightMatch*c.Match.Overall + weightQuality*c.Quality.Overall
|
c.Score = weightMatch*c.Match.Overall + weightQuality*c.Quality.Overall
|
||||||
|
|
||||||
@@ -279,11 +447,12 @@ func scoreQuality(
|
|||||||
audio []CandidateFile,
|
audio []CandidateFile,
|
||||||
priority int,
|
priority int,
|
||||||
prefs AutoDownloadPrefs,
|
prefs AutoDownloadPrefs,
|
||||||
|
runtimeMillis int64,
|
||||||
) QualityScore {
|
) QualityScore {
|
||||||
q := QualityScore{
|
q := QualityScore{
|
||||||
Health: clamp01(c.Health),
|
Health: clamp01(c.Health),
|
||||||
Priority: clamp01(float64(priority) / 100.0),
|
Priority: clamp01(float64(priority) / 100.0),
|
||||||
SizeFit: prefs.sizeFit(c.TotalSize),
|
BitrateFit: prefs.bitrateFit(c, runtimeMillis),
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(audio) == 0 {
|
if len(audio) == 0 {
|
||||||
@@ -310,11 +479,13 @@ func scoreQuality(
|
|||||||
q.FormatRank = worst
|
q.FormatRank = worst
|
||||||
q.Bitrate = bitrateScore(audio)
|
q.Bitrate = bitrateScore(audio)
|
||||||
|
|
||||||
q.Overall = weightFormat*q.FormatRank +
|
wFormat, wBitrate, wHealth, wPriority, wFit := qualityWeights(prefs)
|
||||||
weightBitrate*q.Bitrate +
|
|
||||||
weightHealth*q.Health +
|
q.Overall = wFormat*q.FormatRank +
|
||||||
weightPriority*q.Priority +
|
wBitrate*q.Bitrate +
|
||||||
weightSizeFit*q.SizeFit
|
wHealth*q.Health +
|
||||||
|
wPriority*q.Priority +
|
||||||
|
wFit*q.BitrateFit
|
||||||
|
|
||||||
if q.Mixed {
|
if q.Mixed {
|
||||||
q.Overall *= 0.9
|
q.Overall *= 0.9
|
||||||
@@ -444,6 +615,19 @@ func Rank(
|
|||||||
return out[i].Match.Overall > out[j].Match.Overall
|
return out[i].Match.Overall > out[j].Match.Overall
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Closest to the preferred bitrate wins the tie.
|
||||||
|
//
|
||||||
|
// This is what decides which copy is taken now that auto-pick
|
||||||
|
// no longer requires the winner to be clear of the field: when
|
||||||
|
// several candidates are equally good matches of equal overall
|
||||||
|
// quality, the one the user said they wanted the shape of is
|
||||||
|
// the answer, ahead of provider priority. With no preference
|
||||||
|
// set every BitrateFit is the same neutral value and this
|
||||||
|
// falls through, exactly as before.
|
||||||
|
if out[i].Quality.BitrateFit != out[j].Quality.BitrateFit {
|
||||||
|
return out[i].Quality.BitrateFit > out[j].Quality.BitrateFit
|
||||||
|
}
|
||||||
|
|
||||||
if out[i].Quality.Priority != out[j].Quality.Priority {
|
if out[i].Quality.Priority != out[j].Quality.Priority {
|
||||||
return out[i].Quality.Priority > out[j].Quality.Priority
|
return out[i].Quality.Priority > out[j].Quality.Priority
|
||||||
}
|
}
|
||||||
@@ -454,19 +638,58 @@ func Rank(
|
|||||||
return out
|
return out
|
||||||
}
|
}
|
||||||
|
|
||||||
// AutoPickable reports whether a ranked list has a clear enough winner
|
// Auto-pick gates. Named rather than inlined because AutoPickVeto
|
||||||
// to grab without asking. It demands an anchored request, a high match,
|
// reports which of them refused, and a number in a sentence the user
|
||||||
// decent quality, and daylight between first and second place — if two
|
// reads should be the same number the decision used.
|
||||||
// candidates are close, the choice is the user's.
|
const (
|
||||||
func AutoPickable(dl Download, ranked []Candidate, prefs AutoDownloadPrefs) bool {
|
minMatch = 0.85
|
||||||
const (
|
minQuality = 0.5
|
||||||
minMatch = 0.85
|
)
|
||||||
minQuality = 0.5
|
|
||||||
minLead = 0.08
|
|
||||||
)
|
|
||||||
|
|
||||||
if !dl.Anchored() || len(ranked) == 0 {
|
// AutoPickable reports whether a ranked list has a candidate worth
|
||||||
return false
|
// grabbing without asking: an anchored request with a tracklist behind
|
||||||
|
// it, and a candidate that clears the match and quality bars inside the
|
||||||
|
// user's guardrails.
|
||||||
|
//
|
||||||
|
// **It does not require the winner to be better than the runner-up.**
|
||||||
|
// It used to demand 0.08 of daylight on the combined score, which meant
|
||||||
|
// the check fired hardest in the case it was never written for: a
|
||||||
|
// popular album turns up five *correct* copies, all matching the
|
||||||
|
// tracklist at 95%+ and differing only in format and seeders, their
|
||||||
|
// scores land within a point of each other, and auto-pick refused
|
||||||
|
// forever on the grounds that the choice was the user's. It was not.
|
||||||
|
// There was no question about *what* to fetch, only about which copy —
|
||||||
|
// and abundance is the one condition under which that question matters
|
||||||
|
// least. A candidate does not need to be the best one, only one that
|
||||||
|
// meets the criteria; where several do, `Rank` puts the one closest to
|
||||||
|
// the preferred bitrate first.
|
||||||
|
func AutoPickable(dl Download, ranked []Candidate, prefs AutoDownloadPrefs) bool {
|
||||||
|
return AutoPickVeto(dl, ranked, prefs) == ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// AutoPickVeto returns the reason auto-pick declined, or "" when it
|
||||||
|
// would go ahead.
|
||||||
|
//
|
||||||
|
// It exists because "it rejected all of them" was indistinguishable
|
||||||
|
// from "it found nothing good". The request list's message was built
|
||||||
|
// from `ranked[0]` — the best candidate *before* the size and format
|
||||||
|
// guardrails, and before the lead check — so a request refused because
|
||||||
|
// the user's maximum size excluded every copy, or because three equally
|
||||||
|
// good copies were found, reported "best of 12 found is not a confident
|
||||||
|
// enough match (match 96%, quality 88%)". Numbers that clear both
|
||||||
|
// thresholds, beside a refusal, is a message that teaches the user the
|
||||||
|
// matcher is broken. Each gate names itself now.
|
||||||
|
func AutoPickVeto(
|
||||||
|
dl Download,
|
||||||
|
ranked []Candidate,
|
||||||
|
prefs AutoDownloadPrefs,
|
||||||
|
) string {
|
||||||
|
if len(ranked) == 0 {
|
||||||
|
return "nothing found"
|
||||||
|
}
|
||||||
|
|
||||||
|
if !dl.Anchored() {
|
||||||
|
return "the request is free text, so there is no release to be right about"
|
||||||
}
|
}
|
||||||
|
|
||||||
// An anchor with no tracklist behind it is an anchor in name only:
|
// An anchor with no tracklist behind it is an anchor in name only:
|
||||||
@@ -474,29 +697,42 @@ func AutoPickable(dl Download, ranked []Candidate, prefs AutoDownloadPrefs) bool
|
|||||||
// is exactly the evidence a wrong-album candidate also has. This
|
// is exactly the evidence a wrong-album candidate also has. This
|
||||||
// matters most for the request list, where nobody is watching.
|
// matters most for the request list, where nobody is watching.
|
||||||
if len(dl.Expected) == 0 {
|
if len(dl.Expected) == 0 {
|
||||||
return false
|
return "no tracklist for this release is known yet, so a candidate cannot be checked against it"
|
||||||
}
|
}
|
||||||
|
|
||||||
// The guardrails apply before the match/quality/lead checks: a
|
// The guardrails apply before the match and quality checks: a
|
||||||
// candidate outside the allowed size or format is not a worse
|
// candidate outside the allowed bitrate, size or format is not a
|
||||||
// choice, it is not a choice auto-pick may make at all, so it must
|
// worse choice, it is not a choice auto-pick may make at all, so it
|
||||||
// not count as "the winner" nor as "second place" for the lead
|
// must not count as "the winner" either.
|
||||||
// check below.
|
eligible := prefs.filter(ranked, dl.runtimeMillis())
|
||||||
eligible := prefs.filter(ranked)
|
|
||||||
if len(eligible) == 0 {
|
if len(eligible) == 0 {
|
||||||
return false
|
return fmt.Sprintf(
|
||||||
|
"all %d found are outside the auto-download bitrate, size or format limits",
|
||||||
|
len(ranked),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
best := eligible[0]
|
best := eligible[0]
|
||||||
if best.Match.Overall < minMatch || best.Quality.Overall < minQuality {
|
|
||||||
return false
|
if best.Match.Overall < minMatch {
|
||||||
|
return fmt.Sprintf(
|
||||||
|
"best of %d found matches this release only %.0f%% (needs %.0f%%)",
|
||||||
|
len(ranked),
|
||||||
|
best.Match.Overall*100, //nolint:mnd // percent
|
||||||
|
minMatch*100, //nolint:mnd // percent
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(eligible) > 1 && best.Score-eligible[1].Score < minLead {
|
if best.Quality.Overall < minQuality {
|
||||||
return false
|
return fmt.Sprintf(
|
||||||
|
"best of %d found is the right release but scores %.0f%% on quality (needs %.0f%%)",
|
||||||
|
len(ranked),
|
||||||
|
best.Quality.Overall*100, //nolint:mnd // percent
|
||||||
|
minQuality*100, //nolint:mnd // percent
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
return true
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
// mergeMatched copies MatchedTo assignments from the audio-only slice
|
// mergeMatched copies MatchedTo assignments from the audio-only slice
|
||||||
|
|||||||
+360
-57
@@ -1,6 +1,34 @@
|
|||||||
package download
|
package download
|
||||||
|
|
||||||
import "testing"
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// trackMillis is five minutes; okComputer's four of them make a
|
||||||
|
// twenty-minute release, which is what turns a candidate's byte count
|
||||||
|
// into a bitrate the assertions below can name.
|
||||||
|
const trackMillis = 5 * 60 * 1000
|
||||||
|
|
||||||
|
// okComputerRuntime is that release's runtime, for the helpers that
|
||||||
|
// need it directly.
|
||||||
|
const okComputerRuntime = 4 * trackMillis
|
||||||
|
|
||||||
|
// kbpsCandidate builds an annotated candidate whose audio adds up to
|
||||||
|
// the given average bitrate over okComputer's runtime.
|
||||||
|
func kbpsCandidate(id, ext string, kbps int) Candidate {
|
||||||
|
// bits = kbps × 1000 × (runtimeMillis / 1000), so the thousands
|
||||||
|
// cancel and the byte count is kbps × runtimeMillis / 8.
|
||||||
|
const bitsPerByte = 8
|
||||||
|
|
||||||
|
total := int64(kbps) * okComputerRuntime / bitsPerByte
|
||||||
|
|
||||||
|
c := candidateFor(id, allTitles(), ext, total/int64(len(allTitles())))
|
||||||
|
c.Files = AnnotateFiles(c.Files)
|
||||||
|
c.TotalSize = total
|
||||||
|
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
// okComputer is the reference request used across ranking tests.
|
// okComputer is the reference request used across ranking tests.
|
||||||
func okComputer() Download {
|
func okComputer() Download {
|
||||||
@@ -8,11 +36,15 @@ func okComputer() Download {
|
|||||||
ReleaseMBID: "mbid-ok-computer",
|
ReleaseMBID: "mbid-ok-computer",
|
||||||
Artist: "Radiohead",
|
Artist: "Radiohead",
|
||||||
Album: "OK Computer",
|
Album: "OK Computer",
|
||||||
|
// Four five-minute tracks: twenty minutes, so a candidate's
|
||||||
|
// bitrate is a number these tests can state exactly. Without
|
||||||
|
// lengths there is no runtime and the bitrate window has
|
||||||
|
// nothing to divide by.
|
||||||
Expected: []ExpectedTrack{
|
Expected: []ExpectedTrack{
|
||||||
{Position: 1, Title: "Airbag"},
|
{Position: 1, Title: "Airbag", LengthMillis: trackMillis},
|
||||||
{Position: 2, Title: "Paranoid Android"},
|
{Position: 2, Title: "Paranoid Android", LengthMillis: trackMillis},
|
||||||
{Position: 3, Title: "Subterranean Homesick Alien"},
|
{Position: 3, Title: "Subterranean Homesick Alien", LengthMillis: trackMillis},
|
||||||
{Position: 4, Title: "Exit Music (For a Film)"},
|
{Position: 4, Title: "Exit Music (For a Film)", LengthMillis: trackMillis},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -187,7 +219,7 @@ func TestUnanchoredMatchIsCapped(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestAutoPickableRequiresAnchorAndLead(t *testing.T) {
|
func TestAutoPickableRequiresAnchorAndTracklist(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
dl := okComputer()
|
dl := okComputer()
|
||||||
@@ -211,14 +243,18 @@ func TestAutoPickableRequiresAnchorAndLead(t *testing.T) {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
t.Run("two close candidates are not", func(t *testing.T) {
|
// Two identical copies are a spare, not an ambiguity. This
|
||||||
|
// asserted the opposite while auto-pick required daylight over the
|
||||||
|
// runner-up — a rule that made abundance the thing that stopped a
|
||||||
|
// request being satisfied, which is backwards.
|
||||||
|
t.Run("two equally good candidates still are", func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
twin := best
|
twin := best
|
||||||
twin.ID = "twin"
|
twin.ID = "twin"
|
||||||
|
|
||||||
if AutoPickable(dl, []Candidate{best, twin}, AutoDownloadPrefs{}) {
|
if !AutoPickable(dl, []Candidate{best, twin}, AutoDownloadPrefs{}) {
|
||||||
t.Error("identical candidates must not auto-pick")
|
t.Error("identical good candidates must auto-pick")
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -300,18 +336,11 @@ func TestProviderPriorityBreaksTies(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const mb = 1 << 20
|
|
||||||
|
|
||||||
func TestAutoDownloadPrefsEligible(t *testing.T) {
|
func TestAutoDownloadPrefsEligible(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
flacCandidate := candidateFor("c", allTitles(), ".flac", 30_000_000)
|
flacCandidate := kbpsCandidate("c", ".flac", 900)
|
||||||
flacCandidate.Files = AnnotateFiles(flacCandidate.Files)
|
mp3Candidate := kbpsCandidate("c", ".mp3", 128)
|
||||||
flacCandidate.TotalSize = 300 * mb
|
|
||||||
|
|
||||||
mp3Candidate := candidateFor("c", allTitles(), ".mp3", 3_000_000)
|
|
||||||
mp3Candidate.Files = AnnotateFiles(mp3Candidate.Files)
|
|
||||||
mp3Candidate.TotalSize = 30 * mb
|
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
@@ -321,18 +350,25 @@ func TestAutoDownloadPrefsEligible(t *testing.T) {
|
|||||||
}{
|
}{
|
||||||
{"zero value is permissive", AutoDownloadPrefs{}, flacCandidate, true},
|
{"zero value is permissive", AutoDownloadPrefs{}, flacCandidate, true},
|
||||||
{
|
{
|
||||||
"within min/max window",
|
"within the bitrate window",
|
||||||
AutoDownloadPrefs{MinSizeMB: 100, MaxSizeMB: 500},
|
AutoDownloadPrefs{MinKbps: 320, MaxKbps: 1200},
|
||||||
flacCandidate, true,
|
flacCandidate, true,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"below minimum",
|
"below the minimum bitrate",
|
||||||
AutoDownloadPrefs{MinSizeMB: 400},
|
AutoDownloadPrefs{MinKbps: 500},
|
||||||
|
mp3Candidate, false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"above the maximum bitrate",
|
||||||
|
AutoDownloadPrefs{MaxKbps: 500},
|
||||||
flacCandidate, false,
|
flacCandidate, false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"above maximum",
|
// The ceiling is bytes, not a rate, and it is the guard
|
||||||
AutoDownloadPrefs{MaxSizeMB: 200},
|
// that still works when the bitrate cannot be worked out.
|
||||||
|
"above the hard size ceiling",
|
||||||
|
AutoDownloadPrefs{MaxSizeMB: 50},
|
||||||
flacCandidate, false,
|
flacCandidate, false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
@@ -351,57 +387,131 @@ func TestAutoDownloadPrefsEligible(t *testing.T) {
|
|||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
if got := tt.prefs.eligible(tt.c); got != tt.want {
|
got := tt.prefs.eligible(tt.c, okComputerRuntime)
|
||||||
|
if got != tt.want {
|
||||||
t.Errorf("eligible() = %v, want %v", got, tt.want)
|
t.Errorf("eligible() = %v, want %v", got, tt.want)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A release nobody knows the length of cannot be judged on bitrate, and
|
||||||
|
// the window must not become a silent embargo because MusicBrainz is
|
||||||
|
// missing a track length. The size ceiling still applies — that is why
|
||||||
|
// it is a separate field.
|
||||||
|
func TestBitrateWindowPassesAnUnknownRuntime(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := kbpsCandidate("c", ".mp3", 128)
|
||||||
|
prefs := AutoDownloadPrefs{MinKbps: 900}
|
||||||
|
|
||||||
|
if !prefs.eligible(c, 0) {
|
||||||
|
t.Error("an unknown runtime must pass the bitrate window")
|
||||||
|
}
|
||||||
|
|
||||||
|
if prefs.eligible(c, okComputerRuntime) {
|
||||||
|
t.Error("a known runtime must still be judged")
|
||||||
|
}
|
||||||
|
|
||||||
|
ceiling := AutoDownloadPrefs{MaxSizeMB: 1}
|
||||||
|
if ceiling.eligible(c, 0) {
|
||||||
|
t.Error("the size ceiling must apply even with no runtime")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Artwork is not part of the bitrate. A folder carrying 30 MB of
|
||||||
|
// scans would otherwise read as a better rip than the same music
|
||||||
|
// without them, which is backwards.
|
||||||
|
func TestBitrateIgnoresNonAudioFiles(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := kbpsCandidate("c", ".mp3", 320)
|
||||||
|
bare := candidateKbps(c, okComputerRuntime)
|
||||||
|
|
||||||
|
c.Files = append(c.Files, CandidateFile{
|
||||||
|
Path: "Radiohead - OK Computer/cover.jpg",
|
||||||
|
Size: 30 << 20,
|
||||||
|
})
|
||||||
|
c.Files = AnnotateFiles(c.Files)
|
||||||
|
|
||||||
|
if got := candidateKbps(c, okComputerRuntime); got != bare {
|
||||||
|
t.Errorf("bitrate with artwork = %f, want %f", got, bare)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Where no runtime is known, a stated per-file bitrate is better than
|
||||||
|
// no answer at all.
|
||||||
|
func TestBitrateFallsBackToTheStatedRate(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
c := candidateFor("c", allTitles(), ".mp3", 3_000_000)
|
||||||
|
for i := range c.Files {
|
||||||
|
c.Files[i].Bitrate = 192
|
||||||
|
}
|
||||||
|
|
||||||
|
c.Files = AnnotateFiles(c.Files)
|
||||||
|
|
||||||
|
if got := candidateKbps(c, 0); got != 192 {
|
||||||
|
t.Errorf("stated bitrate = %f, want 192", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestAutoDownloadPrefsFilter(t *testing.T) {
|
func TestAutoDownloadPrefsFilter(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
small := candidateFor("small", allTitles(), ".flac", 10_000_000)
|
lossy := kbpsCandidate("lossy", ".mp3", 128)
|
||||||
small.TotalSize = 50 * mb
|
lossless := kbpsCandidate("lossless", ".flac", 900)
|
||||||
|
|
||||||
big := candidateFor("big", allTitles(), ".flac", 30_000_000)
|
prefs := AutoDownloadPrefs{MinKbps: 500}
|
||||||
big.TotalSize = 500 * mb
|
|
||||||
|
|
||||||
prefs := AutoDownloadPrefs{MinSizeMB: 100, MaxSizeMB: 600}
|
filtered := prefs.filter(
|
||||||
|
[]Candidate{lossy, lossless}, okComputerRuntime,
|
||||||
|
)
|
||||||
|
|
||||||
filtered := prefs.filter([]Candidate{small, big})
|
if len(filtered) != 1 || filtered[0].ID != "lossless" {
|
||||||
|
|
||||||
if len(filtered) != 1 || filtered[0].ID != "big" {
|
|
||||||
t.Errorf("filter() = %v, want only the in-window candidate", filtered)
|
t.Errorf("filter() = %v, want only the in-window candidate", filtered)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestAutoDownloadPrefsSizeFit(t *testing.T) {
|
func TestAutoDownloadPrefsBitrateFit(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
const neutral = 0.5
|
const neutral = 0.5
|
||||||
|
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
prefs AutoDownloadPrefs
|
prefs AutoDownloadPrefs
|
||||||
totalSize int64
|
c Candidate
|
||||||
want float64
|
want float64
|
||||||
}{
|
}{
|
||||||
{"no preference is neutral", AutoDownloadPrefs{}, 300 * mb, neutral},
|
{
|
||||||
|
"no preference is neutral",
|
||||||
|
AutoDownloadPrefs{},
|
||||||
|
kbpsCandidate("c", ".flac", 900), neutral,
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"exact match scores 1",
|
"exact match scores 1",
|
||||||
AutoDownloadPrefs{PreferredSizeMB: 300},
|
AutoDownloadPrefs{PreferredKbps: 320},
|
||||||
300 * mb, 1.0,
|
kbpsCandidate("c", ".mp3", 320), 1.0,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"double the preferred size scores 0",
|
// The floor is neutral, not zero: this term carries 0.40
|
||||||
AutoDownloadPrefs{PreferredSizeMB: 300},
|
// of the quality score once a preference is set, and a
|
||||||
600 * mb, 0.0,
|
// span to zero would let "I like 320" quietly disqualify
|
||||||
|
// every FLAC from auto-pick.
|
||||||
|
"double the preferred rate falls to the neutral floor",
|
||||||
|
AutoDownloadPrefs{PreferredKbps: 320},
|
||||||
|
kbpsCandidate("c", ".flac", 640), neutral,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
"half the preferred size scores 0",
|
"half the preferred rate falls to the neutral floor",
|
||||||
AutoDownloadPrefs{PreferredSizeMB: 300},
|
AutoDownloadPrefs{PreferredKbps: 320},
|
||||||
150 * mb, 0.0,
|
kbpsCandidate("c", ".mp3", 160), neutral,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"an unknowable rate is neutral",
|
||||||
|
AutoDownloadPrefs{PreferredKbps: 320},
|
||||||
|
kbpsCandidate("c", ".mp3", 320), neutral,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -409,30 +519,223 @@ func TestAutoDownloadPrefsSizeFit(t *testing.T) {
|
|||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
if got := tt.prefs.sizeFit(tt.totalSize); got != tt.want {
|
// The last case deliberately withholds the runtime.
|
||||||
t.Errorf("sizeFit(%d) = %f, want %f", tt.totalSize, got, tt.want)
|
runtime := int64(okComputerRuntime)
|
||||||
|
if tt.name == "an unknowable rate is neutral" {
|
||||||
|
runtime = 0
|
||||||
|
}
|
||||||
|
|
||||||
|
if got := tt.prefs.bitrateFit(tt.c, runtime); got != tt.want {
|
||||||
|
t.Errorf("bitrateFit() = %f, want %f", got, tt.want)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// An otherwise-perfect candidate must not auto-pick when it falls
|
// An otherwise-perfect candidate must not auto-pick when it falls
|
||||||
// outside the configured size guard: the guardrail applies before the
|
// outside the configured guardrails: they apply before the match and
|
||||||
// match/quality/lead checks, not as one more input averaged into them.
|
// quality checks, not as one more input averaged into them.
|
||||||
func TestAutoPickableRejectsCandidateOutsideSizeGuard(t *testing.T) {
|
func TestAutoPickableRejectsCandidateOutsideTheGuardrails(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
dl := okComputer()
|
dl := okComputer()
|
||||||
best := Score(dl, candidateFor("a", allTitles(), ".flac", 30_000_000), 50, AutoDownloadPrefs{})
|
best := Score(dl, kbpsCandidate("a", ".flac", 900), 50, AutoDownloadPrefs{})
|
||||||
best.TotalSize = 500 * mb
|
|
||||||
|
|
||||||
if !AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{}) {
|
if !AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{}) {
|
||||||
t.Fatal("expected this candidate to be auto-pickable with no guardrails")
|
t.Fatal("expected this candidate to be auto-pickable with no guardrails")
|
||||||
}
|
}
|
||||||
|
|
||||||
tight := AutoDownloadPrefs{MinSizeMB: 10, MaxSizeMB: 100}
|
if AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{MaxKbps: 320}) {
|
||||||
|
t.Error("candidate above the bitrate window must not auto-pick")
|
||||||
|
}
|
||||||
|
|
||||||
if AutoPickable(dl, []Candidate{best}, tight) {
|
if AutoPickable(dl, []Candidate{best}, AutoDownloadPrefs{MaxSizeMB: 1}) {
|
||||||
t.Error("candidate outside the size guard must not auto-pick")
|
t.Error("candidate above the size ceiling must not auto-pick")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The refusal has to name the gate that refused.
|
||||||
|
//
|
||||||
|
// Before AutoPickVeto, every one of these came back as the same
|
||||||
|
// sentence built from `ranked[0]` — the best candidate before the size
|
||||||
|
// and format guardrails — so a request refused because the user's size
|
||||||
|
// window excluded every copy reported a match and a quality that both
|
||||||
|
// cleared their thresholds. A refusal quoting numbers that pass is
|
||||||
|
// what made the matcher look broken from outside.
|
||||||
|
func TestAutoPickVetoNamesTheGate(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dl := okComputer()
|
||||||
|
best := Score(
|
||||||
|
dl,
|
||||||
|
candidateFor("a", allTitles(), ".flac", 30_000_000),
|
||||||
|
50,
|
||||||
|
AutoDownloadPrefs{},
|
||||||
|
)
|
||||||
|
|
||||||
|
// candidateFor sizes the files and leaves TotalSize at 0, which is
|
||||||
|
// what the guardrails read.
|
||||||
|
sized := func(c Candidate, total int64) Candidate {
|
||||||
|
c.TotalSize = total
|
||||||
|
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
dl Download
|
||||||
|
ranked []Candidate
|
||||||
|
prefs AutoDownloadPrefs
|
||||||
|
wantSub string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "nothing found",
|
||||||
|
dl: dl,
|
||||||
|
ranked: nil,
|
||||||
|
wantSub: "nothing found",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "free text",
|
||||||
|
dl: Download{Artist: "Radiohead", Album: "OK Computer"},
|
||||||
|
ranked: []Candidate{best},
|
||||||
|
wantSub: "free text",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "no tracklist behind the anchor",
|
||||||
|
dl: Download{
|
||||||
|
ReleaseMBID: "mbid-ok-computer",
|
||||||
|
Artist: "Radiohead",
|
||||||
|
Album: "OK Computer",
|
||||||
|
},
|
||||||
|
ranked: []Candidate{best},
|
||||||
|
wantSub: "no tracklist",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
// The candidate is 120 MB and the window tops out at 1 MB:
|
||||||
|
// the old message reported its match and quality instead.
|
||||||
|
name: "outside the size window",
|
||||||
|
dl: dl,
|
||||||
|
ranked: []Candidate{sized(best, 120<<20)},
|
||||||
|
prefs: AutoDownloadPrefs{MaxSizeMB: 1},
|
||||||
|
wantSub: "bitrate, size or format limits",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
got := AutoPickVeto(tt.dl, tt.ranked, tt.prefs)
|
||||||
|
if !strings.Contains(got, tt.wantSub) {
|
||||||
|
t.Errorf("veto = %q, want it to mention %q", got, tt.wantSub)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A clear winner has no veto at all — the sentence is empty, which is
|
||||||
|
// what AutoPickable reads.
|
||||||
|
func TestAutoPickVetoIsEmptyForAClearWinner(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dl := okComputer()
|
||||||
|
best := Score(
|
||||||
|
dl,
|
||||||
|
candidateFor("a", allTitles(), ".flac", 30_000_000),
|
||||||
|
50,
|
||||||
|
AutoDownloadPrefs{},
|
||||||
|
)
|
||||||
|
weak := Score(
|
||||||
|
dl,
|
||||||
|
candidateFor("b", allTitles()[:2], ".mp3", 1_000_000),
|
||||||
|
50,
|
||||||
|
AutoDownloadPrefs{},
|
||||||
|
)
|
||||||
|
|
||||||
|
if got := AutoPickVeto(dl, []Candidate{best, weak}, AutoDownloadPrefs{}); got != "" {
|
||||||
|
t.Errorf("veto = %q, want none", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// With several candidates that all clear the bar, the preferred
|
||||||
|
// bitrate decides which one is taken.
|
||||||
|
//
|
||||||
|
// This is what replaced the daylight requirement. Auto-pick no longer
|
||||||
|
// refuses when the field is close; it takes the copy nearest the shape
|
||||||
|
// the user asked for, which is the question they actually answered in
|
||||||
|
// Settings.
|
||||||
|
func TestPreferredBitrateBreaksTheTie(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dl := okComputer()
|
||||||
|
prefs := AutoDownloadPrefs{PreferredKbps: 320}
|
||||||
|
|
||||||
|
// Same album, same completeness, same health, same provider — the
|
||||||
|
// only difference between them is the rate.
|
||||||
|
lossless := kbpsCandidate("lossless", ".flac", 900)
|
||||||
|
perfect := kbpsCandidate("perfect", ".mp3", 320)
|
||||||
|
|
||||||
|
ranked := Rank(
|
||||||
|
dl, []Candidate{lossless, perfect}, nil, prefs,
|
||||||
|
)
|
||||||
|
|
||||||
|
if ranked[0].ID != "perfect" {
|
||||||
|
t.Errorf(
|
||||||
|
"winner = %q (fit %f) over %q (fit %f), want the 320 kbps copy",
|
||||||
|
ranked[0].ID, ranked[0].Quality.BitrateFit,
|
||||||
|
ranked[1].ID, ranked[1].Quality.BitrateFit,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if AutoPickVeto(dl, ranked, prefs) != "" {
|
||||||
|
t.Error("a close field must still auto-pick")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// With no preference set, nothing changes: BitrateFit is the same
|
||||||
|
// neutral value for every candidate and the older tie-breaks decide.
|
||||||
|
func TestNoPreferredBitrateLeavesRankingAlone(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dl := okComputer()
|
||||||
|
|
||||||
|
lossless := kbpsCandidate("lossless", ".flac", 900)
|
||||||
|
lossy := kbpsCandidate("lossy", ".mp3", 320)
|
||||||
|
|
||||||
|
ranked := Rank(
|
||||||
|
dl, []Candidate{lossy, lossless}, nil, AutoDownloadPrefs{},
|
||||||
|
)
|
||||||
|
|
||||||
|
if ranked[0].ID != "lossless" {
|
||||||
|
t.Errorf(
|
||||||
|
"winner = %q, want the lossless copy on format alone",
|
||||||
|
ranked[0].ID,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A preferred bitrate promotes the copy that matches it and must never
|
||||||
|
// disqualify the ones that do not. It carries 0.40 of the quality
|
||||||
|
// score, so a fit spanning down to zero would put a perfectly good FLAC
|
||||||
|
// under minQuality and out of auto-pick — turning a preference into a
|
||||||
|
// prohibition without saying so. MinKbps and MaxKbps are how a user
|
||||||
|
// says that on purpose.
|
||||||
|
func TestAPreferredBitrateNeverDisqualifies(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dl := okComputer()
|
||||||
|
far := AutoDownloadPrefs{PreferredKbps: 128}
|
||||||
|
|
||||||
|
lossless := Score(dl, kbpsCandidate("flac", ".flac", 900), 50, far)
|
||||||
|
|
||||||
|
if lossless.Quality.Overall < minQuality {
|
||||||
|
t.Errorf(
|
||||||
|
"quality = %f under a far-off preference, want >= %f",
|
||||||
|
lossless.Quality.Overall, minQuality,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
if veto := AutoPickVeto(dl, []Candidate{lossless}, far); veto != "" {
|
||||||
|
t.Errorf("a far-off preference vetoed the candidate: %s", veto)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -36,13 +36,24 @@ func newServiceFixture(t *testing.T) serviceFixture {
|
|||||||
// assertion read it; the second is that same goroutine still writing
|
// assertion read it; the second is that same goroutine still writing
|
||||||
// into `t.TempDir()` after the test returned. One cause, two shapes.
|
// into `t.TempDir()` after the test returned. One cause, two shapes.
|
||||||
//
|
//
|
||||||
// Putting the candidate outside the auto-pick size window stops the
|
// Putting the candidate outside the auto-pick guardrails stops the
|
||||||
// grab from ever starting, which is better than waiting for it: there
|
// grab from ever starting, which is better than waiting for it: there
|
||||||
// is no goroutine to be slow, so the tests state what they mean
|
// is no goroutine to be slow, so the tests state what they mean
|
||||||
// ("the request exists, in this state") without a timing assumption
|
// ("the request exists, in this state") without a timing assumption
|
||||||
// underneath. A test that does want the download has `managerFixture`
|
// underneath. A test that does want the download has `managerFixture`
|
||||||
// and sets its own preferences.
|
// and sets its own preferences.
|
||||||
mf.manager.SetPreferences(AutoDownloadPrefs{MaxSizeMB: 1})
|
//
|
||||||
|
// The guard is a *format* the fake never produces, and it used to be
|
||||||
|
// `MaxSizeMB: 1`, which never fired: the size gates read
|
||||||
|
// `Candidate.TotalSize`, which real providers fill and the fake
|
||||||
|
// leaves at zero, and zero is under every ceiling. So the grab went
|
||||||
|
// ahead anyway and the second failure shape above — the TempDir
|
||||||
|
// cleanup race — kept happening, reproducibly, roughly one run in
|
||||||
|
// fifteen. A guard has to be keyed on something the fixture
|
||||||
|
// actually sets.
|
||||||
|
mf.manager.SetPreferences(AutoDownloadPrefs{
|
||||||
|
AllowedFormats: []Format{FormatWMA},
|
||||||
|
})
|
||||||
|
|
||||||
return serviceFixture{managerFixture: mf, svc: svc}
|
return serviceFixture{managerFixture: mf, svc: svc}
|
||||||
}
|
}
|
||||||
@@ -182,6 +193,11 @@ func TestManualDownloadSatisfiesRequestOnSuccess(t *testing.T) {
|
|||||||
f := newServiceFixture(t)
|
f := newServiceFixture(t)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// This is the one test here that is *about* the download, so it
|
||||||
|
// undoes the fixture's guard rather than relying on it — which is
|
||||||
|
// what it was doing implicitly while the guard did not work.
|
||||||
|
f.manager.SetPreferences(AutoDownloadPrefs{})
|
||||||
|
|
||||||
provider := fakeWithAlbum(1, "source", ".flac")
|
provider := fakeWithAlbum(1, "source", ".flac")
|
||||||
f.manager.installProvider(Config{ID: 1, Priority: 50}, provider)
|
f.manager.installProvider(Config{ID: 1, Priority: 50}, provider)
|
||||||
|
|
||||||
|
|||||||
@@ -302,7 +302,12 @@ type QualityScore struct {
|
|||||||
Bitrate float64 `json:"bitrate"`
|
Bitrate float64 `json:"bitrate"`
|
||||||
Health float64 `json:"health"` // seeders, free slots
|
Health float64 `json:"health"` // seeders, free slots
|
||||||
Priority float64 `json:"priority"` // user's per-provider preference
|
Priority float64 `json:"priority"` // user's per-provider preference
|
||||||
SizeFit float64 `json:"sizeFit"` // closeness to the preferred download size
|
// BitrateFit is closeness to the preferred *rate*, which is what
|
||||||
|
// the auto-download window is expressed in. It replaced a
|
||||||
|
// `SizeFit` measured in megabytes: a size means nothing without
|
||||||
|
// knowing how long the music is, so the same number described a
|
||||||
|
// generous single and a suspiciously small boxset.
|
||||||
|
BitrateFit float64 `json:"bitrateFit"`
|
||||||
|
|
||||||
// Mixed marks a candidate whose files are not all the same format,
|
// Mixed marks a candidate whose files are not all the same format,
|
||||||
// which usually means a hand-assembled folder rather than a rip.
|
// which usually means a hand-assembled folder rather than a rip.
|
||||||
|
|||||||
@@ -25,8 +25,26 @@ const (
|
|||||||
// where cached cover art thumbnails are stored.
|
// where cached cover art thumbnails are stored.
|
||||||
thumbnailDir = CoverArtCacheDirName
|
thumbnailDir = CoverArtCacheDirName
|
||||||
|
|
||||||
// thumbnailTimeout is the HTTP timeout for fetching a thumbnail.
|
// thumbnailTimeout is the HTTP timeout for fetching a thumbnail,
|
||||||
thumbnailTimeout = 10 * time.Second
|
// and it has to cover a redirect the Cover Art Archive does not
|
||||||
|
// serve itself.
|
||||||
|
//
|
||||||
|
// `coverartarchive.org` answers `front-250` with a 307 to an
|
||||||
|
// Internet Archive storage node (`dn######.us.archive.org`), and
|
||||||
|
// those nodes are routinely slow: measured against the twelve
|
||||||
|
// albums on Explore's own shelves, a successful fetch took 14–16 s
|
||||||
|
// and a failing one 13–17 s. At 10 s *every* cover on the page
|
||||||
|
// timed out — 24 cards, 5 of which had art, all of those from the
|
||||||
|
// disk cache — which reads as "Explore has no album art" rather
|
||||||
|
// than as a slow upstream, because a timeout writes nothing and
|
||||||
|
// says nothing.
|
||||||
|
//
|
||||||
|
// 30 s is chosen to clear that measured range with room, not to be
|
||||||
|
// generous: the fetch is off the critical path (each one is its own
|
||||||
|
// goroutine behind an 8/s limiter, and the frontend renders a
|
||||||
|
// placeholder until it lands), so the cost of waiting is nothing
|
||||||
|
// and the cost of giving up early is a blank page.
|
||||||
|
thumbnailTimeout = 30 * time.Second
|
||||||
|
|
||||||
// thumbnailMaxSize is the maximum image size to cache (2 MB).
|
// thumbnailMaxSize is the maximum image size to cache (2 MB).
|
||||||
thumbnailMaxSize = 2 * 1024 * 1024
|
thumbnailMaxSize = 2 * 1024 * 1024
|
||||||
@@ -97,6 +115,20 @@ func (p *CoverArtProxy) GetThumbnail(
|
|||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A 404 is an answer, and it is already on disk.
|
||||||
|
//
|
||||||
|
// `writeCache(mbid, nil)` has recorded "the archive has no art for
|
||||||
|
// this" as an empty file since this was written, and nothing has
|
||||||
|
// ever read it back: `readCache` returns "" for an empty file,
|
||||||
|
// which is indistinguishable from a miss, so every art-less release
|
||||||
|
// group was re-fetched from the network on every render that asked
|
||||||
|
// about it. On Explore's shelves a third of the cards are art-less,
|
||||||
|
// so that was a third of the page spending a live CAA request to be
|
||||||
|
// told again what the last one said.
|
||||||
|
if p.knownMissing(releaseGroupMBID) {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
|
||||||
// Source 3: fetch from Cover Art Archive (slow, cached to disk).
|
// Source 3: fetch from Cover Art Archive (slow, cached to disk).
|
||||||
url := CoverArtGroupURL(releaseGroupMBID)
|
url := CoverArtGroupURL(releaseGroupMBID)
|
||||||
data, cacheable, err := p.fetch(url)
|
data, cacheable, err := p.fetch(url)
|
||||||
@@ -177,8 +209,9 @@ func (p *CoverArtProxy) GetCandidateThumbnail(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Network fetch on release group.
|
// Network fetch on release group — unless a previous one was told
|
||||||
if releaseGroupMBID != "" {
|
// there is none. See `knownMissing`.
|
||||||
|
if releaseGroupMBID != "" && !p.knownMissing(releaseGroupMBID) {
|
||||||
url := CoverArtGroupURL(releaseGroupMBID)
|
url := CoverArtGroupURL(releaseGroupMBID)
|
||||||
data, cacheable, err := p.fetch(url)
|
data, cacheable, err := p.fetch(url)
|
||||||
|
|
||||||
@@ -194,7 +227,7 @@ func (p *CoverArtProxy) GetCandidateThumbnail(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Network fetch on release (fallback).
|
// Network fetch on release (fallback).
|
||||||
if releaseMBID != "" {
|
if releaseMBID != "" && !p.knownMissing(releaseMBID) {
|
||||||
url := CoverArtURL(releaseMBID)
|
url := CoverArtURL(releaseMBID)
|
||||||
data, cacheable, err := p.fetch(url)
|
data, cacheable, err := p.fetch(url)
|
||||||
|
|
||||||
@@ -285,6 +318,17 @@ func (p *CoverArtProxy) cachePath(mbid string) string {
|
|||||||
return filepath.Join(p.cacheDir, mbid+".jpg")
|
return filepath.Join(p.cacheDir, mbid+".jpg")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// knownMissing reports whether a previous fetch was told the archive
|
||||||
|
// has no art for this MBID — the empty file `writeCache(mbid, nil)`
|
||||||
|
// leaves behind. It is deliberately separate from `readCache`, which
|
||||||
|
// answers "what are the bytes" and cannot express the difference
|
||||||
|
// between no answer and an answer of none.
|
||||||
|
func (p *CoverArtProxy) knownMissing(mbid string) bool {
|
||||||
|
info, err := os.Stat(p.cachePath(mbid))
|
||||||
|
|
||||||
|
return err == nil && info.Size() == 0
|
||||||
|
}
|
||||||
|
|
||||||
func (p *CoverArtProxy) readCache(mbid string) string {
|
func (p *CoverArtProxy) readCache(mbid string) string {
|
||||||
path := p.cachePath(mbid)
|
path := p.cachePath(mbid)
|
||||||
|
|
||||||
|
|||||||
+141
-15
@@ -289,8 +289,13 @@ func (l *Library) scanInternal(
|
|||||||
l.mu.Unlock()
|
l.mu.Unlock()
|
||||||
}()
|
}()
|
||||||
|
|
||||||
|
// The configured mode, not a hardcoded "auto". `ScanConcurrency`
|
||||||
|
// has been a validated config field with three values and one
|
||||||
|
// caller passing a constant, so choosing `ssd` or `hdd` by hand
|
||||||
|
// did nothing at all.
|
||||||
|
diskProfile := system.ProfileForPath(libraryPath)
|
||||||
workerCount := resolveScanWorkerCount(
|
workerCount := resolveScanWorkerCount(
|
||||||
ScanConcurrencyAuto,
|
l.conf.ScanConcurrency,
|
||||||
libraryPath,
|
libraryPath,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -300,6 +305,10 @@ func (l *Library) scanInternal(
|
|||||||
"libraryName", libraryName,
|
"libraryName", libraryName,
|
||||||
"libraryPath", libraryPath,
|
"libraryPath", libraryPath,
|
||||||
"workers", workerCount,
|
"workers", workerCount,
|
||||||
|
"mode", l.conf.ScanConcurrency,
|
||||||
|
"device", diskProfile.Device,
|
||||||
|
"rotational", diskProfile.Rotational,
|
||||||
|
"queueDepth", diskProfile.QueueDepth,
|
||||||
)
|
)
|
||||||
|
|
||||||
// Helper to build a ScanProgress with library identification.
|
// Helper to build a ScanProgress with library identification.
|
||||||
@@ -818,7 +827,7 @@ func (l *Library) scanInternal(
|
|||||||
g := new(errgroup.Group)
|
g := new(errgroup.Group)
|
||||||
g.SetLimit(workerCount)
|
g.SetLimit(workerCount)
|
||||||
|
|
||||||
for work := range workChan {
|
for work := range readaheadWork(scanCtx, workChan, diskProfile) {
|
||||||
g.Go(func() error {
|
g.Go(func() error {
|
||||||
if err := l.waitIfPaused(scanCtx); err != nil {
|
if err := l.waitIfPaused(scanCtx); err != nil {
|
||||||
return err
|
return err
|
||||||
@@ -1285,9 +1294,101 @@ func surveyAudioFiles(
|
|||||||
return count, maxModTime
|
return count, maxModTime
|
||||||
}
|
}
|
||||||
|
|
||||||
// hddWorkerCount is the maximum number of concurrent extraction
|
// How many extraction workers a spinning disk gets, and why it is two
|
||||||
// workers when the library resides on a spinning disk.
|
// numbers rather than one.
|
||||||
const hddWorkerCount = 2
|
//
|
||||||
|
// Extraction is not CPU work — every parser here reads headers and
|
||||||
|
// returns — so on a spinning disk the whole cost is seek latency, and
|
||||||
|
// the only question worth asking is how many reads should be in flight
|
||||||
|
// at once. That has two different right answers and the drive says
|
||||||
|
// which:
|
||||||
|
//
|
||||||
|
// - A drive with command queueing (NCQ: /sys/block/<dev>/device/
|
||||||
|
// queue_depth reports 31 or 32 on any SATA disk with it enabled)
|
||||||
|
// reorders outstanding reads into the order its head passes over
|
||||||
|
// them. Handing it several at once is most of why a parallel scan
|
||||||
|
// beats a serial one at all, and four is where the returns flatten:
|
||||||
|
// the drive needs a few requests to have anything to reorder, and
|
||||||
|
// past that it is queueing requests it was already going to
|
||||||
|
// service in that order.
|
||||||
|
// - A drive without it — queue_depth 1, which is what a USB bridge
|
||||||
|
// or a pre-2004 disk reports — services one command at a time in
|
||||||
|
// the order given. Every extra worker there is one more seek
|
||||||
|
// competing for one head, and the scan gets *slower* the harder it
|
||||||
|
// is pushed. Two is kept rather than one because the readahead
|
||||||
|
// hints (see readaheadWork) do the overlapping that concurrency
|
||||||
|
// was standing in for, and one worker cannot hide a stall.
|
||||||
|
//
|
||||||
|
// This used to be a flat 2 for anything rotational, which is a
|
||||||
|
// pre-NCQ assumption: it left a modern spinning disk with a quarter of
|
||||||
|
// the queue depth it can use.
|
||||||
|
const (
|
||||||
|
hddWorkerCountQueued = 4
|
||||||
|
hddWorkerCountSerial = 2
|
||||||
|
)
|
||||||
|
|
||||||
|
// Readahead tuning.
|
||||||
|
const (
|
||||||
|
// readaheadDepth is how many files ahead of the workers the
|
||||||
|
// prefetcher runs. It is the channel's buffer, so it is also the
|
||||||
|
// number of `WILLNEED` hints outstanding at once — comfortably more
|
||||||
|
// than a queueing drive's 32-command window is worth filling with
|
||||||
|
// one library, and small enough that a cancelled scan is not
|
||||||
|
// holding a long tail of queued reads.
|
||||||
|
readaheadDepth = 16
|
||||||
|
|
||||||
|
// readaheadBytes is how much of each file to pull in. Everything
|
||||||
|
// the scanner reads lives at the head: ID3v2 and FLAC's
|
||||||
|
// STREAMINFO/VORBIS_COMMENT/PICTURE blocks, and the first MPEG
|
||||||
|
// frame with its Xing header. 512 KB covers a tag carrying
|
||||||
|
// embedded cover art, which is the large case — and reading a
|
||||||
|
// little too much sequentially costs a spinning disk almost
|
||||||
|
// nothing next to the seek that got there.
|
||||||
|
readaheadBytes = 512 << 10
|
||||||
|
)
|
||||||
|
|
||||||
|
// readaheadWork forwards scan work while asking the kernel to fetch
|
||||||
|
// each file's header before a worker reaches it.
|
||||||
|
//
|
||||||
|
// The buffered channel *is* the lookahead: this goroutine runs ahead
|
||||||
|
// of the workers until the buffer fills, hinting every file as it goes,
|
||||||
|
// so by the time a worker takes an item the read it needs has been in
|
||||||
|
// flight for `readaheadDepth` files' worth of parsing. That is the
|
||||||
|
// only thing that helps a spinning disk here, because the per-file work
|
||||||
|
// is already header-only — every parser in `backend/metadata` reads a
|
||||||
|
// few hundred bytes and returns, so the scan is not waiting on CPU or
|
||||||
|
// on bytes, it is waiting on the head to arrive.
|
||||||
|
//
|
||||||
|
// It runs on rotational disks only. An SSD has no seek to hide and
|
||||||
|
// already has one worker per core; issuing hints there is pure syscall
|
||||||
|
// overhead against an OS readahead that is already ahead of us.
|
||||||
|
func readaheadWork(
|
||||||
|
ctx context.Context,
|
||||||
|
in <-chan scanWork,
|
||||||
|
profile system.DiskProfile,
|
||||||
|
) <-chan scanWork {
|
||||||
|
if !profile.Rotational {
|
||||||
|
return in
|
||||||
|
}
|
||||||
|
|
||||||
|
out := make(chan scanWork, readaheadDepth)
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
defer close(out)
|
||||||
|
|
||||||
|
for work := range in {
|
||||||
|
hintReadahead(work.absolutePath, readaheadBytes)
|
||||||
|
|
||||||
|
select {
|
||||||
|
case out <- work:
|
||||||
|
case <-ctx.Done():
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}()
|
||||||
|
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
// resolveScanWorkerCount returns the number of concurrent
|
// resolveScanWorkerCount returns the number of concurrent
|
||||||
// extraction workers based on the configured concurrency mode
|
// extraction workers based on the configured concurrency mode
|
||||||
@@ -1296,20 +1397,45 @@ func resolveScanWorkerCount(
|
|||||||
mode ScanConcurrency,
|
mode ScanConcurrency,
|
||||||
libraryPath string,
|
libraryPath string,
|
||||||
) int {
|
) int {
|
||||||
|
return workersForProfile(
|
||||||
|
mode,
|
||||||
|
system.ProfileForPath(libraryPath),
|
||||||
|
goruntime.NumCPU(),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// workersForProfile is the policy on its own, so it can be tested
|
||||||
|
// against drives this machine does not have.
|
||||||
|
//
|
||||||
|
// `hdd` and `ssd` override what the device says rather than being a
|
||||||
|
// separate branch: the mode is the user overruling detection, and
|
||||||
|
// detection is right about the queue depth either way — a user who
|
||||||
|
// picks `hdd` on a queueing drive still wants that drive's queue used.
|
||||||
|
func workersForProfile(
|
||||||
|
mode ScanConcurrency,
|
||||||
|
profile system.DiskProfile,
|
||||||
|
cpus int,
|
||||||
|
) int {
|
||||||
|
spinning := profile.Rotational
|
||||||
|
|
||||||
switch mode {
|
switch mode {
|
||||||
case ScanConcurrencySSD:
|
case ScanConcurrencySSD:
|
||||||
return goruntime.NumCPU()
|
spinning = false
|
||||||
case ScanConcurrencyHDD:
|
case ScanConcurrencyHDD:
|
||||||
return min(hddWorkerCount, goruntime.NumCPU())
|
spinning = true
|
||||||
default: // auto
|
case ScanConcurrencyAuto:
|
||||||
if system.IsRotationalDisk(libraryPath) {
|
|
||||||
return min(
|
|
||||||
hddWorkerCount, goruntime.NumCPU(),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
return goruntime.NumCPU()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !spinning {
|
||||||
|
return cpus
|
||||||
|
}
|
||||||
|
|
||||||
|
workers := hddWorkerCountSerial
|
||||||
|
if profile.Queues() {
|
||||||
|
workers = hddWorkerCountQueued
|
||||||
|
}
|
||||||
|
|
||||||
|
return min(workers, cpus)
|
||||||
}
|
}
|
||||||
|
|
||||||
// scanWork represents a file to be processed by a worker.
|
// scanWork represents a file to be processed by a worker.
|
||||||
|
|||||||
@@ -0,0 +1,38 @@
|
|||||||
|
//go:build linux
|
||||||
|
|
||||||
|
package library
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
|
||||||
|
"golang.org/x/sys/unix"
|
||||||
|
)
|
||||||
|
|
||||||
|
// hintReadahead asks the kernel to start fetching the head of a file
|
||||||
|
// that is about to be read.
|
||||||
|
//
|
||||||
|
// `POSIX_FADV_WILLNEED` returns immediately and queues the read, which
|
||||||
|
// is the whole point: on a spinning disk the first access to a file
|
||||||
|
// costs a seek of several milliseconds, and that latency can only be
|
||||||
|
// hidden by having the next seek already in flight while the current
|
||||||
|
// file is being parsed. A drive with command queueing can then service
|
||||||
|
// the queued reads in head order rather than in the order they were
|
||||||
|
// asked for.
|
||||||
|
//
|
||||||
|
// Errors are dropped on purpose. This is a hint: a file that has since
|
||||||
|
// been deleted, a filesystem that does not implement fadvise, or a
|
||||||
|
// permission the walk saw and this open does not, all mean "no
|
||||||
|
// prefetch", never "fail the scan". The read that follows is what
|
||||||
|
// reports a genuine problem.
|
||||||
|
func hintReadahead(path string, bytes int64) {
|
||||||
|
f, err := os.Open(path)
|
||||||
|
if err != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
defer func() { _ = f.Close() }()
|
||||||
|
|
||||||
|
_ = unix.Fadvise(
|
||||||
|
int(f.Fd()), 0, bytes, unix.FADV_WILLNEED,
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -0,0 +1,13 @@
|
|||||||
|
//go:build !linux
|
||||||
|
|
||||||
|
package library
|
||||||
|
|
||||||
|
// hintReadahead is a no-op off Linux.
|
||||||
|
//
|
||||||
|
// macOS has `F_RDADVISE` and Windows has `FILE_FLAG_SEQUENTIAL_SCAN`,
|
||||||
|
// and neither is wired up here for the reason the scan concurrency
|
||||||
|
// heuristic is not either: this package cannot tell a spinning disk
|
||||||
|
// from an SSD on those platforms (see system.ProfileForPath), so it
|
||||||
|
// would be prefetching without knowing whether prefetching is what the
|
||||||
|
// device wants.
|
||||||
|
func hintReadahead(_ string, _ int64) {}
|
||||||
@@ -0,0 +1,122 @@
|
|||||||
|
package library
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"yellowjacket/backend/system"
|
||||||
|
)
|
||||||
|
|
||||||
|
// How many workers a scan gets is decided by two facts about the
|
||||||
|
// device, and the second one is new: a spinning disk that can queue
|
||||||
|
// commands wants several reads in flight, and one that cannot wants
|
||||||
|
// almost none. Before this it was a flat 2 for anything rotational,
|
||||||
|
// which is a pre-NCQ assumption — a modern SATA disk reports a queue
|
||||||
|
// depth of 32 and was being given a quarter of what it can use.
|
||||||
|
func TestWorkersForProfile(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const cpus = 16
|
||||||
|
|
||||||
|
ssd := system.DiskProfile{Device: "sda", QueueDepth: 32}
|
||||||
|
hddQueued := system.DiskProfile{
|
||||||
|
Device: "sdb", Rotational: true, QueueDepth: 32,
|
||||||
|
}
|
||||||
|
hddSerial := system.DiskProfile{
|
||||||
|
Device: "sdc", Rotational: true, QueueDepth: 1,
|
||||||
|
}
|
||||||
|
// Neither NVMe nor a device-mapper volume publishes queue_depth.
|
||||||
|
// An unknown depth must not be read as "cannot queue", or every
|
||||||
|
// such device would be scanned as if it were a 2003 drive.
|
||||||
|
unknown := system.DiskProfile{Device: "dm-0", Rotational: true}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
mode ScanConcurrency
|
||||||
|
profile system.DiskProfile
|
||||||
|
want int
|
||||||
|
}{
|
||||||
|
{"ssd auto", ScanConcurrencyAuto, ssd, cpus},
|
||||||
|
{"queueing hdd auto", ScanConcurrencyAuto, hddQueued, hddWorkerCountQueued},
|
||||||
|
{"serial hdd auto", ScanConcurrencyAuto, hddSerial, hddWorkerCountSerial},
|
||||||
|
{"unknown depth queues", ScanConcurrencyAuto, unknown, hddWorkerCountQueued},
|
||||||
|
|
||||||
|
// The mode overrules detection about the *disk*, never about
|
||||||
|
// its queue: forcing hdd on a queueing drive still uses it.
|
||||||
|
{"forced hdd on an ssd", ScanConcurrencyHDD, ssd, hddWorkerCountQueued},
|
||||||
|
{"forced ssd on an hdd", ScanConcurrencySSD, hddQueued, cpus},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
if got := workersForProfile(tt.mode, tt.profile, cpus); got != tt.want {
|
||||||
|
t.Errorf(
|
||||||
|
"workersForProfile(%q, %+v) = %d, want %d",
|
||||||
|
tt.mode, tt.profile, got, tt.want,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A machine with fewer cores than the policy asks for gets its cores.
|
||||||
|
func TestWorkersNeverExceedTheCPUCount(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
hdd := system.DiskProfile{Rotational: true, QueueDepth: 32}
|
||||||
|
|
||||||
|
if got := workersForProfile(ScanConcurrencyAuto, hdd, 1); got != 1 {
|
||||||
|
t.Errorf("single-core hdd = %d workers, want 1", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The prefetch stage must forward every item and nothing else: it is a
|
||||||
|
// pass-through with a side effect, and a scan that drops a file because
|
||||||
|
// of a *hint* would be a spectacular way to lose part of a library.
|
||||||
|
func TestReadaheadForwardsEveryFile(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
in := make(chan scanWork, 4)
|
||||||
|
for _, p := range []string{"/a", "/b", "/c", "/d"} {
|
||||||
|
in <- scanWork{absolutePath: p}
|
||||||
|
}
|
||||||
|
|
||||||
|
close(in)
|
||||||
|
|
||||||
|
var got []string
|
||||||
|
for w := range readaheadWork(
|
||||||
|
context.Background(),
|
||||||
|
in,
|
||||||
|
system.DiskProfile{Rotational: true, QueueDepth: 32},
|
||||||
|
) {
|
||||||
|
got = append(got, w.absolutePath)
|
||||||
|
}
|
||||||
|
|
||||||
|
want := []string{"/a", "/b", "/c", "/d"}
|
||||||
|
if len(got) != len(want) {
|
||||||
|
t.Fatalf("forwarded %v, want %v", got, want)
|
||||||
|
}
|
||||||
|
|
||||||
|
for i := range want {
|
||||||
|
if got[i] != want[i] {
|
||||||
|
t.Errorf("item %d = %q, want %q", i, got[i], want[i])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// On an SSD the stage is not inserted at all — the channel comes back
|
||||||
|
// unchanged, so a scan there pays nothing for a feature it cannot use.
|
||||||
|
func TestReadaheadIsSkippedOnSolidState(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
in := make(chan scanWork)
|
||||||
|
out := readaheadWork(
|
||||||
|
context.Background(), in, system.DiskProfile{QueueDepth: 32},
|
||||||
|
)
|
||||||
|
|
||||||
|
if out != (<-chan scanWork)(in) {
|
||||||
|
t.Error("an ssd must get the original channel, unwrapped")
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -16,32 +16,107 @@ var errNoBlockDevice = errors.New(
|
|||||||
"no matching block device found",
|
"no matching block device found",
|
||||||
)
|
)
|
||||||
|
|
||||||
// IsRotationalDisk reports whether the block device backing the
|
// DiskProfile is what the scanner needs to know about the device a
|
||||||
// given path is a rotational (spinning) disk. Detection uses the
|
// library sits on. Both fields are about the same question — how many
|
||||||
// Linux sysfs interface at /sys/block/<dev>/queue/rotational.
|
// reads should be in flight at once — and they answer different halves
|
||||||
// Returns false on any error (assumes SSD).
|
// of it, so they travel together rather than as two probes.
|
||||||
func IsRotationalDisk(path string) bool {
|
type DiskProfile struct {
|
||||||
dev, err := deviceForPath(path)
|
// Device is the whole-disk kernel name ("sdb"), or "" when the
|
||||||
if err != nil {
|
// path could not be resolved to one.
|
||||||
return false
|
Device string
|
||||||
}
|
|
||||||
|
|
||||||
rotational, err := os.ReadFile(
|
// Rotational is /sys/block/<dev>/queue/rotational: true for a
|
||||||
filepath.Join(
|
// spinning disk, where a seek costs milliseconds.
|
||||||
"/sys/block", dev, "queue", "rotational",
|
Rotational bool
|
||||||
),
|
|
||||||
)
|
|
||||||
if err != nil {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
|
|
||||||
return strings.TrimSpace(string(rotational)) == "1"
|
// QueueDepth is /sys/block/<dev>/device/queue_depth — how many
|
||||||
|
// commands the drive will accept and reorder at once. This is
|
||||||
|
// NCQ: a SATA disk with it enabled reports 31 or 32, and one
|
||||||
|
// without reports 1. Zero means the file was not there to read,
|
||||||
|
// which is the case for anything that is not a SCSI/SATA device
|
||||||
|
// (NVMe, MMC, device-mapper, loop, a VM's virtio disk).
|
||||||
|
//
|
||||||
|
// It is the difference between concurrency helping and hurting.
|
||||||
|
// With queueing, several outstanding reads let the drive service
|
||||||
|
// them in the order its head passes over them, which is most of
|
||||||
|
// why a parallel scan is faster at all. Without it, every extra
|
||||||
|
// worker is one more seek competing for one head, and the scan
|
||||||
|
// gets slower the harder it is pushed.
|
||||||
|
QueueDepth int
|
||||||
}
|
}
|
||||||
|
|
||||||
// deviceForPath resolves a filesystem path to its underlying block
|
// Queues reports whether the drive can reorder outstanding commands.
|
||||||
// device name (e.g. "sda") by matching the device major:minor
|
//
|
||||||
// from stat(2) against /sys/block/ entries.
|
// An unknown depth (0) counts as queueing: everything that does not
|
||||||
func deviceForPath(path string) (string, error) {
|
// publish this file is a device where concurrency is fine — NVMe has
|
||||||
|
// its own queues, virtio and device-mapper are not the physical layer
|
||||||
|
// at all. The only case worth being careful about is the one that
|
||||||
|
// says so explicitly.
|
||||||
|
func (p DiskProfile) Queues() bool {
|
||||||
|
return p.QueueDepth != 1
|
||||||
|
}
|
||||||
|
|
||||||
|
// IsRotationalDisk reports whether the block device backing the
|
||||||
|
// given path is a rotational (spinning) disk. Returns false on any
|
||||||
|
// error (assumes SSD).
|
||||||
|
func IsRotationalDisk(path string) bool {
|
||||||
|
return ProfileForPath(path).Rotational
|
||||||
|
}
|
||||||
|
|
||||||
|
// ProfileForPath describes the device backing a filesystem path. A
|
||||||
|
// path that cannot be resolved yields the zero profile, which reads as
|
||||||
|
// "not rotational, queueing" — the permissive answer, since assuming a
|
||||||
|
// spinning disk on an SSD would halve a scan for nothing.
|
||||||
|
func ProfileForPath(path string) DiskProfile {
|
||||||
|
dev, err := diskForPath(path)
|
||||||
|
if err != nil {
|
||||||
|
return DiskProfile{}
|
||||||
|
}
|
||||||
|
|
||||||
|
return DiskProfile{
|
||||||
|
Device: dev,
|
||||||
|
Rotational: sysfsInt(dev, "queue", "rotational") == 1,
|
||||||
|
QueueDepth: sysfsInt(dev, "device", "queue_depth"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// sysfsInt reads one small integer out of /sys/block/<dev>/<parts...>,
|
||||||
|
// returning 0 when it is absent or unparseable. Every attribute here
|
||||||
|
// is optional: sysfs layout varies by driver, and a missing file is
|
||||||
|
// "this device does not say", never an error worth propagating.
|
||||||
|
func sysfsInt(dev string, parts ...string) int {
|
||||||
|
p := filepath.Join(
|
||||||
|
append([]string{"/sys/block", dev}, parts...)...,
|
||||||
|
)
|
||||||
|
|
||||||
|
data, err := os.ReadFile(p) //nolint:gosec // sysfs, name from the kernel
|
||||||
|
if err != nil {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
n, err := strconv.Atoi(strings.TrimSpace(string(data)))
|
||||||
|
if err != nil {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
|
||||||
|
return n
|
||||||
|
}
|
||||||
|
|
||||||
|
// diskForPath resolves a filesystem path to the *whole disk* backing
|
||||||
|
// it — "sdb" for a file on "sdb3".
|
||||||
|
//
|
||||||
|
// It goes through /sys/dev/block/<major>:<minor>, which the kernel
|
||||||
|
// maintains as a symlink to the device's own sysfs directory, and then
|
||||||
|
// walks up to the parent when that directory turns out to be a
|
||||||
|
// partition. The previous implementation scanned /sys/block comparing
|
||||||
|
// dev numbers and, failing an exact match, took the first entry whose
|
||||||
|
// *major* agreed — and every SATA disk shares major 8. So a library on
|
||||||
|
// /dev/sdb3 resolved to whatever /sys/block listed first, which is
|
||||||
|
// alphabetical, which is sda. On the machine this was found on that
|
||||||
|
// meant a 6 TB spinning disk was read as the SSD next to it and scanned
|
||||||
|
// with one worker per core. Matching on major alone cannot be right
|
||||||
|
// whenever a machine has two disks, which is the case this exists for.
|
||||||
|
func diskForPath(path string) (string, error) {
|
||||||
var st syscall.Stat_t
|
var st syscall.Stat_t
|
||||||
if err := syscall.Stat(path, &st); err != nil {
|
if err := syscall.Stat(path, &st); err != nil {
|
||||||
return "", fmt.Errorf(
|
return "", fmt.Errorf(
|
||||||
@@ -49,48 +124,50 @@ func deviceForPath(path string) (string, error) {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Extract major and minor device numbers.
|
// Linux packs dev_t as 12 bits of major and 20 of minor, split
|
||||||
major := (st.Dev >> 8) & 0xff
|
// across the word. Masking the low byte of each — which is what
|
||||||
minor := st.Dev & 0xff
|
// this used to do — is right only for the first 256 of either.
|
||||||
|
major := unixMajor(uint64(st.Dev))
|
||||||
|
minor := unixMinor(uint64(st.Dev))
|
||||||
|
|
||||||
// Scan /sys/block/ for a matching device.
|
link := filepath.Join(
|
||||||
entries, err := os.ReadDir("/sys/block")
|
"/sys/dev/block",
|
||||||
|
strconv.FormatUint(major, 10)+":"+
|
||||||
|
strconv.FormatUint(minor, 10),
|
||||||
|
)
|
||||||
|
|
||||||
|
target, err := filepath.EvalSymlinks(link)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return "", fmt.Errorf(
|
return "", fmt.Errorf(
|
||||||
"could not read /sys/block: %w", err,
|
"%w: %s (%w)", errNoBlockDevice, link, err,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
majorStr := strconv.FormatUint(major, 10)
|
// A partition's directory sits inside its disk's, and only the
|
||||||
devStr := majorStr + ":" +
|
// disk carries `queue`. Climb at most one level: sysfs nests a
|
||||||
strconv.FormatUint(minor, 10)
|
// partition exactly one deep under its disk.
|
||||||
|
name := filepath.Base(target)
|
||||||
|
|
||||||
for _, entry := range entries {
|
if _, err := os.Stat(filepath.Join(target, "queue")); err != nil {
|
||||||
devFile := filepath.Join(
|
name = filepath.Base(filepath.Dir(target))
|
||||||
"/sys/block", entry.Name(), "dev",
|
|
||||||
)
|
|
||||||
|
|
||||||
data, err := os.ReadFile(devFile)
|
|
||||||
if err != nil {
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
|
|
||||||
content := strings.TrimSpace(string(data))
|
|
||||||
|
|
||||||
if content == devStr {
|
|
||||||
return entry.Name(), nil
|
|
||||||
}
|
|
||||||
|
|
||||||
// The filesystem might be on a partition (e.g. sda1)
|
|
||||||
// whose parent block device is sda. Check if the
|
|
||||||
// major number matches.
|
|
||||||
parts := strings.SplitN(content, ":", 2)
|
|
||||||
if len(parts) == 2 && parts[0] == majorStr {
|
|
||||||
return entry.Name(), nil
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return "", fmt.Errorf(
|
if name == "" || name == "." || name == string(filepath.Separator) {
|
||||||
"%w for %s", errNoBlockDevice, devStr,
|
return "", fmt.Errorf(
|
||||||
)
|
"%w for %d:%d", errNoBlockDevice, major, minor,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return name, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// unixMajor and unixMinor decode a Linux dev_t. Spelled out rather
|
||||||
|
// than taken from golang.org/x/sys/unix so this file stays readable
|
||||||
|
// beside the encoding it is undoing.
|
||||||
|
func unixMajor(dev uint64) uint64 {
|
||||||
|
return (dev>>8)&0xfff | (dev >> 32 & ^uint64(0xfff))
|
||||||
|
}
|
||||||
|
|
||||||
|
func unixMinor(dev uint64) uint64 {
|
||||||
|
return dev&0xff | (dev >> 12 & ^uint64(0xff))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,9 +2,34 @@
|
|||||||
|
|
||||||
package system
|
package system
|
||||||
|
|
||||||
|
// DiskProfile is what the scanner needs to know about the device a
|
||||||
|
// library sits on. See the Linux implementation for what each field
|
||||||
|
// means; off Linux nothing fills them, because neither macOS nor
|
||||||
|
// Windows publishes an equivalent of sysfs's `rotational` and
|
||||||
|
// `queue_depth` without going through platform APIs this package
|
||||||
|
// deliberately does not link.
|
||||||
|
type DiskProfile struct {
|
||||||
|
Device string
|
||||||
|
Rotational bool
|
||||||
|
QueueDepth int
|
||||||
|
}
|
||||||
|
|
||||||
|
// Queues reports whether the drive can reorder outstanding commands.
|
||||||
|
// Always true here: an unknown depth is the permissive answer, and
|
||||||
|
// assuming otherwise would halve every scan on every Mac.
|
||||||
|
func (p DiskProfile) Queues() bool {
|
||||||
|
return p.QueueDepth != 1
|
||||||
|
}
|
||||||
|
|
||||||
// IsRotationalDisk reports whether the block device backing the
|
// IsRotationalDisk reports whether the block device backing the
|
||||||
// given path is a rotational (spinning) disk. On non-Linux
|
// given path is a rotational (spinning) disk. On non-Linux
|
||||||
// platforms this always returns false (assumes SSD).
|
// platforms this always returns false (assumes SSD).
|
||||||
func IsRotationalDisk(_ string) bool {
|
func IsRotationalDisk(_ string) bool {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ProfileForPath describes the device backing a filesystem path. Off
|
||||||
|
// Linux that is the zero profile, which reads as "an SSD that queues".
|
||||||
|
func ProfileForPath(_ string) DiskProfile {
|
||||||
|
return DiskProfile{}
|
||||||
|
}
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ import { test, expect, callBinding } from '../support/fixtures.js';
|
|||||||
* and produced two: every one of the eight call sites was a two-way
|
* and produced two: every one of the eight call sites was a two-way
|
||||||
* ternary, so an album already on the request list showed a plus and
|
* ternary, so an album already on the request list showed a plus and
|
||||||
* said "is not in your library" — on the same page, forty pixels from a
|
* said "is not in your library" — on the same page, forty pixels from a
|
||||||
* filled button reading "Wanted".
|
* filled button reading "Requested".
|
||||||
*
|
*
|
||||||
* This spec exists at this tier rather than only in the component one
|
* This spec exists at this tier rather than only in the component one
|
||||||
* because of what it drags in with it: reaching the requested state is
|
* because of what it drags in with it: reaching the requested state is
|
||||||
@@ -181,7 +181,7 @@ test.describe('the requested badge', () => {
|
|||||||
const ds = document.querySelector('explore-album-details')
|
const ds = document.querySelector('explore-album-details')
|
||||||
?.shadowRoot;
|
?.shadowRoot;
|
||||||
const btn = [...(ds?.querySelectorAll('wa-button') ?? [])].find(
|
const btn = [...(ds?.querySelectorAll('wa-button') ?? [])].find(
|
||||||
(b) => /Wanted/.test(b.textContent ?? ''),
|
(b) => /Requested/.test(b.textContent ?? ''),
|
||||||
);
|
);
|
||||||
|
|
||||||
return btn?.querySelector('wa-icon')?.getAttribute('name') ?? '';
|
return btn?.querySelector('wa-icon')?.getAttribute('name') ?? '';
|
||||||
|
|||||||
@@ -3,28 +3,52 @@
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* AutoDownloadPrefs gates and scores what AutoPickable may choose
|
* AutoDownloadPrefs gates and scores what AutoPickable may choose
|
||||||
* without asking. Zero values are permissive: no size window and no
|
* without asking. Zero values are permissive: no bitrate window, no
|
||||||
* format restriction.
|
* size ceiling and no format restriction.
|
||||||
|
*
|
||||||
|
* **The window is a rate, not a size.** It used to be three numbers in
|
||||||
|
* megabytes, which cannot mean anything on their own: 300 MB is a
|
||||||
|
* generous FLAC single and a suspiciously small boxset, and the user
|
||||||
|
* setting the number has no idea which release the pipeline will
|
||||||
|
* eventually apply it to. A bitrate is the same statement normalised
|
||||||
|
* by how long the music is, so one number holds across a 9-minute EP
|
||||||
|
* and a 3-hour opera — and it is the unit the thing being described is
|
||||||
|
* actually measured in. The runtime is known for every request
|
||||||
|
* auto-pick can act on (`Download.Expected` carries per-track lengths,
|
||||||
|
* and an anchored request is the only kind that reaches here), so this
|
||||||
|
* costs no extra lookup.
|
||||||
*/
|
*/
|
||||||
export interface AutoDownloadPrefs {
|
export interface AutoDownloadPrefs {
|
||||||
/**
|
/**
|
||||||
* MinSizeMB and MaxSizeMB bound what auto-pick will grab. Zero
|
* MinKbps and MaxKbps bound the average bitrate auto-pick will
|
||||||
* means no bound on that side. A candidate outside the window is
|
* grab. Zero means no bound on that side. A candidate outside the
|
||||||
* filtered out of auto-pick entirely, not merely scored down — a
|
* window is filtered out of auto-pick entirely, not merely scored
|
||||||
* tiny "sampler" torrent or a boxset ten times the expected size is
|
* down — a 96 kbps rip of the right album is not a worse copy the
|
||||||
* usually the wrong thing entirely, not a worse copy of the right
|
* user might accept, it is one they said not to take unattended.
|
||||||
* thing.
|
*
|
||||||
|
* For reference: 320 is the top of MP3, ~500–1000 is FLAC depending
|
||||||
|
* on the material, and anything under ~128 is a transcode.
|
||||||
*/
|
*/
|
||||||
"minSizeMb": number;
|
"minKbps": number;
|
||||||
"maxSizeMb": number;
|
"maxKbps": number;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* PreferredSizeMB nudges the score toward a target size within the
|
* PreferredKbps nudges the score toward a target rate within the
|
||||||
* min/max window (a lossless rip and a heavily-padded lossless rip
|
* window, and breaks the tie when several candidates are equally
|
||||||
* can both pass the window). Zero disables the nudge; sizeFit then
|
* good matches. Zero disables the nudge; bitrateFit then returns a
|
||||||
* returns a neutral value that does not affect ranking.
|
* neutral value that does not affect ranking.
|
||||||
*/
|
*/
|
||||||
"preferredSizeMb": number;
|
"preferredKbps": number;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* MaxSizeMB is a hard ceiling on the whole candidate, and it is
|
||||||
|
* deliberately still a size. It answers a different question from
|
||||||
|
* the window above — not "is this the quality I want" but "is this
|
||||||
|
* going to fill the disk" — and it has to hold even for a candidate
|
||||||
|
* whose bitrate cannot be worked out, which is exactly the shape a
|
||||||
|
* mislabelled boxset arrives in. Zero means no ceiling.
|
||||||
|
*/
|
||||||
|
"maxSizeMb": number;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* AllowedFormats restricts auto-pick to candidates whose audio
|
* AllowedFormats restricts auto-pick to candidates whose audio
|
||||||
@@ -487,9 +511,13 @@ export interface QualityScore {
|
|||||||
"priority": number;
|
"priority": number;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* closeness to the preferred download size
|
* BitrateFit is closeness to the preferred *rate*, which is what
|
||||||
|
* the auto-download window is expressed in. It replaced a
|
||||||
|
* `SizeFit` measured in megabytes: a size means nothing without
|
||||||
|
* knowing how long the music is, so the same number described a
|
||||||
|
* generous single and a suspiciously small boxset.
|
||||||
*/
|
*/
|
||||||
"sizeFit": number;
|
"bitrateFit": number;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Mixed marks a candidate whose files are not all the same format,
|
* Mixed marks a candidate whose files are not all the same format,
|
||||||
|
|||||||
@@ -0,0 +1 @@
|
|||||||
|
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 576 512"><!--! Font Awesome Free 7.3.1 by @fontawesome - https://fontawesome.com License - https://fontawesome.com/license/free (Icons: CC BY 4.0, Fonts: SIL OFL 1.1, Code: MIT License) Copyright 2026 Fonticons, Inc. --><path fill="currentColor" d="M288.1-32c9 0 17.3 5.1 21.4 13.1L383 125.3 542.9 150.7c8.9 1.4 16.3 7.7 19.1 16.3s.5 18-5.8 24.4L441.7 305.9 467 465.8c1.4 8.9-2.3 17.9-9.6 23.2s-17 6.1-25 2L288.1 417.6 143.8 491c-8 4.1-17.7 3.3-25-2s-11-14.2-9.6-23.2L134.4 305.9 20 191.4c-6.4-6.4-8.6-15.8-5.8-24.4s10.1-14.9 19.1-16.3l159.9-25.4 73.6-144.2c4.1-8 12.4-13.1 21.4-13.1zm0 76.8L230.3 158c-3.5 6.8-10 11.6-17.6 12.8l-125.5 20 89.8 89.9c5.4 5.4 7.9 13.1 6.7 20.7l-19.8 125.5 113.3-57.6c6.8-3.5 14.9-3.5 21.8 0l113.3 57.6-19.8-125.5c-1.2-7.6 1.3-15.3 6.7-20.7l89.8-89.9-125.5-20c-7.6-1.2-14.1-6-17.6-12.8L288.1 44.8z"/></svg>
|
||||||
|
After Width: | Height: | Size: 889 B |
@@ -10,6 +10,7 @@ import type {
|
|||||||
VisibilityChangedEvent,
|
VisibilityChangedEvent,
|
||||||
} from '@lit-labs/virtualizer';
|
} from '@lit-labs/virtualizer';
|
||||||
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
|
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
|
||||||
|
import { gridSpacingFor } from '@utils/grid-spacing';
|
||||||
import {
|
import {
|
||||||
GetAlbumsByArtist,
|
GetAlbumsByArtist,
|
||||||
GetFilePathsByAlbums,
|
GetFilePathsByAlbums,
|
||||||
@@ -147,8 +148,6 @@ export class ArtistsView
|
|||||||
|
|
||||||
// ----- Grid spacing constants -----
|
// ----- Grid spacing constants -----
|
||||||
|
|
||||||
private static readonly GRID_GAP = 8;
|
|
||||||
private static readonly GRID_PADDING = 8;
|
|
||||||
private static readonly CARD_PADDING = 5;
|
private static readonly CARD_PADDING = 5;
|
||||||
|
|
||||||
private get imageSize(): number {
|
private get imageSize(): number {
|
||||||
@@ -177,20 +176,41 @@ export class ArtistsView
|
|||||||
private createGridLayout() {
|
private createGridLayout() {
|
||||||
const w = this.cardSize ?? CARD_SIZE_DEFAULT;
|
const w = this.cardSize ?? CARD_SIZE_DEFAULT;
|
||||||
const h = w + this.cardTextHeight;
|
const h = w + this.cardTextHeight;
|
||||||
const gap = ArtistsView.GRID_GAP;
|
|
||||||
const pad = ArtistsView.GRID_PADDING;
|
// One number for the gap, the row gap and the padding: whatever
|
||||||
|
// a row could not spend on another card, shared out equally, so
|
||||||
|
// the outside is never wider than the inside. See
|
||||||
|
// `utils/grid-spacing.ts`.
|
||||||
|
const spacing = this.spacingFor(this.containerWidth);
|
||||||
|
|
||||||
|
this.lastLayoutSpacing = spacing;
|
||||||
|
|
||||||
return grid({
|
return grid({
|
||||||
itemSize: {
|
itemSize: {
|
||||||
width: `${w}px`,
|
width: `${w}px`,
|
||||||
height: `${h}px`,
|
height: `${h}px`,
|
||||||
},
|
},
|
||||||
gap: `${gap}px`,
|
gap: `${spacing}px`,
|
||||||
padding: `${pad}px`,
|
padding: `${spacing}px`,
|
||||||
justify: 'center',
|
justify: 'start',
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The width the grid lays itself out in. */
|
||||||
|
private get containerWidth(): number {
|
||||||
|
return (
|
||||||
|
this.renderRoot?.querySelector<HTMLElement>(
|
||||||
|
'.grid-scroll-container',
|
||||||
|
)?.clientWidth ||
|
||||||
|
this.clientWidth ||
|
||||||
|
0
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private spacingFor(width: number): number {
|
||||||
|
return gridSpacingFor(width, this.cardSize);
|
||||||
|
}
|
||||||
|
|
||||||
/** Sort direction for the artist grid.
|
/** Sort direction for the artist grid.
|
||||||
*
|
*
|
||||||
* There is only one key to sort by: `library.Artist` carries a
|
* There is only one key to sort by: `library.Artist` carries a
|
||||||
@@ -478,6 +498,8 @@ export class ArtistsView
|
|||||||
override disconnectedCallback() {
|
override disconnectedCallback() {
|
||||||
super.disconnectedCallback();
|
super.disconnectedCallback();
|
||||||
this.detachWheelListener();
|
this.detachWheelListener();
|
||||||
|
this.gridResizeObserver?.disconnect();
|
||||||
|
this.gridResizeObserver = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** The wheel listener and the scroll debounce belong to the grid
|
/** The wheel listener and the scroll debounce belong to the grid
|
||||||
@@ -730,10 +752,34 @@ export class ArtistsView
|
|||||||
* ================================================================ */
|
* ================================================================ */
|
||||||
|
|
||||||
private lastLayoutWidth = 0;
|
private lastLayoutWidth = 0;
|
||||||
|
private lastLayoutSpacing = 0;
|
||||||
|
|
||||||
|
/** Watches the scroller so a window resize rebuilds the layout:
|
||||||
|
* the spacing is derived from its width, and nothing else asks
|
||||||
|
* this view to update when only that changes. */
|
||||||
|
private gridResizeObserver: ResizeObserver | null = null;
|
||||||
|
|
||||||
|
private observeGridWidth() {
|
||||||
|
const container =
|
||||||
|
this.renderRoot?.querySelector<HTMLElement>(
|
||||||
|
'.grid-scroll-container',
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!container || this.gridResizeObserver) return;
|
||||||
|
|
||||||
|
this.gridResizeObserver = new ResizeObserver(() =>
|
||||||
|
this.requestUpdate(),
|
||||||
|
);
|
||||||
|
this.gridResizeObserver.observe(container);
|
||||||
|
}
|
||||||
|
|
||||||
private updateGridLayout() {
|
private updateGridLayout() {
|
||||||
|
this.observeGridWidth();
|
||||||
|
|
||||||
if (
|
if (
|
||||||
this.cardSize === this.lastLayoutWidth
|
this.cardSize === this.lastLayoutWidth &&
|
||||||
|
this.lastLayoutSpacing ===
|
||||||
|
this.spacingFor(this.containerWidth)
|
||||||
) {
|
) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -86,9 +86,10 @@ export class DownloadClients extends LitElement {
|
|||||||
/** Working copy of the auto-download guardrails. */
|
/** Working copy of the auto-download guardrails. */
|
||||||
@state()
|
@state()
|
||||||
private prefs: download.AutoDownloadPrefs = {
|
private prefs: download.AutoDownloadPrefs = {
|
||||||
minSizeMb: 0,
|
minKbps: 0,
|
||||||
|
maxKbps: 0,
|
||||||
|
preferredKbps: 0,
|
||||||
maxSizeMb: 0,
|
maxSizeMb: 0,
|
||||||
preferredSizeMb: 0,
|
|
||||||
allowedFormats: [],
|
allowedFormats: [],
|
||||||
} as download.AutoDownloadPrefs;
|
} as download.AutoDownloadPrefs;
|
||||||
|
|
||||||
@@ -284,25 +285,72 @@ export class DownloadClients extends LitElement {
|
|||||||
: nothing}
|
: nothing}
|
||||||
|
|
||||||
<div class="form">
|
<div class="form">
|
||||||
|
<!-- Bitrate, not megabytes. A size means nothing
|
||||||
|
on its own: 300 MB is a generous single and a
|
||||||
|
suspiciously small boxset, and whoever fills
|
||||||
|
this in has no idea which release it will be
|
||||||
|
applied to. A rate is the same statement
|
||||||
|
divided by how long the music is, so one number
|
||||||
|
holds across an EP and an opera. -->
|
||||||
<div class="field-row">
|
<div class="field-row">
|
||||||
<wa-input
|
<wa-input
|
||||||
label="Minimum size (MB)"
|
label="Minimum bitrate (kbps)"
|
||||||
type="number"
|
type="number"
|
||||||
min="0"
|
min="0"
|
||||||
placeholder="No minimum"
|
placeholder="No minimum"
|
||||||
.value=${this.prefs.minSizeMb ? String(this.prefs.minSizeMb) : ''}
|
.value=${this.prefs.minKbps ? String(this.prefs.minKbps) : ''}
|
||||||
@input=${(e: Event) => {
|
@input=${(e: Event) => {
|
||||||
this.prefs = {
|
this.prefs = {
|
||||||
...this.prefs,
|
...this.prefs,
|
||||||
minSizeMb: Number((e.target as HTMLInputElement).value) || 0,
|
minKbps: Number((e.target as HTMLInputElement).value) || 0,
|
||||||
};
|
};
|
||||||
}}
|
}}
|
||||||
></wa-input>
|
></wa-input>
|
||||||
<wa-input
|
<wa-input
|
||||||
label="Maximum size (MB)"
|
label="Maximum bitrate (kbps)"
|
||||||
type="number"
|
type="number"
|
||||||
min="0"
|
min="0"
|
||||||
placeholder="No maximum"
|
placeholder="No maximum"
|
||||||
|
.value=${this.prefs.maxKbps ? String(this.prefs.maxKbps) : ''}
|
||||||
|
@input=${(e: Event) => {
|
||||||
|
this.prefs = {
|
||||||
|
...this.prefs,
|
||||||
|
maxKbps: Number((e.target as HTMLInputElement).value) || 0,
|
||||||
|
};
|
||||||
|
}}
|
||||||
|
></wa-input>
|
||||||
|
<wa-input
|
||||||
|
label="Preferred bitrate (kbps)"
|
||||||
|
type="number"
|
||||||
|
min="0"
|
||||||
|
placeholder="No preference"
|
||||||
|
.value=${this.prefs.preferredKbps
|
||||||
|
? String(this.prefs.preferredKbps)
|
||||||
|
: ''}
|
||||||
|
@input=${(e: Event) => {
|
||||||
|
this.prefs = {
|
||||||
|
...this.prefs,
|
||||||
|
preferredKbps:
|
||||||
|
Number((e.target as HTMLInputElement).value) || 0,
|
||||||
|
};
|
||||||
|
}}
|
||||||
|
></wa-input>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div class="requires">
|
||||||
|
320 is the top of MP3; a FLAC rip is usually
|
||||||
|
500–1000 depending on the music. Preferred
|
||||||
|
decides between copies that are otherwise equally
|
||||||
|
good — it never rules one out, which is what the
|
||||||
|
minimum and maximum are for.
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div class="field-row">
|
||||||
|
<wa-input
|
||||||
|
label="Never grab more than (MB)"
|
||||||
|
type="number"
|
||||||
|
min="0"
|
||||||
|
placeholder="No limit"
|
||||||
.value=${this.prefs.maxSizeMb ? String(this.prefs.maxSizeMb) : ''}
|
.value=${this.prefs.maxSizeMb ? String(this.prefs.maxSizeMb) : ''}
|
||||||
@input=${(e: Event) => {
|
@input=${(e: Event) => {
|
||||||
this.prefs = {
|
this.prefs = {
|
||||||
@@ -311,22 +359,14 @@ export class DownloadClients extends LitElement {
|
|||||||
};
|
};
|
||||||
}}
|
}}
|
||||||
></wa-input>
|
></wa-input>
|
||||||
<wa-input
|
</div>
|
||||||
label="Preferred size (MB)"
|
|
||||||
type="number"
|
<div class="requires">
|
||||||
min="0"
|
A ceiling on the download itself, in case a
|
||||||
placeholder="No preference"
|
mislabelled boxset gets through. Still a size
|
||||||
.value=${this.prefs.preferredSizeMb
|
because it is a question about disk space, and
|
||||||
? String(this.prefs.preferredSizeMb)
|
because it has to apply to a candidate whose
|
||||||
: ''}
|
bitrate cannot be worked out at all.
|
||||||
@input=${(e: Event) => {
|
|
||||||
this.prefs = {
|
|
||||||
...this.prefs,
|
|
||||||
preferredSizeMb:
|
|
||||||
Number((e.target as HTMLInputElement).value) || 0,
|
|
||||||
};
|
|
||||||
}}
|
|
||||||
></wa-input>
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div>
|
<div>
|
||||||
|
|||||||
@@ -111,13 +111,32 @@ const gridStyles = css`
|
|||||||
scale: 0.95;
|
scale: 0.95;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Title and year on one line, and only the title truncates.
|
||||||
|
|
||||||
|
The year used to be part of the same run of text, so it was the
|
||||||
|
first thing an ellipsis ate: a card wide enough for a long album
|
||||||
|
name never showed its year, and browsing by year showed years
|
||||||
|
only for the albums with short names -- the sort said one thing
|
||||||
|
and the cards showed another.
|
||||||
|
|
||||||
|
A flex row rather than a second line, because the card's height
|
||||||
|
is what the virtualizer measures rows by. */
|
||||||
.album-name {
|
.album-name {
|
||||||
font-size: var(--album-name-font, 14px);
|
font-size: var(--album-name-font, 14px);
|
||||||
font-weight: 400;
|
font-weight: 400;
|
||||||
color: var(--yj-text-primary, #fff);
|
color: var(--yj-text-primary, #fff);
|
||||||
|
display: flex;
|
||||||
|
justify-content: center;
|
||||||
|
align-items: baseline;
|
||||||
|
gap: 0.35em;
|
||||||
|
min-width: 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
.album-title {
|
||||||
white-space: nowrap;
|
white-space: nowrap;
|
||||||
overflow: hidden;
|
overflow: hidden;
|
||||||
text-overflow: ellipsis;
|
text-overflow: ellipsis;
|
||||||
|
min-width: 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
.artist-name {
|
.artist-name {
|
||||||
@@ -131,6 +150,8 @@ const gridStyles = css`
|
|||||||
|
|
||||||
.album-year {
|
.album-year {
|
||||||
color: var(--yj-text-tertiary, #888);
|
color: var(--yj-text-tertiary, #888);
|
||||||
|
flex: 0 0 auto;
|
||||||
|
white-space: nowrap;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* ========================================
|
/* ========================================
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import { LibraryController } from '@store/controllers/library-controller';
|
|||||||
import { SearchController } from '@store/controllers/search-controller';
|
import { SearchController } from '@store/controllers/search-controller';
|
||||||
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
import { ViewLifecycleMixin } from '@utils/view-lifecycle';
|
||||||
import { RovingGridController } from '@utils/roving-grid';
|
import { RovingGridController } from '@utils/roving-grid';
|
||||||
|
import { gridColumnsFor, gridSpacingFor } from '@utils/grid-spacing';
|
||||||
import { queueStore } from '@store/queue-store';
|
import { queueStore } from '@store/queue-store';
|
||||||
import type { QueueSource } from '@store/queue-store';
|
import type { QueueSource } from '@store/queue-store';
|
||||||
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
import '@awesome.me/webawesome/dist/components/popup/popup.js';
|
||||||
@@ -97,19 +98,36 @@ export class CoverGrid
|
|||||||
private lastAlbumsRef: library.Album[] | null =
|
private lastAlbumsRef: library.Album[] | null =
|
||||||
null;
|
null;
|
||||||
|
|
||||||
// Fixed grid spacing constants.
|
|
||||||
private static readonly GRID_GAP = 8;
|
|
||||||
private static readonly GRID_PADDING = 8;
|
|
||||||
private static readonly CARD_PADDING = 5;
|
private static readonly CARD_PADDING = 5;
|
||||||
|
|
||||||
private ctxMenu = new ContextMenuController(this);
|
private ctxMenu = new ContextMenuController(this);
|
||||||
private favCtrl = new FavoritesController(this);
|
private favCtrl = new FavoritesController(this);
|
||||||
private selMgr = new AlbumSelectionManager();
|
private selMgr = new AlbumSelectionManager();
|
||||||
private scrollMgr = new ScrollManager(this, {
|
private scrollMgr = new ScrollManager(this, {
|
||||||
GRID_GAP: CoverGrid.GRID_GAP,
|
columnsFor: (width: number) => this.columnsFor(width),
|
||||||
GRID_PADDING: CoverGrid.GRID_PADDING,
|
spacingFor: (width: number) => this.spacingFor(width),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How many cards fit across `width`, by the same arithmetic the
|
||||||
|
* virtualizer's `space-evenly` grid uses — no gap and no padding
|
||||||
|
* are reserved, because both come out of what is left over.
|
||||||
|
*
|
||||||
|
* The scroll manager restores a position by rebuilding the grid's
|
||||||
|
* geometry, so this and `spacingFor` must agree with the layout
|
||||||
|
* rather than approximate it; they were two constants that no
|
||||||
|
* longer describe anything once the spacing became elastic.
|
||||||
|
*/
|
||||||
|
columnsFor(width: number): number {
|
||||||
|
return gridColumnsFor(width, this.cardWidth);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The spacing that width produces: between columns, between rows,
|
||||||
|
* and around the outside, all the same number. */
|
||||||
|
spacingFor(width: number): number {
|
||||||
|
return gridSpacingFor(width, this.cardWidth);
|
||||||
|
}
|
||||||
|
|
||||||
private lastSelectedAlbumIndex: number | null = null;
|
private lastSelectedAlbumIndex: number | null = null;
|
||||||
private lastSelectedTrackIndex: number | null = null;
|
private lastSelectedTrackIndex: number | null = null;
|
||||||
|
|
||||||
@@ -148,10 +166,30 @@ export class CoverGrid
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Virtualizer grid layout instance — recreated when
|
// Virtualizer grid layout instance — recreated when
|
||||||
// the card size changes.
|
// the card size or the container width changes.
|
||||||
private gridLayout = this.createGridLayout();
|
private gridLayout = this.createGridLayout();
|
||||||
private gridLayoutWidth = 0;
|
private gridLayoutWidth = 0;
|
||||||
|
|
||||||
|
/** The spacing the current layouts were built with. */
|
||||||
|
private gridLayoutSpacing = 0;
|
||||||
|
|
||||||
|
/** Watches the scroll container so a window resize rebuilds the
|
||||||
|
* layout: the spacing is derived from its width, and nothing else
|
||||||
|
* asks this component to update when only that changes. */
|
||||||
|
private gridResizeObserver: ResizeObserver | null =
|
||||||
|
null;
|
||||||
|
|
||||||
|
private observeGridWidth(): void {
|
||||||
|
const container = this.scrollContainer;
|
||||||
|
|
||||||
|
if (!container || this.gridResizeObserver) return;
|
||||||
|
|
||||||
|
this.gridResizeObserver = new ResizeObserver(
|
||||||
|
() => this.requestUpdate(),
|
||||||
|
);
|
||||||
|
this.gridResizeObserver.observe(container);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Secondary layout for the "after" virtualizer in
|
* Secondary layout for the "after" virtualizer in
|
||||||
* split mode. Uses zero top padding so there is no
|
* split mode. Uses zero top padding so there is no
|
||||||
@@ -169,22 +207,49 @@ export class CoverGrid
|
|||||||
}
|
}
|
||||||
|
|
||||||
const h = w + this.cardTextHeight;
|
const h = w + this.cardTextHeight;
|
||||||
const gap = CoverGrid.GRID_GAP;
|
|
||||||
const pad = CoverGrid.GRID_PADDING;
|
// The spacing is whatever the row could not spend on another
|
||||||
|
// card, shared out equally — so it is the same number between
|
||||||
|
// two cards, between two rows, and down each outside edge.
|
||||||
|
// See `utils/grid-spacing.ts` for why it is computed rather
|
||||||
|
// than handed to the virtualizer as `space-evenly`.
|
||||||
|
const spacing = this.spacingFor(
|
||||||
|
this.containerWidth,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!noTopPad) {
|
||||||
|
this.gridLayoutSpacing = spacing;
|
||||||
|
}
|
||||||
|
|
||||||
return grid({
|
return grid({
|
||||||
itemSize: {
|
itemSize: {
|
||||||
width: `${w}px`,
|
width: `${w}px`,
|
||||||
height: `${h}px`,
|
height: `${h}px`,
|
||||||
},
|
},
|
||||||
gap: `${gap}px`,
|
gap: `${spacing}px`,
|
||||||
padding: noTopPad
|
padding: noTopPad
|
||||||
? `0 ${pad}px ${pad}px`
|
? `0 ${spacing}px ${spacing}px`
|
||||||
: `${pad}px`,
|
: `${spacing}px`,
|
||||||
justify: 'center',
|
justify: 'start',
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The width the grid lays itself out in.
|
||||||
|
*
|
||||||
|
* Read from the scroll container when there is one; before the
|
||||||
|
* first render there is not, and the fallback only has to be
|
||||||
|
* plausible — the layout is rebuilt from the real width as soon as
|
||||||
|
* one exists.
|
||||||
|
*/
|
||||||
|
private get containerWidth(): number {
|
||||||
|
return (
|
||||||
|
this.scrollContainer?.clientWidth ||
|
||||||
|
this.clientWidth ||
|
||||||
|
0
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
private dragImageEl: HTMLElement | null = null;
|
private dragImageEl: HTMLElement | null = null;
|
||||||
|
|
||||||
// -- Memoisation caches for filtered albums --
|
// -- Memoisation caches for filtered albums --
|
||||||
@@ -466,6 +531,9 @@ export class CoverGrid
|
|||||||
);
|
);
|
||||||
this.wheelListenerAttached = false;
|
this.wheelListenerAttached = false;
|
||||||
|
|
||||||
|
this.gridResizeObserver?.disconnect();
|
||||||
|
this.gridResizeObserver = null;
|
||||||
|
|
||||||
this.scrollMgr.teardown();
|
this.scrollMgr.teardown();
|
||||||
this.scrollMgr.revealContainer(
|
this.scrollMgr.revealContainer(
|
||||||
this.scrollContainer,
|
this.scrollContainer,
|
||||||
@@ -603,10 +671,18 @@ export class CoverGrid
|
|||||||
this.wheelListenerAttached = true;
|
this.wheelListenerAttached = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Recreate the virtualizer grid layout when
|
this.observeGridWidth();
|
||||||
// the card size changes.
|
|
||||||
|
// Recreate the virtualizer grid layout when the card size
|
||||||
|
// changes — or when the spacing the container width produces
|
||||||
|
// does, since that is now a derived number rather than a
|
||||||
|
// constant. Keyed on the spacing rather than on the width, or
|
||||||
|
// every pixel of a drag rebuilds a layout that would come out
|
||||||
|
// the same.
|
||||||
const cardSizeChanged =
|
const cardSizeChanged =
|
||||||
this.gridLayoutWidth !== this.cardWidth;
|
this.gridLayoutWidth !== this.cardWidth ||
|
||||||
|
this.gridLayoutSpacing !==
|
||||||
|
this.spacingFor(this.containerWidth);
|
||||||
|
|
||||||
if (cardSizeChanged) {
|
if (cardSizeChanged) {
|
||||||
this.gridLayout = this.createGridLayout();
|
this.gridLayout = this.createGridLayout();
|
||||||
@@ -1822,10 +1898,10 @@ export class CoverGrid
|
|||||||
class="album-name"
|
class="album-name"
|
||||||
title="${album.Name}"
|
title="${album.Name}"
|
||||||
>
|
>
|
||||||
${album.Name}${album.Year
|
<span class="album-title">${album.Name}</span
|
||||||
? html`
|
>${album.Year
|
||||||
<span class="album-year">
|
? html`<span class="album-year"
|
||||||
(${album.Year})</span
|
>(${album.Year})</span
|
||||||
>`
|
>`
|
||||||
: nothing}
|
: nothing}
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -6,12 +6,21 @@ import type { LibraryController } from '@store/controllers/library-controller';
|
|||||||
import type { GridEntry } from './cover-grid-types.js';
|
import type { GridEntry } from './cover-grid-types.js';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Grid spacing constants shared between the scroll
|
* Grid geometry, asked of the host rather than written down.
|
||||||
* manager and the host component.
|
*
|
||||||
|
* These were two constants, `GRID_GAP` and `GRID_PADDING`, which stopped
|
||||||
|
* describing anything the moment the grid's spacing became elastic: the
|
||||||
|
* gap, the padding and the column count are all derived from the
|
||||||
|
* container width now, and a scroll position rebuilt from a stale 8px
|
||||||
|
* lands in the wrong row.
|
||||||
*/
|
*/
|
||||||
export interface GridConstants {
|
export interface GridConstants {
|
||||||
readonly GRID_GAP: number;
|
/** Columns that fit across `width`. */
|
||||||
readonly GRID_PADDING: number;
|
columnsFor(width: number): number;
|
||||||
|
|
||||||
|
/** The spacing `width` produces — between columns, between rows,
|
||||||
|
* and around the outside, all the same number. */
|
||||||
|
spacingFor(width: number): number;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -275,8 +284,8 @@ export class ScrollManager {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
const pad = this.gc.GRID_PADDING;
|
const pad = gap;
|
||||||
const rowStep =
|
const rowStep =
|
||||||
this.host.cardHeight + gap;
|
this.host.cardHeight + gap;
|
||||||
|
|
||||||
@@ -293,7 +302,7 @@ export class ScrollManager {
|
|||||||
() => {
|
() => {
|
||||||
const rowStep =
|
const rowStep =
|
||||||
this.host.cardHeight +
|
this.host.cardHeight +
|
||||||
this.gc.GRID_GAP;
|
this.spacing(container);
|
||||||
|
|
||||||
if (this.pendingFocus === null) {
|
if (this.pendingFocus === null) {
|
||||||
this.isResizing = true;
|
this.isResizing = true;
|
||||||
@@ -351,7 +360,7 @@ export class ScrollManager {
|
|||||||
container: HTMLElement,
|
container: HTMLElement,
|
||||||
rowStep: number,
|
rowStep: number,
|
||||||
): void {
|
): void {
|
||||||
const pad = this.gc.GRID_PADDING;
|
const pad = this.spacing(container);
|
||||||
const cols = this.currentColumnCount;
|
const cols = this.currentColumnCount;
|
||||||
const filtered =
|
const filtered =
|
||||||
this.host.cachedFilteredAlbums;
|
this.host.cachedFilteredAlbums;
|
||||||
@@ -410,17 +419,15 @@ export class ScrollManager {
|
|||||||
): number {
|
): number {
|
||||||
if (!container) return 1;
|
if (!container) return 1;
|
||||||
|
|
||||||
const gap = this.gc.GRID_GAP;
|
return this.gc.columnsFor(
|
||||||
const pad = this.gc.GRID_PADDING;
|
container.clientWidth,
|
||||||
const availableWidth =
|
);
|
||||||
container.clientWidth - pad * 2;
|
}
|
||||||
|
|
||||||
return Math.max(
|
/** The grid's current spacing, which is also its padding. */
|
||||||
1,
|
private spacing(container?: HTMLElement): number {
|
||||||
Math.floor(
|
return this.gc.spacingFor(
|
||||||
(availableWidth + gap) /
|
container?.clientWidth ?? 800,
|
||||||
(this.host.cardWidth + gap),
|
|
||||||
),
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -439,7 +446,7 @@ export class ScrollManager {
|
|||||||
container?: HTMLElement,
|
container?: HTMLElement,
|
||||||
): number {
|
): number {
|
||||||
const cols = this.getColumnCount(container);
|
const cols = this.getColumnCount(container);
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
cols * this.host.cardWidth +
|
cols * this.host.cardWidth +
|
||||||
@@ -460,7 +467,7 @@ export class ScrollManager {
|
|||||||
|
|
||||||
const cols = this.getColumnCount(container);
|
const cols = this.getColumnCount(container);
|
||||||
const colIndex = idx % cols;
|
const colIndex = idx % cols;
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
colIndex *
|
colIndex *
|
||||||
@@ -597,8 +604,8 @@ export class ScrollManager {
|
|||||||
|
|
||||||
if (!this.host.splitMode) return raw;
|
if (!this.host.splitMode) return raw;
|
||||||
|
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
const pad = this.gc.GRID_PADDING;
|
const pad = gap;
|
||||||
const columns =
|
const columns =
|
||||||
this.getColumnCount(container);
|
this.getColumnCount(container);
|
||||||
const rowStep = this.host.cardHeight + gap;
|
const rowStep = this.host.cardHeight + gap;
|
||||||
@@ -678,8 +685,8 @@ export class ScrollManager {
|
|||||||
|
|
||||||
if (expandedIndex < 0) return;
|
if (expandedIndex < 0) return;
|
||||||
|
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
const pad = this.gc.GRID_PADDING;
|
const pad = gap;
|
||||||
const columns =
|
const columns =
|
||||||
this.getColumnCount(container);
|
this.getColumnCount(container);
|
||||||
const rowStep = this.host.cardHeight + gap;
|
const rowStep = this.host.cardHeight + gap;
|
||||||
@@ -772,8 +779,8 @@ export class ScrollManager {
|
|||||||
|
|
||||||
if (idx < 0) return;
|
if (idx < 0) return;
|
||||||
|
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
const pad = this.gc.GRID_PADDING;
|
const pad = gap;
|
||||||
const cols =
|
const cols =
|
||||||
this.getColumnCount(container);
|
this.getColumnCount(container);
|
||||||
const rowStep = this.host.cardHeight + gap;
|
const rowStep = this.host.cardHeight + gap;
|
||||||
@@ -854,9 +861,8 @@ export class ScrollManager {
|
|||||||
this.getExpandedAlbumIndex();
|
this.getExpandedAlbumIndex();
|
||||||
|
|
||||||
if (idx >= 0) {
|
if (idx >= 0) {
|
||||||
const gap = this.gc.GRID_GAP;
|
const gap = this.spacing(container);
|
||||||
const pad =
|
const pad = gap;
|
||||||
this.gc.GRID_PADDING;
|
|
||||||
const cols =
|
const cols =
|
||||||
this.getColumnCount(
|
this.getColumnCount(
|
||||||
container,
|
container,
|
||||||
|
|||||||
@@ -400,7 +400,7 @@ export class DownloadsView extends ViewLifecycleMixin(LitElement) {
|
|||||||
private renderEmptyRequests() {
|
private renderEmptyRequests() {
|
||||||
return html`
|
return html`
|
||||||
<div class="empty">
|
<div class="empty">
|
||||||
Nothing requested yet. Use “Want this” on an album or artist
|
Nothing requested yet. Use “Request this” on an album or artist
|
||||||
to add it here.
|
to add it here.
|
||||||
</div>
|
</div>
|
||||||
`;
|
`;
|
||||||
|
|||||||
@@ -2680,7 +2680,7 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
|
|||||||
slot="start"
|
slot="start"
|
||||||
name=${this.isRequested ? 'solid/bookmark' : 'regular/bookmark'}
|
name=${this.isRequested ? 'solid/bookmark' : 'regular/bookmark'}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
${this.isRequested ? 'Wanted' : 'Want this'}
|
${this.isRequested ? 'Requested' : 'Request this'}
|
||||||
</wa-button>
|
</wa-button>
|
||||||
`;
|
`;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2753,7 +2753,7 @@ export class ExploreArtistDetails extends LitElement implements ContextMenuHost
|
|||||||
slot="icon"
|
slot="icon"
|
||||||
name=${requested ? 'xmark' : 'bookmark'}
|
name=${requested ? 'xmark' : 'bookmark'}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
${requested ? 'Cancel Request' : 'Want This'}
|
${requested ? 'Cancel Request' : 'Request This'}
|
||||||
</wa-dropdown-item>
|
</wa-dropdown-item>
|
||||||
`
|
`
|
||||||
: nothing}
|
: nothing}
|
||||||
|
|||||||
@@ -1509,9 +1509,24 @@ export class ExploreView extends ViewLifecycleMixin(LitElement) implements Conte
|
|||||||
if (url) {
|
if (url) {
|
||||||
this.thumbnailCache.set(req.mbid, url);
|
this.thumbnailCache.set(req.mbid, url);
|
||||||
this.requestUpdate();
|
this.requestUpdate();
|
||||||
|
|
||||||
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// An empty answer is not necessarily "there
|
||||||
|
// is no art" — a slow Internet Archive node
|
||||||
|
// is answered by a timeout, which looks
|
||||||
|
// exactly the same from here. Drop the
|
||||||
|
// in-flight marker so the next time this
|
||||||
|
// release group is on screen it is asked
|
||||||
|
// again; the backend records a genuine 404
|
||||||
|
// on disk and answers that one instantly,
|
||||||
|
// so a real miss costs nothing to re-ask.
|
||||||
|
this.thumbnailCache.delete(req.mbid);
|
||||||
})
|
})
|
||||||
.catch(() => {});
|
.catch(() => {
|
||||||
|
this.thumbnailCache.delete(req.mbid);
|
||||||
|
});
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
.catch(() => {
|
.catch(() => {
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import type {
|
|||||||
VisibilityChangedEvent,
|
VisibilityChangedEvent,
|
||||||
} from '@lit-labs/virtualizer';
|
} from '@lit-labs/virtualizer';
|
||||||
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
|
import { grid } from '@lit-labs/virtualizer/layouts/grid.js';
|
||||||
|
import { gridSpacingFor } from '@utils/grid-spacing';
|
||||||
import {
|
import {
|
||||||
GetFilePathsByGenres,
|
GetFilePathsByGenres,
|
||||||
} from '@go/library/library.js';
|
} from '@go/library/library.js';
|
||||||
@@ -155,8 +156,6 @@ export class GenresView
|
|||||||
|
|
||||||
// ----- Grid spacing constants -----
|
// ----- Grid spacing constants -----
|
||||||
|
|
||||||
private static readonly GRID_GAP = 8;
|
|
||||||
private static readonly GRID_PADDING = 8;
|
|
||||||
private static readonly CARD_PADDING = 5;
|
private static readonly CARD_PADDING = 5;
|
||||||
|
|
||||||
private get imageSize(): number {
|
private get imageSize(): number {
|
||||||
@@ -185,20 +184,41 @@ export class GenresView
|
|||||||
private createGridLayout() {
|
private createGridLayout() {
|
||||||
const w = this.cardSize ?? CARD_SIZE_DEFAULT;
|
const w = this.cardSize ?? CARD_SIZE_DEFAULT;
|
||||||
const h = w + this.cardTextHeight;
|
const h = w + this.cardTextHeight;
|
||||||
const gap = GenresView.GRID_GAP;
|
|
||||||
const pad = GenresView.GRID_PADDING;
|
// One number for the gap, the row gap and the padding: whatever
|
||||||
|
// a row could not spend on another card, shared out equally, so
|
||||||
|
// the outside is never wider than the inside. See
|
||||||
|
// `utils/grid-spacing.ts`.
|
||||||
|
const spacing = this.spacingFor(this.containerWidth);
|
||||||
|
|
||||||
|
this.lastLayoutSpacing = spacing;
|
||||||
|
|
||||||
return grid({
|
return grid({
|
||||||
itemSize: {
|
itemSize: {
|
||||||
width: `${w}px`,
|
width: `${w}px`,
|
||||||
height: `${h}px`,
|
height: `${h}px`,
|
||||||
},
|
},
|
||||||
gap: `${gap}px`,
|
gap: `${spacing}px`,
|
||||||
padding: `${pad}px`,
|
padding: `${spacing}px`,
|
||||||
justify: 'center',
|
justify: 'start',
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** The width the grid lays itself out in. */
|
||||||
|
private get containerWidth(): number {
|
||||||
|
return (
|
||||||
|
this.renderRoot?.querySelector<HTMLElement>(
|
||||||
|
'.grid-scroll-container',
|
||||||
|
)?.clientWidth ||
|
||||||
|
this.clientWidth ||
|
||||||
|
0
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
private spacingFor(width: number): number {
|
||||||
|
return gridSpacingFor(width, this.cardSize);
|
||||||
|
}
|
||||||
|
|
||||||
/** Sort key and direction for the genre grid (H-19: it had none). */
|
/** Sort key and direction for the genre grid (H-19: it had none). */
|
||||||
@state()
|
@state()
|
||||||
private sortField: 'name' | 'tracks' = 'name';
|
private sortField: 'name' | 'tracks' = 'name';
|
||||||
@@ -483,6 +503,8 @@ export class GenresView
|
|||||||
override disconnectedCallback() {
|
override disconnectedCallback() {
|
||||||
super.disconnectedCallback();
|
super.disconnectedCallback();
|
||||||
this.detachWheelListener();
|
this.detachWheelListener();
|
||||||
|
this.gridResizeObserver?.disconnect();
|
||||||
|
this.gridResizeObserver = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** See artists-view: off-screen the grid cannot be scrolled, and
|
/** See artists-view: off-screen the grid cannot be scrolled, and
|
||||||
@@ -737,10 +759,34 @@ export class GenresView
|
|||||||
* ================================================================ */
|
* ================================================================ */
|
||||||
|
|
||||||
private lastLayoutWidth = 0;
|
private lastLayoutWidth = 0;
|
||||||
|
private lastLayoutSpacing = 0;
|
||||||
|
|
||||||
|
/** Watches the scroller so a window resize rebuilds the layout:
|
||||||
|
* the spacing is derived from its width, and nothing else asks
|
||||||
|
* this view to update when only that changes. */
|
||||||
|
private gridResizeObserver: ResizeObserver | null = null;
|
||||||
|
|
||||||
|
private observeGridWidth() {
|
||||||
|
const container =
|
||||||
|
this.renderRoot?.querySelector<HTMLElement>(
|
||||||
|
'.grid-scroll-container',
|
||||||
|
);
|
||||||
|
|
||||||
|
if (!container || this.gridResizeObserver) return;
|
||||||
|
|
||||||
|
this.gridResizeObserver = new ResizeObserver(() =>
|
||||||
|
this.requestUpdate(),
|
||||||
|
);
|
||||||
|
this.gridResizeObserver.observe(container);
|
||||||
|
}
|
||||||
|
|
||||||
private updateGridLayout() {
|
private updateGridLayout() {
|
||||||
|
this.observeGridWidth();
|
||||||
|
|
||||||
if (
|
if (
|
||||||
this.cardSize === this.lastLayoutWidth
|
this.cardSize === this.lastLayoutWidth &&
|
||||||
|
this.lastLayoutSpacing ===
|
||||||
|
this.spacingFor(this.containerWidth)
|
||||||
) {
|
) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -48,7 +48,7 @@ export type LibraryStatus =
|
|||||||
*
|
*
|
||||||
* Colours and glyphs:
|
* Colours and glyphs:
|
||||||
* - in-library → green circle, check mark
|
* - in-library → green circle, check mark
|
||||||
* - queued → amber circle, hourglass
|
* - queued → amber circle, bookmark ("on your list")
|
||||||
* - not-in-library → grey circle, plus sign
|
* - not-in-library → grey circle, plus sign
|
||||||
*
|
*
|
||||||
* Usage:
|
* Usage:
|
||||||
@@ -241,12 +241,23 @@ export class LibraryStatusIndicator extends LitElement {
|
|||||||
}
|
}
|
||||||
`;
|
`;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The glyph for each state.
|
||||||
|
*
|
||||||
|
* `queued` is a **bookmark**, not the hourglass it used to be. An
|
||||||
|
* hourglass says "wait, this is under way", which overstates what a
|
||||||
|
* request is: nothing may be downloading, nothing may ever be found,
|
||||||
|
* and the user can leave one sitting on the list indefinitely. A
|
||||||
|
* bookmark says the honest thing — it is on your list — and reads as
|
||||||
|
* the opposite of the plus that put it there, which is what a
|
||||||
|
* toggle's two states have to do.
|
||||||
|
*/
|
||||||
private iconName(): string {
|
private iconName(): string {
|
||||||
switch (this.status) {
|
switch (this.status) {
|
||||||
case 'in-library':
|
case 'in-library':
|
||||||
return 'check';
|
return 'check';
|
||||||
case 'queued':
|
case 'queued':
|
||||||
return 'hourglass-half';
|
return 'bookmark';
|
||||||
default:
|
default:
|
||||||
return 'plus';
|
return 'plus';
|
||||||
}
|
}
|
||||||
@@ -276,7 +287,7 @@ export class LibraryStatusIndicator extends LitElement {
|
|||||||
if (this.actionable) {
|
if (this.actionable) {
|
||||||
return this.status === 'queued'
|
return this.status === 'queued'
|
||||||
? `Cancel the request for ${kind}${name}`
|
? `Cancel the request for ${kind}${name}`
|
||||||
: `Want ${kind}${name}`;
|
: `Request ${kind}${name}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
switch (this.status) {
|
switch (this.status) {
|
||||||
@@ -354,25 +365,6 @@ export class LibraryStatusIndicator extends LitElement {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const title = this.tooltip();
|
const title = this.tooltip();
|
||||||
const icon = this.iconName()
|
|
||||||
? html`<wa-icon name=${this.iconName()} aria-hidden="true"></wa-icon>`
|
|
||||||
: nothing;
|
|
||||||
|
|
||||||
if (this.actionable) {
|
|
||||||
return html`
|
|
||||||
<button
|
|
||||||
class="badge"
|
|
||||||
type="button"
|
|
||||||
title=${title}
|
|
||||||
aria-label=${title}
|
|
||||||
?disabled=${this.busy}
|
|
||||||
@click=${this.onActivate}
|
|
||||||
@keydown=${this.onKeydown}
|
|
||||||
>
|
|
||||||
${icon}
|
|
||||||
</button>
|
|
||||||
`;
|
|
||||||
}
|
|
||||||
|
|
||||||
// The ring stands in for the icon wherever the icon would go —
|
// The ring stands in for the icon wherever the icon would go —
|
||||||
// including inside the button, because a partly-held album is
|
// including inside the button, because a partly-held album is
|
||||||
|
|||||||
@@ -307,7 +307,7 @@ export class NowPlayingView extends LitElement {
|
|||||||
: `Add ${track.title} to ${this.favCtrl.playlistName}`}
|
: `Add ${track.title} to ${this.favCtrl.playlistName}`}
|
||||||
@click=${this.toggleFavorite}
|
@click=${this.toggleFavorite}
|
||||||
>
|
>
|
||||||
<wa-icon name=${this.favCtrl.iconName}></wa-icon>
|
<wa-icon name=${this.favCtrl.iconFor(favorited)}></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -527,7 +527,7 @@ export class NowPlaying extends LitElement {
|
|||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
<wa-icon
|
<wa-icon
|
||||||
name=${this.favCtrl.iconName}
|
name=${this.favCtrl.iconFor(isFav)}
|
||||||
variant=${favVariant}
|
variant=${favVariant}
|
||||||
></wa-icon>
|
></wa-icon>
|
||||||
</button>
|
</button>
|
||||||
|
|||||||
@@ -1756,7 +1756,7 @@ export class PlaylistView extends ViewLifecycleMixin(LitElement) {
|
|||||||
${entry.summary.ID === this.favCtrl.playlistId
|
${entry.summary.ID === this.favCtrl.playlistId
|
||||||
? html`<wa-icon
|
? html`<wa-icon
|
||||||
class="playlist-icon"
|
class="playlist-icon"
|
||||||
name=${this.favCtrl.iconName}
|
name=${this.favCtrl.iconFor(true)}
|
||||||
></wa-icon>`
|
></wa-icon>`
|
||||||
: entry.summary.IsSmart
|
: entry.summary.IsSmart
|
||||||
? html`<wa-icon
|
? html`<wa-icon
|
||||||
|
|||||||
@@ -16,6 +16,7 @@
|
|||||||
# fetch-icons.mjs for why that is not negotiable. Re-vendor with:
|
# fetch-icons.mjs for why that is not negotiable. Re-vendor with:
|
||||||
# node frontend/scripts/fetch-icons.mjs
|
# node frontend/scripts/fetch-icons.mjs
|
||||||
regular/heart
|
regular/heart
|
||||||
|
regular/star
|
||||||
solid/arrow-down-wide-short
|
solid/arrow-down-wide-short
|
||||||
solid/arrow-left
|
solid/arrow-left
|
||||||
solid/arrow-rotate-right
|
solid/arrow-rotate-right
|
||||||
|
|||||||
@@ -74,12 +74,36 @@ export class FavoritesController
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the icon name for the current icon style.
|
* The icon name for the current icon style, unfilled.
|
||||||
|
*
|
||||||
|
* Prefer `iconFor(isFav)` — this getter is the name of the *empty*
|
||||||
|
* glyph, which is what every caller that does not know the state
|
||||||
|
* should draw.
|
||||||
*/
|
*/
|
||||||
get iconName(): string {
|
get iconName(): string {
|
||||||
return this.iconStyle === 'star'
|
return this.iconFor(false);
|
||||||
? 'star'
|
}
|
||||||
: 'heart';
|
|
||||||
|
/**
|
||||||
|
* The glyph for one track's favourite state.
|
||||||
|
*
|
||||||
|
* **A filled shape means favourited and an outline means not**, in
|
||||||
|
* every list in the app. Nine components rendered `iconName`, which
|
||||||
|
* was the *solid* glyph in both states — so "not a favourite" was a
|
||||||
|
* filled heart in a duller colour, and the only thing separating
|
||||||
|
* the two states was hue. That fails for anyone who cannot see the
|
||||||
|
* difference between them, and reads as "everything is a favourite"
|
||||||
|
* to everyone else. `track-list` and `album-dropdown` already drew
|
||||||
|
* it correctly, from inline SVG paths of their own; this is the
|
||||||
|
* same rule for the `<wa-icon>` call sites.
|
||||||
|
*
|
||||||
|
* The Font Awesome family is part of the name — `regular/heart` is
|
||||||
|
* the outline, a bare `heart` is the solid one (`src/icons`).
|
||||||
|
*/
|
||||||
|
iconFor(favorited: boolean): string {
|
||||||
|
const shape = this.iconStyle === 'star' ? 'star' : 'heart';
|
||||||
|
|
||||||
|
return favorited ? shape : `regular/${shape}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
// ===============================================================
|
// ===============================================================
|
||||||
|
|||||||
@@ -0,0 +1,58 @@
|
|||||||
|
/**
|
||||||
|
* Even spacing for the three card grids — albums, artists, genres.
|
||||||
|
*
|
||||||
|
* All three used `justify: 'center'` with a fixed 8px gap and 8px
|
||||||
|
* padding, which gives the row a fixed width and pushes everything left
|
||||||
|
* over to the two margins: on a 1440px window the albums grid drew its
|
||||||
|
* cards 16px apart inside 78px of nothing down each side. The outside
|
||||||
|
* was five times the inside.
|
||||||
|
*
|
||||||
|
* The fix is to spend the leftover on the spacing instead, so there is
|
||||||
|
* one number: between two cards, between two rows, and down each edge.
|
||||||
|
* The virtualizer has a word for that — `justify: 'space-evenly'` with
|
||||||
|
* `gap: 'auto'` — and it cannot be used, because it fits
|
||||||
|
* `floor(width / cardWidth)` columns without reserving the gap it is
|
||||||
|
* about to need: a width one card short of exact fits seven cards a
|
||||||
|
* pixel apart. Deciding the column count here is what puts a floor
|
||||||
|
* under the spacing, and the grid is then given plain numbers.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** The narrowest the spacing is allowed to get. */
|
||||||
|
export const MIN_GRID_SPACING = 8;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* How many cards of `cardWidth` fit across `width`.
|
||||||
|
*
|
||||||
|
* A row of c cards spends c×cardWidth on cards and (c+1)×spacing on the
|
||||||
|
* spaces between and beside them, so c is bounded by
|
||||||
|
* (width − spacing) / (cardWidth + spacing) at the minimum spacing.
|
||||||
|
*/
|
||||||
|
export function gridColumnsFor(
|
||||||
|
width: number,
|
||||||
|
cardWidth: number,
|
||||||
|
): number {
|
||||||
|
if (cardWidth <= 0) return 1;
|
||||||
|
|
||||||
|
const fit = Math.floor(
|
||||||
|
(width - MIN_GRID_SPACING) / (cardWidth + MIN_GRID_SPACING),
|
||||||
|
);
|
||||||
|
|
||||||
|
return Math.max(1, fit);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The spacing `width` produces — the gap, the row gap and the padding,
|
||||||
|
* which are all the same number.
|
||||||
|
*/
|
||||||
|
export function gridSpacingFor(
|
||||||
|
width: number,
|
||||||
|
cardWidth: number,
|
||||||
|
): number {
|
||||||
|
const columns = gridColumnsFor(width, cardWidth);
|
||||||
|
const leftover = width - columns * cardWidth;
|
||||||
|
|
||||||
|
return Math.max(
|
||||||
|
MIN_GRID_SPACING,
|
||||||
|
Math.floor(leftover / (columns + 1)),
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,87 @@
|
|||||||
|
/**
|
||||||
|
* The year on an album card survives a long album name.
|
||||||
|
*
|
||||||
|
* The year used to be part of the same run of text as the title, inside
|
||||||
|
* one `text-overflow: ellipsis` box — so it was the first thing the
|
||||||
|
* ellipsis ate. A card wide enough for a long name never showed its
|
||||||
|
* year at all, which means sorting the grid *by year* showed years only
|
||||||
|
* for the albums with short names: the sort said one thing and the
|
||||||
|
* cards showed another.
|
||||||
|
*
|
||||||
|
* The fix is a flex row in which only the title truncates, rather than
|
||||||
|
* a second line, because the card's height is what the virtualizer
|
||||||
|
* measures rows by.
|
||||||
|
*/
|
||||||
|
import { describe, expect, it, beforeEach } from 'vitest';
|
||||||
|
import type { LitElement } from 'lit';
|
||||||
|
|
||||||
|
import '@components/cover-grid/cover-grid';
|
||||||
|
import { emit, stub, flush, resetHarness } from '@test/support/harness';
|
||||||
|
import { Events } from '../../src/events';
|
||||||
|
import { fixture, shadowAll } from '@test/support/render';
|
||||||
|
|
||||||
|
const LONG =
|
||||||
|
'The Rise and Fall of a Midwest Princess in the Key of Everything';
|
||||||
|
|
||||||
|
/** Long names throughout: the fault only shows on a card under
|
||||||
|
* pressure, and a grid of "Album 3" proves nothing. */
|
||||||
|
const ALBUMS = Array.from({ length: 12 }, (_, i) => ({
|
||||||
|
ID: i + 1,
|
||||||
|
Name: `${LONG} ${i + 1}`,
|
||||||
|
ArtistName: 'Aurora Fields',
|
||||||
|
Year: 2019 + (i % 5),
|
||||||
|
}));
|
||||||
|
|
||||||
|
/** Give the virtualizer a viewport; a zero-height host renders nothing. */
|
||||||
|
function sized(el: HTMLElement): void {
|
||||||
|
el.style.display = 'block';
|
||||||
|
el.style.height = '600px';
|
||||||
|
el.style.width = '900px';
|
||||||
|
}
|
||||||
|
|
||||||
|
async function settle(el: LitElement): Promise<void> {
|
||||||
|
await flush();
|
||||||
|
await el.updateComplete;
|
||||||
|
await new Promise((r) => setTimeout(r, 80));
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('the album card’s year', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetHarness();
|
||||||
|
stub('library.Library.GetAlbums', ALBUMS);
|
||||||
|
stub('library.Library.GetTracks', []);
|
||||||
|
emit(Events.LibraryScanComplete);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is rendered on every card, however long the name', async () => {
|
||||||
|
const el = await fixture<LitElement>('cover-grid');
|
||||||
|
|
||||||
|
sized(el);
|
||||||
|
await settle(el);
|
||||||
|
|
||||||
|
const cards = shadowAll(el, '.album-card');
|
||||||
|
const years = shadowAll(el, '.album-year');
|
||||||
|
|
||||||
|
expect(cards.length).toBeGreaterThan(0);
|
||||||
|
expect(years).toHaveLength(cards.length);
|
||||||
|
expect(years.every((y) => /^\(\d{4}\)$/.test(y.textContent!.trim()))).toBe(
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('is not what the ellipsis eats', async () => {
|
||||||
|
const el = await fixture<LitElement>('cover-grid');
|
||||||
|
|
||||||
|
sized(el);
|
||||||
|
await settle(el);
|
||||||
|
|
||||||
|
const year = shadowAll(el, '.album-year')[0]!;
|
||||||
|
const title = shadowAll(el, '.album-title')[0]!;
|
||||||
|
|
||||||
|
// The title is the box that gives way...
|
||||||
|
expect(title.scrollWidth).toBeGreaterThan(title.clientWidth);
|
||||||
|
// ...and the year keeps every pixel it asked for.
|
||||||
|
expect(year.clientWidth).toBeGreaterThan(0);
|
||||||
|
expect(year.scrollWidth).toBeLessThanOrEqual(year.clientWidth + 1);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -120,7 +120,7 @@ describe('the context menu on an artist page release', () => {
|
|||||||
expect(items).toContain('Add to Queue');
|
expect(items).toContain('Add to Queue');
|
||||||
expect(items).toContain('Play Next');
|
expect(items).toContain('Play Next');
|
||||||
// Owned: there is nothing left to ask for.
|
// Owned: there is nothing left to ask for.
|
||||||
expect(items).not.toContain('Want This');
|
expect(items).not.toContain('Request This');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('offers a request, and no playback, for a release nobody owns', async () => {
|
it('offers a request, and no playback, for a release nobody owns', async () => {
|
||||||
@@ -132,7 +132,7 @@ describe('the context menu on an artist page release', () => {
|
|||||||
|
|
||||||
expect(items).not.toContain('Play');
|
expect(items).not.toContain('Play');
|
||||||
expect(items).not.toContain('Add to Queue');
|
expect(items).not.toContain('Add to Queue');
|
||||||
expect(items).toContain('Want This');
|
expect(items).toContain('Request This');
|
||||||
expect(items).toContain('View on MusicBrainz');
|
expect(items).toContain('View on MusicBrainz');
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -148,7 +148,7 @@ describe('the context menu on an artist page release', () => {
|
|||||||
// …but a `local:` id names nothing upstream, and wanting something
|
// …but a `local:` id names nothing upstream, and wanting something
|
||||||
// already in the library is not a thing to offer.
|
// already in the library is not a thing to offer.
|
||||||
expect(items).not.toContain('View on MusicBrainz');
|
expect(items).not.toContain('View on MusicBrainz');
|
||||||
expect(items).not.toContain('Want This');
|
expect(items).not.toContain('Request This');
|
||||||
});
|
});
|
||||||
|
|
||||||
it('opens from the keyboard on Shift+F10', async () => {
|
it('opens from the keyboard on Shift+F10', async () => {
|
||||||
|
|||||||
@@ -166,7 +166,7 @@ describe('<library-status-indicator>', () => {
|
|||||||
glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
|
glyphs.push(shadow(el, 'wa-icon')?.getAttribute('name'));
|
||||||
}
|
}
|
||||||
|
|
||||||
expect(glyphs).toEqual(['check', 'hourglass-half', 'plus']);
|
expect(glyphs).toEqual(['check', 'bookmark', 'plus']);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('phrases its label around the entity it describes', async () => {
|
it('phrases its label around the entity it describes', async () => {
|
||||||
|
|||||||
@@ -249,7 +249,7 @@ describe('<library-status-indicator> as a control', () => {
|
|||||||
const el = await badge({ requestMbid: 'rg-1' });
|
const el = await badge({ requestMbid: 'rg-1' });
|
||||||
|
|
||||||
expect(shadow(el, '.badge')?.getAttribute('aria-label')).toBe(
|
expect(shadow(el, '.badge')?.getAttribute('aria-label')).toBe(
|
||||||
'Want album "Abbey Road"',
|
'Request album "Abbey Road"',
|
||||||
);
|
);
|
||||||
|
|
||||||
await update(el, { status: 'queued' });
|
await update(el, { status: 'queued' });
|
||||||
|
|||||||
Reference in New Issue
Block a user