Woof - this is a target-rich environment.

`const int y = 1;`

This variable, y, is the increment that variable x, the phase counter, gets bumped by on each iteration. With y=1, there will be about 6 iterations per cycle. The argument to the sin() function is in radians, and a radian is about 57 degrees. With y=1, you won't get much resolution.

`const double pi = 3.14;`

There's no benefit to using an abbreviated value for pi. There's certainly no point in declaring it

*double* - which is the same as

*float* on the Arduino - and then rounding it to two decimals. If you're going to use pi, get close.

`double p1 = 0;`

This value is only used in analogWrite(), which takes a value between 0 and 255. It could be

*unsigned char*.

`p1 = 127.5*sin(x+(2*pi/3))+127.5;`

Wow! Two multiplications, two additions, and a division, all floating point, and a trig function. That's a lot for an 8-bit processor without a floating point unit. I don't know how much optimization the compiler can do with this, but there's at least some that you can do: define a

*const float* = 2*pi/3, and another = 4*pi/3, and avoid doing that division each iteration; change the additive constant to 128 and do the addition after the storing the value in an

*unsigned char* - your output would then range from 1 to 255, rather than from 0 to 255, but a lot of calculation delay would go away.

`if(x = 2*pi) x = 0;`

This won't work. Instead of a test, it's an assignment; as a test, though, it's always true. So, the program will set x to 0 on every iteration, and the analog outputs won't change.

... the brightness on each led doesn't change ...

That's why. But, if it were a working test - if it said, "if(x == 2*pi)" - it would always fail, because x has type int, and 2*pi is 6.28 or thereabouts, and you'd never get equality. If x were a

*float*, the test would still always fail, because x starts at zero and increments by 1 - it'll pass right by 6.28, and x will eventually get so big that adding 1 to it won't have any effect - the additional 1 will get rounded off in the floating point calculation. But, that would take a long time.

`delay(freqpin/10);`

This will technically work. What you'll get is a delay that ranges from 0 to about 102 milliseconds. With about six samples per cycle, that corresponds to about 600 milliseconds per cycle maximum, for a calculated minimum frequency of about 1.67Hz - actually, it'll be lower, because the loop calculations will take a long time. You won't have much control when the analog input is close to zero, since the delay will increase by a proportionally large amount with one tick of the ADC output. The maximum frequency will be determined by the speed of the loop calculations. Or, it would be if x weren't set to zero each time through the loop.

What changes(if any) should i make to the code to make it work?

Many changes. Read some tutorials about programming in C. If you're intent on using floating point, then make x and y

*float*. Set y to pi/<some integer>, and use a counter to detect the end of a cycle and reset x. Fix the test by changing = to ==. When you get it running, see whether you like the performance. Then, search "direct digital synthesis," since that's what you're trying to do here, and find a better method.