current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 2 down vote favorite
1

Here is the code I am using to apply DCT on 8x8 blocks, which I'm not sure is correct. I have tried to follow Wikipedia's discrete cosine transformation (DCT) formula as closely as possible. Please let me know if there are any changes to be made.

//round up a float type and show two decimal places
double rndup(double n)
{
  double t;
  double temp;
  t=n-floor(n);
  if (t>=0.5)    
  {
          n*=100.0;//where n is the multi-decimal float
          n=ceil(n);
          temp = floor(n);
          temp = n - temp;
          n-=temp;
          n/=100.0;
          }
  else 
  {
          n*=100.0;//where n is the multi-decimal float
          n=floor(n);
          temp = floor(n);
          temp = n - temp;
          n-=temp;
          n/=100.0;
  }
  return n;
}  
//two functions that work together to apply DCT to a 8x8 block
void a_check(int u,int v, double *Cu, double *Cv)
{
if (u == 0) *Cu = sqrt(1.0 / 8.0); else *Cu = sqrt(2.0 / 8.0); 
if (v == 0) *Cv = sqrt(1.0 / 8.0); else *Cv = sqrt(2.0 / 8.0); 
}
void applyDCT(double *DCTMatrix, double *Matrix,int x, int y)   //arguments: DCTMatrix = matrix where DCT values go, Matrix = input Matrix which needs to be processed
{
double Cu=0,Cv=0,cos_x=0,cos_y=0, DCT_value = 0;
for(int u=0;u<8;u++)
{
    for(int v=0;v<8;v++)
    {
        a_check(u,v,&Cu,&Cv);
        cos_x = cos( (3.14/8) * (double)u * ( (double) ( (x+u) + (1.0/2.0))));
        cos_y = cos( (3.14/8) * (double)v * ( (double) ( (y+v) +(1.0/2.0))));
        DCT_value = Cu * Cv * Matrix[u*8 + v] * cos_x * cos_y;
        DCT_value = rndup(DCT_value);
        DCTMatrix[u*8 + v] = DCT_value;
    }
} 
}
share|improve this question
1  
It will be easier to answer your question if you include a link to the Wikipedia article you mentioned, and describe which formula from the article you are trying to implement – almaz Dec 1 '12 at 23:02
up vote 6 down vote

I'll comment on readability as it could use some serious improvements.

  • Your indentation is very inconsistent. Sometimes you indent two spaces, four spaces, eight spaces, or no spaces. Choose a style and keep it consistent throughout the program.

    Two common styles (using four spaces):

    void func()
{
    if (isTrue)
    {
        // ...
    }
}


    void func() {
    if (isTrue) {
        // ...
    }
}

  • Your use of whitespace is also inconsistent. It'd be more readable to separate all operators and operands with one space and keep those consistent throughout the program.

    Don't do this:

    int number=1+2+3;

    Do this:

    int number = 1 + 2 + 3;

  • This comment:

    //where n is the multi-decimal float

    can just be placed above the function. There's no need to put it in multiple places.

  • The comment next to applyDCT() should probably be placed on top of the function instead. A comment that long should either be relocated or broken up into separate-lined comments.

  • These:

    if (u == 0) *Cu = sqrt(1.0 / 8.0); else *Cu = sqrt(2.0 / 8.0); 
if (v == 0) *Cv = sqrt(1.0 / 8.0); else *Cv = sqrt(2.0 / 8.0);

    are not very readable nor maintainable. Prefer to apply curly braces, especially if you'll need to add to any of the conditions:

    if (u == 0)
{
    *Cu = sqrt(1.0 / 8.0);
}
else
{
    *Cu = sqrt(2.0 / 8.0);
}

// the same applies to the second aforementioned line

  • Do not declare/initialize variables on the same line:

    double Cu=0,Cv=0,cos_x=0,cos_y=0, DCT_value = 0;

    This is also not very readable nor maintainable, especially if you'll ever need to add comments or modify individual variables.

    Instead, have them on their own line:

    double Cu = 0;
double Cv = 0;
double cos_x = 0;
double cos_y = 0;
double DCT_value = 0;

    Moreover, in C++, you should try to use variables as close in scope as possible (next to where they will first be used). This will reduce the need for long lists of variables like above.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.