-
Notifications
You must be signed in to change notification settings - Fork 30
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
Use an actual Generator
for WaveForm
#12686
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,57 +11,41 @@ import { useId, useMemo } from 'react'; | |
* the same results, given the same seed. | ||
* | ||
* Copilot helped me with it... | ||
* | ||
* @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator | ||
*/ | ||
const getSeededRandomNumberGenerator = (seedString: string) => { | ||
function* waveformGenerator( | ||
/** the URL of the audio source file */ | ||
url: string, | ||
/** the number of bars to generate, i.e. the length of the array */ | ||
bars: number, | ||
/** a destination range for values */ | ||
[min, max]: [number, number], | ||
) { | ||
const modulus = 2147483648; | ||
/** a sort of hash of the URL */ | ||
const seed = url | ||
.split('') | ||
.reduce( | ||
(sum, character) => (sum + character.charCodeAt(0)) % modulus, | ||
0, | ||
); | ||
const multiplier = 1103515245; | ||
const increment = 12345; | ||
|
||
// convert string to numerical seed | ||
let hashedSeed = 0; | ||
for (let i = 0; i < seedString.length; i++) { | ||
const char = seedString.charCodeAt(i); | ||
hashedSeed = (hashedSeed << 5) - hashedSeed + char; | ||
hashedSeed |= 0; // Convert to 32bit integer | ||
} | ||
|
||
const seed = Math.abs(hashedSeed) % modulus; | ||
let state = seed; | ||
|
||
return function () { | ||
let count = 0; | ||
while (count++ < bars) { | ||
state = (multiplier * state + increment) % modulus; | ||
return state / modulus; | ||
}; | ||
}; | ||
|
||
/** | ||
* Compresses an of values to a range between the threshold and the existing | ||
* maximum. | ||
*/ | ||
const compress = (array: number[], threshold: number) => { | ||
const minValue = Math.min(...array); | ||
const maxValue = Math.max(...array); | ||
|
||
return array.map( | ||
(value) => | ||
((value - minValue) / (maxValue - minValue)) * | ||
(maxValue - threshold) + | ||
threshold, | ||
); | ||
}; | ||
|
||
// Generate an array of fake audio peaks based on the URL | ||
function generateWaveform(url: string, bars: number) { | ||
const getSeededRandomNumber = getSeededRandomNumberGenerator(url); | ||
|
||
// Generate an array of fake peaks, pseudo random numbers seeded by the URL | ||
const peaks = Array.from( | ||
{ length: bars }, | ||
() => getSeededRandomNumber() * 100, | ||
); | ||
|
||
// Return the compressed fake audio data (like a podcast would be) | ||
return compress(peaks, 60); | ||
/** Get a number in the range [0, 1] */ | ||
const normalised = state / modulus; | ||
/** Compress the amplitude of the fake audio data, like a podcast would */ | ||
const compressed = min + (max - min) * normalised; | ||
/** sub-pixel precision is pointless data to send over the wire */ | ||
const rounded = Math.round(compressed); | ||
|
||
yield rounded; | ||
} | ||
Comment on lines
+17
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this might be a personal thing, but although this makes for less code, i think it actually makes it harder to read and understand what's going on. a pattern like this: const complexFunctionA => () => {}
const complexFunctionB => () => {}
const c => () => {
const a = complexFunctionA();
const b = complexFunctionB();
return a + b;
} allows for progressive disclosure of the complexity. it reduces context you need to make a change, because you can focus on the bit you're interested in right now, and only need to dig into specifics (implementations) if you need to (e.g. if, in this case, compression wasn't working correctly). pulling everything into a single function means you need to understand how everything in there works before you know what you can safely ignore and what you need to look at. it's a balance for sure ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting insight — so the complexity of the PRNG “pollutes” the simplicity of the other operations: normalise & remap. I think the bit that threw me off in the beginning was that the pseudorandom numbers were not in the [0,1) range. Maybe that's all that would be needed here? In any case I'm happy to leave this in draft or even close it and let the actual team looking after decide on whether these are the right patterns. I just find generators powerful and appropriate to model these kinds of things, and wanted to share an actual example where they might help ❤️ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One last thought, would function hoisting help with progressive disclosure? If someone comes to the current module, the first piece of code they’ll encounter is actually the most complex, so you could opt instead for keeping those lower down the file. I also think (and tested) that as long as the functions are not called they could use the const generateWaveform = (url: string, bars: number) => {
const seed = seedFromUrl(url);
/** the first few bars are consistent with podcasts starting with the same intro */
const waveform = [72, 84, 96, 84, 96];
for (const pseudoRanomNumber of PRNG(seed)) {
/**
* Convert from the [0, 1) range to [60,100].
* Starting at 60 simulates audio compression.
*/
const remapped = 60 + Math.round(pseudoRanomNumber * 40);
waveform.push(remapped);
if (waveform.length >= bars) return waveform;
}
};
/** Convert a string to a numeric hash. Do not use for cryptographic purposes */
const seedFromUrl = (url: string) =>
url.split('').reduce((sum, character) => sum + character.charCodeAt(0), 0);
/** Pseudo-random number generator of numbers in range [0, 1) */
function* PRNG(seed: number) {
const modulus = 2147483648;
/** a sort of hash of the URL */
const multiplier = 1103515245;
const increment = 12345;
let state = seed % modulus;
while (true) {
state = (multiplier * state + increment) % modulus;
const normalised = state / modulus;
yield normalised;
}
} Note The above code does not work with the current target according to TS: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yeah the peril of (me) relying on machines. but this conversation (and continued offline) did lead me to https://github.com/dworthen/prng. i'm thinking there's a good case to add something from this (alea?) to this is a rabbit hole i didn't even know existed, it's fascinating! |
||
} | ||
|
||
type Theme = { | ||
|
@@ -97,7 +81,10 @@ export const WaveForm = ({ | |
...props | ||
}: Props) => { | ||
// memoise the waveform data so they aren't recalculated on every render | ||
const barHeights = useMemo(() => generateWaveform(src, bars), [src, bars]); | ||
const barHeights = useMemo( | ||
() => Array.from(waveformGenerator(src, bars, [60, 100])), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i didn't realise you could do this with generators in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👀 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
[src, bars], | ||
); | ||
const totalWidth = useMemo( | ||
() => bars * (barWidth + gap) - gap, | ||
[bars, barWidth, gap], | ||
|
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.
An alternative would be to directly return an array like so:
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 like this approach, generators v cool ofc, but this approach seems more in line with https://github.com/guardian/recommendations/blob/d7ff620f81a6f59758aded4d11b69965858c7fac/coding-with-empathy.md