-
-
Notifications
You must be signed in to change notification settings - Fork 103
Overhaul audio section #350
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
Conversation
71ee168
to
5b39681
Compare
I think the "Common concepts" section should precede everything else, otherwise, really good job! |
Thanks! It's not possible to talk about any of these concepts without introducing the architecture (e.g. channels). That said, maybe moving the "APU" section before the architecture would indeed be better? I also want to introduce the diagram early, so that it can serve as a visual aid to go back to for the rest of the page. |
b51f3b6
to
e595472
Compare
Minor trivial feedback: use either "wave length" or "wavelength" consistently; you're currently using both. |
As per #350 (comment), it should be "wavelength". That should be added to the document style, and it'll have to be caught during review due to size of the change. |
Hi @ISSOtm did you have any chance to come back on this? I will be fixing the conflict/rebasing, splitting it and merging what we have if no activity is expected in the near future as I don't want this to become blocking or get lost in time |
I am not planning to work on this in the foreseeable future due to being busy on many other things (as you know), but I can provide guidance to anyone else willing to pick this up. I don't think it's production-ready in its current state due to how incomplete it is—it doesn't have all the info the page it's meant to replace has. Most of the info is available though not necessarily readily, if only from the amount of people writing GB emulators by the day. |
Let's not let it get lost to time. :) |
df312e9
to
d163cf8
Compare
I fixed issues and feedback on the "Audio" and "Sound registers" pages, I'll work on the "Audio details" now, but feedback on the first two would be appreciated. Thanks! |
3468fde
to
6180889
Compare
Co-authored-by: Antonio Vivace <avivace4@gmail.com>
Pushed changes we agreed upon, and rendered the draft at http://eldred.fr/pandocs/Audio.html. |
Bump. I'd really like to merge this soon, please. It's going to become a blocker for other projects of mine soon. |
I can see some threads here still waiting for your follow up.. can you take a look? |
I don't see any of the open threads currently waiting on me, can you give me some links? |
This one has been changed: Line 93 in a4049ec
...and I'm waiting for approval or further modification requests on the updated text. |
A number of very interesting threads were opened by @nitro2k01, for some of which I feel they should've been blocking, but this PR has been sitting here for more than a year and I really don't feel like it's progressing in any sensible way since months. Exceptionally, I "moved" those conversations into issues #444 #445 #446 (and in a minor fashion #194) and I'm going to merge what we have now in this chapter, which is really a block of awesome work and information previously scattered unorganically around. Thanks @ISSOtm and @nitro2k01 ! Let's try to keep the conversation going in those issues and open more PRs to iterate upon this content. |
I had forgotten to force-push, oops :(
Waaay not finished, only posting for discussion and early review. (Anything changed and not marked with a TODO is intended to be final.)
I won't be requesting any reviews until it's finished, but please feel free to chime in so as to make the process piecemeal as possible :)
TODO:
Fixes #347 and fixes #119.
An issue that @LIJI32 raised is the time units. Currently, I only use "APU ticks", being 2 MiHz (the smallest common denominator), and provide measurements in multiples of those. Liji instead suggested introducing separate units whenever necessary (e.g. "1 128 Hz tick" instead of "16 [APU] ticks"). The pro is that the units make more sense (no multiples of powers of 2), the con is potentially more mental complexity.I'm not attached to either, so I used the solution I originally came up with in this PR, please feel free to discuss and provide feedback in the comments.Other names for the time unit were suggested: "sample" (given that 2 MiHz is the final sample rate), "boop", maybe others I missed?