-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"--explain" does not state what can be maximal allowed value for "--max-lpc-order" #699
Comments
(edit: messed up tabs in attachment. New uploaded) Yeah ... the explain text has not been the top priority, and options that were long gone since at least 1.1.4 were not removed from the text until 1.4.0. Still it refers to https://xiph.org/flac/documentation_bugs.html which is obsolete for more than seven years. More broadly, one could discuss - or just make an executive decision on:
I took the liberty to re-draft the whole thing. Re-ordered with General options, then Encoding options, etc. Hopefully got most things factually correct. Because I would surely make a mess out of things by trying to make a pull request, I am simply attaching it here. In "the right margin", to the right of the 80th character, I put some remarks preceeded by %%%. Comments, anyone? |
Thanks for your suggestions @H2Swine, it indeed seems long overdue. Could I perhaps persuade you to try to make a PR out of this? Makes comparing and commenting a whole lot easier. I'll walk you through it.
If you could do that, that would be great! As a bonus, you'll get properly credited of course. Note: depending on your github privacy settings, a 'github noreply' address will be set as the commit email. Maybe you'll want to review that first. |
Didn't look too intimidating, let's see if I've made it as far as to step 6? H2Swine@bbc6e7f Input requested on what is not included in current --explain text, that should be? EDIT: I could have updated myself, -j is already in. Those commented as undocumented should stay so, including I see that the --help file could use some help too, but ... old |
Yes, that looks like what I had in mind. Note that after you've created a PR (step 7), you can still add more commits to that branch. So, creating a PR is not the final step per se. Considering Some more comments (not all):
I don't know, I don't like allcaps in general. The old FLAC spec used to be full of ALLCAPS but I've removed them all. It is probably useful in a terminal text without any other form of emphasis, but I think FIXED and SUBSET are words not usually needing emphasis. I think placeholders like INPUTFILE are good to have ALLCAPS, and names, extensions and abbreviations like WAVE, FLAC and PCM.
I'm not sure. Currently we have 4 levels of help text:
For each boundaries have to be set. Now that I think about it, maybe
Same issue: the manpage is meant to be exhaustive, the common help is meant to document most options succinctly... What is the explain option meant to do? |
Maybe the explain text should be removed and the -H option should display the file https://github.com/xiph/flac/tree/master/man/flac.md?plain=1 verbatim? Maybe with a few tweaks like removing emphasis or backslashes? That will add quite a bit of weight to the binary though. |
Yeah, good idea consider first whether it makes sense to have that explain text at all. If the executable is to be kept slim, could the "man page" (whether it will stay that way) be distributed as a separate file? |
That is already the case. See flac-1.4.3-win.zip, it has a 'manuals' directory with them in markdown format. On unix systems, the manpage is in the ancient groff format, which can be read by the It would be most helpful if on Windows they could be compiled into a |
For the novice user who actually needs it, .html is better than .md. But I wonder if the .md is just fine if you replace away the escape slashes and the formatting - is it feasible in order to maintain just one? And the EXAMPLES could get an indent if that solves anything. But what is then the sane workflow? Have a look at the --help text and work with the .md files for it to replace the --explain text? (Also I think there is a double asterisk missing in flac.md : Line 608 in cfe3afc
|
The problem with removing the backslashes is that it renders wrong when someone views it with an actual markdown editor or viewer. Also, as we already use pandoc to convert to groff, converting to html isn't too much work to automate. |
I didn't think of removing it from the document. I thought, if you want a plaintext output then you could code it as "fetch md file, query replace, view it to user. But whatever is most convenient ... ... which is: just don't work with the --explain text until further notice? |
Anyway, if I am to look into the --help text instead of the --explain text, I'd anyway like to get the facts right. From common sense and text-searching main.c I have the following hunches:
I'm ignoring the "Be sure to read the list of known bugs at:" |
I don't know the answers yet, I'll have to find some time to check the code. The manpage says the following
But I'll have to check of course |
It only states that: "Max LPC order; 0 => only fixed predictors. Must be <= 12 for Subset streams if sampleate is <=48kHz"
No mention of maximal value 32.
Maximal value for --qlp-coeff-precision is not stated too.
Also description for --rice-partition-order seems to be a bit incorrect: "<...> the default is -r 0 <...>".
But "the default" is -r 5 actually.
FLAC 1.4.3
The text was updated successfully, but these errors were encountered: