Search code examples
crefactoringjpegmjpeg

How can I split this function into smaller functions without sacrificing its functionality?

I got the code for MJPEG decoding from here and I am trying to split the code for IDCT into smaller functions.

The IDCT function in the original code is as follows:

void IDCT(int32_t *input, uint8_t *output) {
    int32_t Y[64];
    int32_t k, l;

    for (k = 0; k < 8; k++) {
        for (l = 0; l < 8; l++) Y(k, l) = SCALE(input[(k << 3) + l], S_BITS);
        idct_1d(&Y(k, 0));
    }

    for (l = 0; l < 8; l++) {
        int32_t Yc[8];

        for (k = 0; k < 8; k++) Yc[k] = Y(k, l);

        idct_1d(Yc);

        for (k = 0; k < 8; k++) {
            int32_t r = 128 + DESCALE(Yc[k], S_BITS + 3);
            r = r > 0 ? (r < 255 ? r : 255) : 0;
            X(k, l) = r;
        }
    }
}

More details of the functions can be found in this link.

I was able to further break this code down in the following way:

In the X-direction:

void IDCTforX(int32_t *input, uint8_t *output) {

    int32_t Y[64];
    int32_t k, l;
    int32_t Yc[8];

    for (k = 0; k < 8; k++) {
        for (l = 0; l < 8; l++)
        {
            Y(k, l) = SCALE(input[(k << 3) + l], S_BITS);
        }
    }
}

void IDCTfor1dim(int32_t *input, uint8_t *output)
{
int32_t Y[64];
    int32_t k, l;
    int32_t Yc[8];

    for (k= 0; k < 8; k++)
    {
        idct_1d(&Y(k, 0));
    }
}

In the Y-direction:

void IDCTforY(int32_t *input, uint8_t *output) {

int32_t Y[64];
int32_t k, l;

for (l = 0; l < 8; l++) {
    int32_t Yc[8];
        for (k = 0; k < 8; k++)
        {
        Yc[k] = Y(k, l);
        }

        idct_1d(Yc);

    for (k = 0; k < 8; k++) {                   
    int32_t r = 128 + DESCALE(Yc[k], S_BITS + 3);
    r = r > 0 ? (r < 255 ? r : 255) : 0;
    X(k, l) = r;
    }
}

The code for DESCALE is as follows:

static inline int32_t DESCALE (int32_t x, int32_t n)
{
    return (x + (1 << (n - 1)) - (x < 0)) >> n;
}

Re-organizing IDCT in the manner shown above is giving me the same output as the original code. However, after re-organizing the code for IDCTforY in the following way I got a blurred image:

 void IDCTforY(int32_t *input, uint8_t *output) {

 int32_t Y[64];
  int32_t k, l;
int32_t Yc[8];

  for (l = 0; l < 8; l++) {

   for (k = 0; k < 8; k++)
    {
    Yc[k] = Y(k, l);
    }

    idct_1d(Yc);
    }
    //Running the loop for de-scaling separately....

  for (l = 0; l < 8; l++) {
  for (k = 0; k < 8; k++) {   
   int32_t r = 128 + DESCALE(Yc[k], S_BITS + 3);
    r = r > 0 ? (r < 255 ? r : 255) : 0;
    X(k, l) = r;
   }
    }
}

My output frames looks like this with the above code:

enter image description here

What is the meaning of a blurred image in JPEG decoding?

How can I split IDCTforY in such a way that the nature of my code doesn't get compromised?

Solution

  • The function IDCT() declares the array Y[] which transfers data across all for-loops. In your refactored code every function declares its own Y[] array. The same error you did with the Yc[] array. Make the arrays global and see if the code runs.

    Edit 2017_08-28

    Give Yc[] an extra dimension:

    void IDCTforY(int32_t *input, uint8_t *output) 
{
  int32_t Y[64];
  int32_t k, l;
  int32_t Yc[8][8];

  for (l = 0; l < 8; l++) 
  {
      for (k = 0; k < 8; k++)
          Yc[l][k] = Y(k, l);
      idct_1d(Yc[l]);
   }

   //Running the loop for de-scaling separately....

   for (l = 0; l < 8; l++) 
   {
       for (k = 0; k < 8; k++) 
       {   
           int32_t r = 128 + DESCALE(Yc[l][k], S_BITS + 3);
           r = r > 0 ? (r < 255 ? r : 255) : 0;
           X(k, l) = r;
       }
   }
}

    Edit 2017-08-29

    I cannot explain the optical effect, but you broke the data flow. The original code was like this:

    for (l = 0; l < 8; l++) 
{
    int32_t Yc[8];

    Fill(Yc);

    idct_1d(Yc);

    Descale_and_WriteOut(Yc);
}

    You made of it:

    int32_t Yc[8];
for (l = 0; l < 8; l++) 
{
    Fill(Yc);
    idct_1d(Yc);
}
for (l = 0; l < 8; l++) 
{
  Descale_and_WriteOut(Yc);
}

    You see, that only the result of the last iteration of the input-and-process-loop is passed to the output-loop. I gave every l-iteration own memory in Yc[][].