Skip to content

Conversation

@H2Swine
Copy link
Contributor

@H2Swine H2Swine commented Mar 3, 2025

Hope I haven't messed up how pulls work. Wouldn't be the first time.

This intends to fix #816 , the man page wrongly states -q 16 is permitted.

I have worked on more on the man page, but I don't want to put in too much in one request.

  • Also, reworded -p for consistency with -q; -m and -M for clarity; and --lax for brevity.
  • The following should have been done in the previous round: the --help text has the tagging options right after --picture. Did so here too.
  • But, and this should either also be fixed in the help text or be rejected: --no-utf-8 is a tagging option - aren't those encoding-only? Moved and reworded, but reject as appropriate.

* max -q value was wrong, issue xiph#816 . Fixed that. 
* Also, reworded -m, -M for clarity, -p for consistency with -q, --lax for brevity. 
* This should have been done in the previous round: the --help text has the tagging options right after --picture. Did so here too. * But, and this should either also be fixed in the help text or be rejected: --no-utf-8 is a tagging option - aren't those encoding-only? Moved and reworded, but reject as appropriate.
Reworded the wording of -j, correcting the max to the actual 64.
@H2Swine
Copy link
Contributor Author

H2Swine commented Mar 5, 2025

Looks like max thread count was wrong as well. Also reworded, clarifying that default is single-thread.

man/flac.md Outdated

**-M**, **\--adaptive-mid-side**
: Adaptive mid-side coding for all frames (stereo only, otherwise ignored).
: Like -m, but adaptive choice (faster, slightly weaker compression).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe use the word heuristics here? I think that is more descriptive than adaptive. I know, the word adaptive was there already, but it is rather unclear what it means I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No objection to that. I'll look over it and - if you don't mind doing it in this thread - submit more textual suggestions. I have briefly tested picture and RG features and tried to make sense out of actual behaviour vs help text, so a question -- indeed two:

  • Could RG still leave PADDING even if --no-padding is used? I didn't get to provoke that. Looks like something similar was addressed in 1.2.1b?
  • General formatting: Should one more generally write out e.g. -w with ticks instead of -w without? I assume it would be a good thing to write flac double-asterisk-surrounded for the tool, to contrast it from the format (which likely should be FLAC in allcaps?), that is quite common manpage style?

Copy link
Contributor Author

@H2Swine H2Swine Jun 8, 2025

Choose a reason for hiding this comment

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

submit more textual suggestions.

I tried my own test branch in order not to mess up too much. @ktmf01 , mind having a look at b359d00 where I commented the changes?

... and if you think they are useful, educate me on how to link that one up to this PR. I'm clumsy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That draft nearly managed to clear up #840 if only "by collateral damage". To do when we get that far.

@ktmf01 ktmf01 merged commit 901195b into xiph:master Jul 1, 2025
18 checks passed
ktmf01 referenced this pull request Jul 2, 2025
Test to comment each change
@H2Swine
Copy link
Contributor Author

H2Swine commented Jul 6, 2025

@ktmf01 : You had a few comments on the previous draft, I worked my way up to include the Examples subsection too, yes I introduced some errors there, now rectified. And I did some choice you might not approve upon (like emphasis on error and warning, and more subsectioning), which may be reverted before I run a PR.

If/when this is ready for a PR ...

@H2Swine
Copy link
Contributor Author

H2Swine commented Jul 31, 2025

Time to try to finish it. A comprehensive rewrite, where the first part now looks more like xz man page style - whether that is an improvement is anyone's say I guess, but the information content was overdue for an overhaul.

H2Swine@a555fe1#r163204042 comments on those changes that are new since H2Swine@1639bed . There are still a couple of questions up on changes that could have been taken on board.

Again: if/when this is ready for a PR ... now it is so much that I guess, not only "if/when @ktmf01 has the time to review whether a PR based on that first part is even a good idea", but also what details are still valid when 1.5.1 is shaping up. (Example: the document does say that as of 1.5.0, treatment of the ".aifc" file extension is inconsistent, and maybe that part of is obsoleted by 1.5.1.)

Edit: Now also with main.c updated to reflect the changes and fixes: 51b21f7#r163615829 . Of course I overlooked a couple of mistakes when doing that, so that comment links to an update.

@ktmf01
Copy link
Collaborator

ktmf01 commented Aug 14, 2025

I have completely lost track of what you're doing, mixing up too many things. Please just file a PR already, because I keep forgetting where to find your changes.

And if possible, please split uncorrelated stuff up. Maybe first fix things, then after we merge that, do some reordering. Or the other way around.

Tremus pushed a commit to Tremus/flac that referenced this pull request Sep 19, 2025
* max -q value was wrong, issue xiph#816 . Fixed that. 
* Also, reworded -m, -M for clarity, -p for consistency with -q, --lax for brevity. 
* move the tagging options right after --picture.
* moved and reworded --no-utf-8 is a tagging option
* Correct -j description
Co-authored-by: Martijn van Beurden <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Documentation - incorrect range of qlp-coeff-precision parameter

2 participants