Skip to content

DlgTrackInfo: add editable tuning field - #16029

Merged
ronso0 merged 1 commit into
mixxxdj:mainfrom
Swarnadip-Kar:add-tuning-field-trackinfo
Mar 10, 2026
Merged

DlgTrackInfo: add editable tuning field#16029
ronso0 merged 1 commit into
mixxxdj:mainfrom
Swarnadip-Kar:add-tuning-field-trackinfo

Conversation

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor

Summary

Adds an editable tuning frequency field to the Track Editor (Properties) dialog.

Daniel suggested this on Zulip to complement the tuning detection in PR #15817.

What it does

  • Adds a QDoubleSpinBox (range 400-480 Hz) for A4 tuning frequency
  • Auto-computes and displays the cents offset from A440 standard tuning
  • Placed in the tags section next to the Key field (tuning is related to key)
  • User-entered values are saved to the existing tuning_frequency_hz DB column
  • Shows empty when no tuning data is available

Screenshots

Before

Before Screenshot

After

After Screenshot
After.Recording.mp4

Testing

  • Opened Track Properties for unanalyzed track → field shows empty
  • Entered 442 Hz → cents label shows "+8 ct"
  • Entered 432 Hz → cents label shows "-32 ct"
  • Applied, closed, reopened → value persists

Related: #15817

@daschuer

Copy link
Copy Markdown
Member

Works good. Thank you. While it looks good from the layout aspect, I am just a bit torn about the place between Genre and Grouping. What are alternatives?
It belongs clearly to "Key" but it is more a technical property like BPM, which may change thou-out the track.

For me it is good enough for a first version
@ronso0 do you have preferences?

@Swarnadip-Kar

Swarnadip-Kar commented Feb 23, 2026

Copy link
Copy Markdown
Contributor Author

I am just a bit torn about the place between Genre and Grouping. What are alternatives? It belongs clearly to "Key" but it is more a technical property like BPM, which may change thou-out the track.

I got it.
IMO move Key Tuning and Offset one Row down Each and move Grouping one row Above and Track Two Rows above.
What Do you think?

Row Input Right Col
────────────────────────
Title 0001_12step [Album Art]
Artist
Album
Album Artist . . . .
Composer Year
Genre [input] Track # (moved up 2 rows)
Grouping Key
Tuning 437.1 Hz Offset -11 ct

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

@ronso0 do you have preferences?

Hi @ronso0 👋 — since you're already in the zone reviewing my other PRs, would you mind taking a quick look at this one too? @daschuer also wanted your thoughts on the placement of the tuning field in the layout. Happy to adjust anything!

@Swarnadip-Kar
Swarnadip-Kar force-pushed the add-tuning-field-trackinfo branch 2 times, most recently from eab4a78 to 1e93999 Compare March 4, 2026 10:50
@ronso0

ronso0 commented Mar 4, 2026

Copy link
Copy Markdown
Member

@daschuer also wanted your thoughts on the placement of the tuning field in the layout.

Hmm, this is a bit tricky IMO. On the one hand, we currently have long text fields on the left and short num fields/stars/key on the right. Otoh, Tuning and the Offset label should better be in a row, ideally also close to Key.
Shall we move all three below Grouping?

Genre    [__________________________]  Track # [____]
Grouping [__________________________]
Key      [_______]   Tuning [_______]  Offset   ____

Comment thread src/library/dlgtrackinfo.cpp Outdated
Comment thread src/library/dlgtrackinfo.cpp Outdated
@Swarnadip-Kar
Swarnadip-Kar force-pushed the add-tuning-field-trackinfo branch from 1e93999 to dd96801 Compare March 4, 2026 11:47
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

i have removed the extra (AI) comments although I kept some I though would be good. (Rebased to one commit)

@Swarnadip-Kar
Swarnadip-Kar requested a review from ronso0 March 4, 2026 12:18
@ronso0

ronso0 commented Mar 9, 2026

Copy link
Copy Markdown
Member

This is still using the old layout which is suboptimal IMO.
What do you think about the proposal I posted above #16029 (comment) ?

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Hmm, this is a bit tricky IMO. On the one hand, we currently have long text fields on the left and short num fields/stars/key on the right. Otoh, Tuning and the Offset label should better be in a row, ideally also close to Key. Shall we move all three below Grouping?

Genre    [__________________________]  Track # [____]
Grouping [__________________________]
Key      [_______]   Tuning [_______]  Offset   ____

That layout looks great to me. I'll implement it — Key, Tuning, and Offset all in one row below Grouping makes more sense.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Here's how it currently looks — Key sits next to Grouping, and Tuning/Offset share the row below. Does this work for you?

Screenshot 2026-03-09 at 7 50 08 PM

Your original suggestion of putting Key, Tuning, and Offset all in one row is tricky to implement cleanly — the tags grid is 4 columns wide, and fitting all three alongside Key would either require adding extra columns (which affects every other row) or nesting a sub-layout, both of which add unnecessary complexity.

@ronso0

ronso0 commented Mar 9, 2026

Copy link
Copy Markdown
Member

What's tricky about a QHBoxLayout in the second column?
image
Size policies can be adjusted though, maybe add a tiny spacer between [..key..] and Tuning

commit is here 84f5a8d7d2

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Thanks! Got it working with a QHBoxLayout — used Expanding instead of Preferred for the size policy as it looked better. Will add a small spacer between Key and Tuning too. Introduced another bug in the process that I'm currently tracking down, will push once that's fixed.

@Swarnadip-Kar
Swarnadip-Kar force-pushed the add-tuning-field-trackinfo branch 2 times, most recently from d76befd to f147893 Compare March 9, 2026 16:46
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Implemented the layout changes you suggested. Also fixed two issues I noticed along the way — a merge conflict from my BPM lock PR (#16025), and a bug where the tuning spinbox always showed 400 Hz even for tracks with analysed tuning data.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Thanks! Got it working with a QHBoxLayout — used Expanding instead of Preferred for the size policy as it looked better. Will add a small spacer between Key and Tuning too.

Update: ended up using right-alignment on the Tuning label instead of a spacer — IMO it looks better when the window is resized.

@ronso0

ronso0 commented Mar 9, 2026

Copy link
Copy Markdown
Member

Looks good (I don't see a difference to my commit tbh).
Wdyt about adding that spacer? The key field is much wider than necessary, and it can't shrink, so a spacer a spacer would IMO help a bit with relating the Tuning field to its spinbox
image

And the tabstops need to be adjusted to the new item order.
Do we need to set buddy for the new items?


Btw this was a full rebase onto main again. Another implication, besides the blown Compare view I mentioned, is that users that have ccache set up and are checking out your branch again to build it also have to rebuild more than just your actual changes.
Please avoid full rebase from now on unless instructed to do so. Thanks.

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Will add the spacer and fix the tabstops. On buddy — will check if the new labels need it.

Btw this was a full rebase onto main again. Another implication, besides the blown Compare view I mentioned, is that users that have ccache set up and are checking out your branch again to build it also have to rebuild more than just your actual changes.
Please avoid full rebase from now on unless instructed to do so. Thanks.

Apologies for the unnecessary rebase — I had a merge conflict from my BPM lock PR and rebased to resolve it. What would have been the correct way to handle that without a full rebase?

@Swarnadip-Kar

Swarnadip-Kar commented Mar 9, 2026

Copy link
Copy Markdown
Contributor Author

And the tabstops need to be adjusted to the new item order.
Do we need to set buddy for the new items?

Added the spacer and updated the tabstops. On buddy — I checked and none of the existing labels in the file use shortcuts, so I don't think buddy relationships are needed for the new items. Let me know if you think otherwise

@ronso0

ronso0 commented Mar 9, 2026

Copy link
Copy Markdown
Member

What would have been the correct way to handle that without a full rebase?

If you rebased to resolve conflicts cuased by another PR: none I know of.
If there are no conflicts, I always use (what I already mentioned in another PR of yours)
git rebase [first commit of feature branch]^1
in order to squash, fixup, rename, reorder etc.

@ronso0

ronso0 commented Mar 9, 2026

Copy link
Copy Markdown
Member

On buddy — I checked and none of the existing labels in the file use shortcuts

Oh, okay. Then there's no need to add them in this PR.
But we should keep it in mind (naa, doesn't work -- I'll file a bug) to do it since buddy can help screenreaders to relate input widgets to labels, and thereby make the GUI easier to understand.
Though I'm not sure if we already provide enough info (descriptions, tooltips) to leverag that potential.

@ronso0

ronso0 commented Mar 9, 2026

Copy link
Copy Markdown
Member

Will check the new commits tomorrow..

@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

But we should keep it in mind (naa, doesn't work -- I'll file a bug) to do it since buddy can help screenreaders to relate input widgets to labels, and thereby make the GUI easier to understand.

Good to know, thanks for the context on accessibility! Happy to help with that when the bug is filed.

Comment thread src/library/dlgtrackinfo.ui Outdated
Comment thread src/library/dlgtrackinfo.cpp Outdated
Comment thread src/library/dlgtrackinfo.cpp Outdated
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

Current Look

Screenshot 2026-03-10 at 6 53 00 PM

@ronso0

ronso0 commented Mar 10, 2026

Copy link
Copy Markdown
Member

Great, no please squash the commits.

Add a QDoubleSpinBox for entering the A4 tuning frequency (Hz)
and a read-only label showing the offset from standard A440
tuning in cents to the Track Editor Summary tab.

The user can enter a tuning value in Hz (400-480 Hz range) and
the cents offset is computed and displayed automatically. The
tuning is stored in the Keys object alongside the key data and
persisted via the existing tuning_frequency_hz database column.

Complements the tuning detection in PR mixxxdj#15817.
@Swarnadip-Kar
Swarnadip-Kar force-pushed the add-tuning-field-trackinfo branch from e1b041a to 8ad1378 Compare March 10, 2026 15:15
@Swarnadip-Kar

Copy link
Copy Markdown
Contributor Author

If there are no conflicts, I always use (what I already mentioned in another PR of yours)
git rebase [first commit of feature branch]^1
in order to squash, fixup, rename, reorder etc.

Done, Squashed and Pushed

@ronso0
ronso0 enabled auto-merge March 10, 2026 15:28
@ronso0
ronso0 merged commit 7045f61 into mixxxdj:main Mar 10, 2026
17 checks passed
@Swarnadip-Kar
Swarnadip-Kar deleted the add-tuning-field-trackinfo branch March 28, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants