-
Notifications
You must be signed in to change notification settings - Fork 326
flac.md : Some more fixes: errors, language. #845
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
base: master
Are you sure you want to change the base?
Conversation
By request, I have not submitted a full proposal, thus some "Everywhere" changes are not at all (yet) implemented everywhere. **Everywhere / general** - boldfaced **flac**-the-tool, capitalized FLAC-the-format - italicized *warnings* and *errors*. - What in the text was Warning-but-not-a "*warning*", is now "CAUTION:" (4 instances in this commit). "Note:" is now "NOTE:" Both start on newline. - Tried to stick to more consistent phrases about "print" a *warning*, and "exit with" an *error* (when it actually exits), and file already "exists" rather than a mix between "is present" and whatnot. - Got rid of a bunch of "specify"/"specifies", when the use was far from a _specification_ as in the FLAC specification or PICTURE specification. I just imagine that someone who text-searches for specification will be more interested in those which are left. Apart from those, here are some of them explained by referreing to line numbers in *current* ("old") flac.md; also some possible changes that I have **not** taken on board (yet). Ignoring those listed above, and some I think are self-explanatory, and also skipping justification for some that have gotten a thumb up from @ktmf01 at xiph@b359d00 - 182: Bugfix! Previous change of mine (my blame!) couldn't stick consistent on "Track04" vs Track07 ... - also: rewritten. - 205-206: It does not "exit" with an error if processing multiple files. And the "already exists". - 222: Tried to improve language. - 231: Rewritten. - 245: As discussed earlier: Referring to the **Format options** - 273 deviates from a suggestion from @ktmf01 - "print" a *warning* etc. - 282: I didn't like "force" ... cf -f. - 298: Longer explanation. - 312: Elaborates. Written to fit \--until. - 321: **FIX!** The syntax was wrong. Hopefully it is now right. - 322: Admittedly, this is now written in more detail *because* I first got it wrong. - 328 as mentioned earlier: "mode" is better reserved for encoding mode, decoding mode, ... - 335: Statement is not completely correct, but I put a reservation in the `--apply-replaygain-which-is-not-lossless` text. - 340: Another ref to **Format options** - 342: Elaborates. With CAUTION:. - 344: There are exceptions, at least \--apply-replaygain-which-is-not-lossless. - 366 not yet done: Could need a rewrite. Have it drafted. - 376: Encoding starts here. No need to mention the tukey, but something about format choice ... - Not yet done: I have a draft with level-3 headlines for compression/audio options and for metadata options. - 378: -V moved downwards. - 410 to 421: Have been discussed. Mostly, the "specifies" ... and then re-wrote. - 438: Reworded. **QUESTION:** Since it did say the wrong comma-separated, should it now outright say semicolon-separated rather than just removing it? - Up to 463: I think these have been discussed. - 464-ish: Moved `--ignore-chunk-sizes` here, it is audio ... hopefully. - 484: ReplayGain is tags, moved down. Also: I couldn't provoke the warning about extra padding - omitted it, I don't think it is really necessary. - 498 to 511: Moved - and are now longer. Please read through. - 516: Do I understand correctly that -T ... --no-utf-8 -T ... will apply no-utf-8 to the latter and not the first tagging? - 529: I don't think it has to say anything about UTF-8 when that is said under -T and this is like -T except ... - 537: Rewritten ... - 554: -P is rewritten to get it on the format where it first says what it *does*. - 605: Elaborated on `--force-raw-format` after testing. In particular that you should usually omit it when encoding. - 610 ff: Clearer what is input/output, rid of "Specifies", make clear that \--bps must be multiple of 8 - 650: Also to negate defaults ... if that wasn't obvious. - 672 ff: I have drafted extensive changes from here on, but for now I am sticking to fewer changes. - 720 ff: 0 to to are now taken from the RFC. - 789 fixed the comma/semicolon error. (I have drafted a full rewrite, but not here). - But: "subdivide_tukey(1/P)" is not implemented ... should be mentioned? - 827: Useful place to put the info on where it is maintained?
Comments to #818 has lots of loose ends on this, also links to drafted
But for now: not doing everything at once. |
-V -j 2 -m -3 -f -o Track07.flac , or as -foTrack07.flac -3mVj2. | ||
The `-- ` (with whitespace!) signifies end of options, treating | ||
everything to follow as filename; that is needed when an input | ||
filename could otherwise be read as an option, like this "-7". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a double space at the end of this line. Is that intentional? It renders correctly in the HTML, but not in the manpage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some were intended to be for line breaks, some might have been mishaps. Feel free to overrule for this or that reason.
another. Make sure output volume is limited, as corrupted audio can | ||
generate loud noises. | ||
: Decode abc.flac to abc.wav, not aborting upon *error*. Potentially | ||
useful for recovering as much as possible from a corrupted file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a double space here.
from the picture file); see subsection **Picture specification**. | ||
Currently the **flac** encoder handles up to 64 \--picture options; | ||
for more, add afterwards (**metaflac** or tagging software). | ||
NOTE: The FLAC format is limited to 16 MiB *total* metadata. Also, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't true. The limit is 16 MiB per block (so, in this case, per picture)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I don't think it is necessary to mention that using a large number of pictures might cause some programs to reject the files. The same is true of other kinds of metadata as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good, ... you just remove the entire "NOTE:" then?
options, all tags in the input file will be ignored, not only those | ||
set with -T / \--tag. | ||
: Set a FLAC tag; like Ogg, the FLAC format uses Vorbis comments, see | ||
the format specification section 8.6. Quote content as necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use RFC 9639 here for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All fine with me.
**-S** {\#\|\#x\|\#s\|X}, **\--seekpoint**={\#\|\#x\|\#s\|X} | ||
: Sets seekpoint(s), overriding the default choice of one per ten seconds | ||
('-s 10s'). When several -S options are given, the resulting SEEKTABLE | ||
will contain all the seekpoints (duplicates removed), max 32768. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that number is correct. Where did you get it from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is wrong, it doesn't matter where I got it from.
I don't quite remember but it could have been from (mis-)reading https://xiph.org/flac/changelog.html . It is mentioned for the 1.3.2 release.
Thanks for all the work you did on this one. And thanks for chopping this up for me. It is still quite a large PR (for documentation anyway) but doable. I'd like to remove the double spaces (i.e. line breaks) because the manpage format cannot handle those. After discussion this and the other comments, I will merge. |
If there is any principle on when (not) to use double space, please let me know - (or) there will be more in the next PR. Edit: Also if there are other "basic Markup" features that cannot be used. In particular: level 4 headlines? (Intended to use for "example:" headings. Take "Specification examples: " at the end of "Picture specification", now just boldfaced.) (I assume you want a new PR and not any sort of revision?) |
It seems a double space (i.e. a line break) in an indented region (following the : ) doesn't render correctly in man.
I don't know whether that works or not.
No, you (or I for that matter) can just add a commit. That way I can easily check what you have changed. When merging, I can squash the commits into one. So, then the PR has the complete history, but the end result is just a single commit. |
Added an example as "level 4" heading at the end of the apodization functions. - The example is meant as a suggestion to the text, but - The "level 4" heading is meant as a test.
"doesn't render correctly" is kinda ambiguous as to whether it does anything wrong. If it just doesn't work as intended but stays a double-space without doing any harm, then what?
We'll see - if you can see it from last hour's commit, I followed-up on this:
Like I just did? H2Swine@916f1f4
But, for "next time": Should I wait until previous discussions are fixed? I can surely reword the PICTURE text (and SEEKTABLE too) at that stage. |
What I mean is that when you use a double space at the end of a line, you get a linebreak. That works perfectly fine. However, when you use it in an indented region, the stuff after the line break is still indented in html, but not in man. That is what I mean with "does not render correctly".
Yes
Yes for html, not really for man. There is only so much you can do with formatting in a terminal.
Not really. At least, not in this way, as you can see in the screenshots.
I think it is too much, too specific for a manual.
Wait with what? I don't understand what you mean. |
Yeah, but ... consequences? What is the least bad overall? Say a "hierarchy of emphasis" like (with attempted capitalization for illustration): "LEVEL 1", "LEVEL 2", "Level 3", "level 4", "just boldfaced text". On a monochrome console output, there will be at least some that are just too close to each other to distinguish visually. But if that is unavoidable, does it make anything wrong? It looks like the only distinction between "Level 3" and "level 4" headline will be that I manually have capitalized the first letter in "Level 3", but that does not mean that there will be any bigger visual clue if I use "just boldfaced text".
Say: Is the indentation in the picture you showed due to the quadruple-hash level 4 heading - or due to interpreting the "-A" as beginning of an item? In the latter case, it might be better then not to have the linebreak and rather write (on one line):
... "might" as in: I don't know.
I think it works better than not having it there? Apart from the possible issue of "-A", at least. Edit: forgot to mention that in those screenshots, I had forgotten a double space before the first item.
I could shorten it down surely - but then someone (and that is devs, not me) should also consider the usage examples at the top before the options. Those are long in total, and yes I was the one who put a long one there, but that could be sacrificed sure: up there is more noise than at the end where hardly anyone reads. (And I have raised the question: should we even have a Considerations up to devs.
There were items where the discussion was not (or "maybe was not") concluded. I could have
What fits your workflow? |
Nothing, really. Just try to use them sparingly.
Yes, with indentation
The problem is that pandoc is more picky in the markdown formatting it accepts. It only works when the itemized list has an empty line before and after it. It is not the only markdown renderer that does this, GitHubs one is more forgiving in quite a few cases. Anyway, I pushed a commit to fix this. You can look at the diff to see the differences.
I'd like to get as much as this merged before diving down into specifics. So yes, don't touch them now, get this merged, and then have a new PR with the rest. That way, I don't have to check the simple ones over and over, because I keep forgetting after 2-3 weeks. |
... by removing them when rendering to man
Trying to get both itemized list and the PICTURE TYPE "numbered list" (starting at zero) to look decent in man and markdown.
OK ... for this commit, I fiddled around with pandoc (do the options matter?
If so, the examples would shape up as the commit just made, where I
Edit: I said "later", let me stick to "later". |
I made myself a "to do" list (including, "have drafted" and "maybe to decide against doing"), and have noticed some glitches where fixes are already drafted. Also drafted: new apodizations part, so you could largely disregard how it looks as per previous commit, apart from taking it as a test on formatting. But have a look at the PICTURE specification subsection, as that may content-wise be close to a proposal (I'll fix a typo "wil" -> "will"), and also hopefully the formatting now works although that does not mean it is the most suited. Next up I think I will provisionally propose the following formatting changes to the PICTURE spec:
Suggested then, that a final decision on headlines structure could wait until the whole (/larger parts of the) document is reviewed, and then I could undo "unwanted" levels. |
By request, I have not submitted a full proposal, thus some "Everywhere" changes are not at all (yet) implemented everywhere.
Everywhere / general
Apart from those, here are some of them explained by referring to line numbers in current ("old") flac.md; also some possible changes that I have not taken on board (yet). Ignoring those listed above, and some I think are self-explanatory, and also skipping justification for some that have gotten a thumb up from @ktmf01 at b359d00
--apply-replaygain-which-is-not-lossless
text.--ignore-chunk-sizes
here, it is audio ...--force-raw-format
after testing. In particular that you should usually omit it when encoding.