Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is code I need to maintain. I'm trying to make this as an example for code gone bad, for C programmers going to C++.

Please provide any comment you can. The original code is about 40 pages long, so I just gave this "small example".

void Algo::compute( Image* pInImg, Image* pImg, Image* deBugImg)
{
    int ind_r, ind_c, ind_app_r, ind_app_c , ind_g_r, ind_g_c;
        int Gv_approx[7][7],  Gh_approx[7][7], Bv_approx[5][5], Bh_approx[5][5], Rv_approx[5][5], Rh_approx[5][5];
        int Gv_clip_approx[7][7],  Gh_clip_approx[7][7];
        int HH_h, HH_v;
        int Yv_approx[5][3], Yh_approx[3][5],  Uv_approx[5][3], Uh_approx[3][5], Vv_approx[5][3], Vh_approx[3][5] ; 
        int R_ahd[5][5], G_ahd[5][5], B_ahd[5][5];
        int Y_ahd[5][5], U_ahd[5][5], V_ahd[5][5];
        int w_v_q, w_h_q;
        int avg_gr_q=0, avg_gb_q=0, avg_G_q, avg_G3_q, G_std_q, G3_std_q, Gr_std_q, Gb_std_q, AvgG_std_q;
        int avg_G3_q_rm1, avg_G3_q_rp1, avg_G3_q_cm1, avg_G3_q_cp1, G3_std_q_rm1, G3_std_q_rp1, G3_std_q_cm1, G3_std_q_cp1;// Research checkers FCC
        int std_prec = (1<<HWP_MSB_STD)-1;
        int HL_h, HL_v,HW_h, HW_v, HW_h2, HW_v2;
        int HL_hq, HL_vq,HH_hq, HH_vq, wh_quant;
        int correction_coef_q; 
        int avg_Uv, avg_Uh, avg_Vv, avg_Vh, avg_Cvq, avg_Chq;
        int coring_shift;
        int* ByerrBufferInLine;
        int* ByerrBufferInLine_min1;
        int* ByerrBufferInLine_min2;
        int* ByerrBufferInLine_pls1;
        int* ByerrBufferInLine_pls2;
        int margin_h = 5, margin_v = 0,buf_r;
        int MaxPxlVal = (1<<HWP_PXL_BIT)-1;
        int valinlog, MSB, MSB2;
        int OLD_VERSION = 0;
        /// DBG
        std::array<int, 1> x_vect = {2310};
        std::array<int, 1> y_vect = {1723};

        FILE *fp_log=fopen(m_OutputFileName.c_str(),"wt");
        int w_clp_array[7], w_clp_array_even[7], w_clp_array_odd[7];

        // define config units
        Prepare_Config_Unit_DIAG1();
        Prepare_config_Unit_DIAG2();
        Prepare_config_Unit_HF();
        Prepare_config_Unit_DIAGSAT();

        // produce corinng threshold calculation outside main loop
        int Corin_TH_array[4096];
        for(int ii = 0; ii<4096; ii++)
        {
            if(ii>1)
            {
                MSB = (int)(log((float)ii)/log(2.0));
                valinlog= ii - (1<<MSB);
                if(valinlog>0)
                {
                    MSB2 =(int)(log((float)valinlog)/log(2.0));
                    Corin_TH_array[ii] = MSB + (MSB2 == (MSB-1));
                }
                else
                {
                    Corin_TH_array[ii] = MSB;
                }
            }
            else
            {
                Corin_TH_array[ii] = 0;
            }
        }

        for( int row=0 + margin_v; row < m_Height - margin_v; row++){
                // define rellevant lines' pointers. Our restriction that inout buffert have full quads only => even number of lines
                buf_r = min( max(row-2,abs((row-2)& 1)) , m_Height - 2 + abs((row-2)& 1) );
                int* ByerrBufferLine_min2 = pInImg->m_R[buf_r];
                buf_r = min( max(row-1,abs((row-1)& 1)) , m_Height - 2 + abs((row-1)& 1) );
                int* ByerrBufferLine_min1 = pInImg->m_R[buf_r];
                buf_r = min( max(row,abs((row)& 1)) , m_Height - 2 + abs((row)& 1) );
                int* ByerrBufferLine = pInImg->m_R[buf_r];
                buf_r = min( max(row+1,abs((row+1)& 1)) , m_Height - 2 + abs((row+1)& 1) );
                int* ByerrBufferLine_pls1 = pInImg->m_R[buf_r];
                buf_r = min( max(row+2,abs((row+2)& 1)) , m_Height - 2 + abs((row+2)& 1) );
                int* ByerrBufferLine_pls2 = pInImg->m_R[buf_r];
                //// DBG
                buf_r = min( max(row+3,abs((row+3)& 1)) , m_Height - 3 + abs((row+3)& 1) );
                int* ByerrBufferLine_pls3 = pInImg->m_R[buf_r];
                buf_r = min( max(row+4,abs((row+4)& 1)) , m_Height - 4 + abs((row+4)& 1) );
                int* ByerrBufferLine_pls4 = pInImg->m_R[buf_r];
                buf_r = min( max(row-3,abs((row-3)& 1)) , m_Height - 3 + abs((row-3)& 1) );
                int* ByerrBufferLine_min3 = pInImg->m_R[buf_r];
                buf_r = min( max(row-4,abs((row-4)& 1)) , m_Height - 4 + abs((row-4)& 1) );
                int* ByerrBufferLine_min4 = pInImg->m_R[buf_r];
            for( int col=0 + margin_h; col < m_Width - margin_h; col++){

                ////////////// DBG
                //if(DEBUG_MODE)
                //{
                    int log_this_pix=0;
                    for(int dbg_indx = 0; dbg_indx < x_vect.size(); dbg_indx ++)
                    {
                        if((fp_log != NULL) && (((col == x_vect[dbg_indx] && (row == y_vect[dbg_indx]) ))))
                        {
                            log_this_pix = 1;
                            break;
                        }
                    }
                    if(log_this_pix)
                    {
                        fprintf(fp_log,"org_pix %d x- %d y- %d",ByerrBufferLine[col], col, row);
                        if(FC(row,col)== Green)
                        {
                            if(FC(row,col-1)== Red)
                            {
                                 fprintf(fp_log,"\tpixel GREEN RED\n");
                            }
                            else
                            {
                                 fprintf(fp_log,"\tpixel GREEN BLUE\n");
                            }
                        }
                        else if(FC(row,col)== Red)
                        {
                             fprintf(fp_log,"\tpixel RED\n");
                        }
                        else
                        {
                             fprintf(fp_log,"\tpixel BLUE\n");
                        }
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_min4[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_min3[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_min2[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_min1[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_pls1[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_pls2[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_pls3[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                        for(int dbg_x_shft = col-4; dbg_x_shft<col+4; dbg_x_shft++)
                        {
                            fprintf(fp_log,"%d\t", ByerrBufferLine_pls4[dbg_x_shft]);
                        }
                        fprintf(fp_log,"\n");
                    }
                //} 
                ///////////////////////////////
                // Patch statistics calculation.
                //////////////////////

                // Calculate statistics 

                if(FC(row,col)== Green)
                {
                    if(FC(row, col-1)== Red)
                    {
                        avg_gr_q = (455*(int)(ByerrBufferLine_min2 [col-2] + ByerrBufferLine_min2 [col] + ByerrBufferLine_min2 [col+2]
                                                +ByerrBufferLine[col-2]       + ByerrBufferLine[col]       + ByerrBufferLine[col+2]
                                                +ByerrBufferLine_pls2[col-2]  + ByerrBufferLine_pls2[col]  + ByerrBufferLine_pls2[col+2]))>>12;
                        avg_gr_q = CLIP_VAL(avg_gr_q, 0, MaxPxlVal);

                        avg_gb_q = ((int)((ByerrBufferLine_min1[col-1] + ByerrBufferLine_min1[col+1]
                                             + ByerrBufferLine_pls1[col-1] + ByerrBufferLine_pls1[col+1] + 2)>>2));
                        avg_gb_q = CLIP_VAL(avg_gb_q, 0, MaxPxlVal);
                    }
                    else
                    {
                        avg_gr_q = (int)((ByerrBufferLine_min1[col-1]  + ByerrBufferLine_min1[col+1]
                                            + ByerrBufferLine_pls1[col-1]  + ByerrBufferLine_pls1[col+1] + 2)>>2);
                        avg_gr_q = CLIP_VAL(avg_gr_q, 0, MaxPxlVal);
                        avg_gb_q = (455*(int)(ByerrBufferLine_min2[col-2] + ByerrBufferLine_min2[col] + ByerrBufferLine_min2[col+2]
                                                + ByerrBufferLine[col-2]      + ByerrBufferLine[col]      + ByerrBufferLine[col+2]
                                                + ByerrBufferLine_pls2[col-2] + ByerrBufferLine_pls2[col] + ByerrBufferLine_pls2[col+2])>>12);
                        avg_gb_q = CLIP_VAL(avg_gb_q, 0, MaxPxlVal);
                    }
                    avg_G3_q = 819*(int)(ByerrBufferLine_min1[col-1] + ByerrBufferLine_min1[col+1] + ByerrBufferLine[col] +
                        ByerrBufferLine_pls1[col-1] + ByerrBufferLine_pls1[col+1])>>12;
                    avg_G3_q = CLIP_VAL(avg_G3_q, 0, MaxPxlVal);
                    // Research FCC checker removal
                    avg_G3_q_rm1 = (int)(ByerrBufferLine_min2[col] + ByerrBufferLine_min1[col-1] + ByerrBufferLine_min1[col+1] +
                        ByerrBufferLine[col] + 2) >> 2; 
                    avg_G3_q_rm1 = CLIP_VAL(avg_G3_q_rm1, 0, MaxPxlVal);
                    avg_G3_q_rp1 = (int)(ByerrBufferLine[col] + ByerrBufferLine_pls1[col-1] + ByerrBufferLine_pls1[col+1] +
                        ByerrBufferLine_pls2[col] + 2) >> 2; 
                    avg_G3_q_rp1 = CLIP_VAL(avg_G3_q_rp1, 0, MaxPxlVal);
                    avg_G3_q_cm1 = (int)(ByerrBufferLine[col-2] + ByerrBufferLine_min1[col-1] + ByerrBufferLine[col] +
                        ByerrBufferLine_pls1[col-1] + 2) >> 2; 
                    avg_G3_q_cm1 = CLIP_VAL(avg_G3_q_cm1, 0, MaxPxlVal);
                    avg_G3_q_cp1 = (int)(ByerrBufferLine[col] + ByerrBufferLine_min1[col+1] + ByerrBufferLine[col+2] +
                        ByerrBufferLine_pls1[col+1] + 2) >> 2; 
                    avg_G3_q_cp1 = CLIP_VAL(avg_G3_q_cp1, 0, MaxPxlVal);
                }
                else
                {
                    if(FC(row,col)== Red)
                    {
                        avg_gr_q = (682*(int)(ByerrBufferLine_min2[col-1] + ByerrBufferLine_min2[col+1] 
                                                 +ByerrBufferLine[col-1]      + ByerrBufferLine[col+1]
                                                 +ByerrBufferLine_pls2[col-1] + ByerrBufferLine_pls2[col+1])>>12);
                        avg_gr_q = CLIP_VAL(avg_gr_q, 0, MaxPxlVal);
                        avg_gb_q = (682*(int)(ByerrBufferLine_min1[col-2] + ByerrBufferLine_min1[col] + ByerrBufferLine_min1[col+2] 
                                                 +ByerrBufferLine_pls1[col-2] + ByerrBufferLine_pls1[col] + ByerrBufferLine_pls1[col+2])>>12);
                        avg_gb_q = CLIP_VAL(avg_gb_q, 0, MaxPxlVal);
                    }
                    else
                    {
                        avg_gr_q = (682*(int)(ByerrBufferLine_min1[col-2] + ByerrBufferLine_min1[col] + ByerrBufferLine_min1[col+2]
                                                 +ByerrBufferLine_pls1[col-2] + ByerrBufferLine_pls1[col] + ByerrBufferLine_pls1[col+2])>>12);
                        avg_gr_q = CLIP_VAL(avg_gr_q, 0, MaxPxlVal);
                        avg_gb_q = (682*(int)(ByerrBufferLine_min2[col-1] + ByerrBufferLine_min2[col+1] 
                                                 +ByerrBufferLine[col-1]      + ByerrBufferLine[col+1]
                                                 +ByerrBufferLine_pls2[col-1] + ByerrBufferLine_pls2[col+1])>>12);
                        avg_gb_q = CLIP_VAL(avg_gb_q, 0, MaxPxlVal);
                    }
                    avg_G3_q = (int)(ByerrBufferLine_min1[col] + ByerrBufferLine[col-1] + ByerrBufferLine[col+1] +
                        ByerrBufferLine_pls1[col] + 2) >> 2; 
                    avg_G3_q = CLIP_VAL(avg_G3_q, 0, MaxPxlVal);
                    /// Research FCC, checker removal
                    avg_G3_q_rm1 = 819*(int)(ByerrBufferLine_min2[col-1] + ByerrBufferLine_min2[col+1] + ByerrBufferLine_min1[col] +
                        ByerrBufferLine[col-1] + ByerrBufferLine[col+1])>>12;
                    avg_G3_q_rm1 = CLIP_VAL(avg_G3_q_rm1, 0, MaxPxlVal);
                    avg_G3_q_rp1 = 819*(int)(ByerrBufferLine[col-1] + ByerrBufferLine[col+1] + ByerrBufferLine_pls1[col] +
                        ByerrBufferLine_pls2[col-1] + ByerrBufferLine_pls2[col+1])>>12;
                    avg_G3_q_rp1 = CLIP_VAL(avg_G3_q_rp1, 0, MaxPxlVal);
                    avg_G3_q_cm1 =819*(int)(ByerrBufferLine_min1[col-2] + ByerrBufferLine_min1[col] + ByerrBufferLine[col-1] +
                        ByerrBufferLine_pls1[col-2] + ByerrBufferLine_pls1[col+2])>>12;
                    avg_G3_q_cm1 = CLIP_VAL(avg_G3_q_cm1, 0, MaxPxlVal);
                    avg_G3_q_cp1 = 819*(int)(ByerrBufferLine_min1[col] + ByerrBufferLine_min1[col+2] + ByerrBufferLine[col+1] +
                        ByerrBufferLine_pls1[col] + ByerrBufferLine_pls1[col+2])>>12;
                    avg_G3_q_cp1 = CLIP_VAL(avg_G3_q_cp1, 0, MaxPxlVal);
                }
                avg_G_q = (avg_gr_q + avg_gb_q + 1)>>1;

                // calc std
                if(FC(row,col)== Green){
                    if(FC(row,col-1)== Red)
                    {
                        Gr_std_q = (455*(int)(min(abs(ByerrBufferLine_min2[col-2]  - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col]  - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+2]  - avg_gr_q) , std_prec )
                                             +min(abs(ByerrBufferLine[col-2] - avg_gr_q) , std_prec )       + min(abs(ByerrBufferLine[col] - avg_gr_q) , std_prec )       + min(abs(ByerrBufferLine[col+2] - avg_gr_q) , std_prec )
                                             +min(abs(ByerrBufferLine_pls2[col-2] - avg_gr_q) , std_prec )  + min(abs(ByerrBufferLine_pls2[col] - avg_gr_q) , std_prec )  + min(abs(ByerrBufferLine_pls2[col+2] - avg_gr_q) , std_prec )))>>12;
                        Gb_std_q = (min(abs(ByerrBufferLine_min1[col-1] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+1] - avg_gb_q) , std_prec )
                                   +min(abs(ByerrBufferLine_pls1[col-1] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+1] - avg_gb_q) , std_prec ) + 2)>>2;
                    }
                    else
                    {
                        Gr_std_q = (min(abs(ByerrBufferLine_min1[col-1] - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+1] - avg_gr_q) , std_prec )
                                   +min(abs(ByerrBufferLine_pls1[col-1] - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+1] - avg_gr_q) , std_prec ) + 2)>>2; 
                        Gb_std_q = (455*(int)(min(abs(ByerrBufferLine_min2[col-2] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+2] - avg_gb_q) , std_prec )
                                             +min(abs(ByerrBufferLine[col-2] - avg_gb_q) , std_prec )      + min(abs(ByerrBufferLine[col]     - avg_gb_q) , std_prec )  + min(abs(ByerrBufferLine[col+2]   - avg_gb_q) , std_prec )
                                             +min(abs(ByerrBufferLine_pls2[col-2] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col+2] - avg_gb_q) , std_prec )))>>12;
                    }
                    G3_std_q = (819*(int)(min(abs(ByerrBufferLine_min1[col-1] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+1] - avg_G3_q) , std_prec )
                                                                +min(abs(ByerrBufferLine[col] - avg_G3_q) , std_prec ) 
                                                +min(abs(ByerrBufferLine_pls1[col-1] - avg_G3_q) , std_prec )  + min(abs(ByerrBufferLine_pls1[col+1] - avg_G3_q) , std_prec )))>>12;
                    G_std_q =  (315*(int)(min(abs(ByerrBufferLine_min2[col-2] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+2] - avg_G_q) , std_prec )
                                                               +min(abs(ByerrBufferLine_min1[col-1] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+1] - avg_G_q) , std_prec )
                                            +min(abs(ByerrBufferLine[col-2] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine[col] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine[col+2] - avg_G_q) , std_prec )
                                                               +min(abs(ByerrBufferLine_pls1[col-1] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+1] - avg_G_q) , std_prec )
                                            +min(abs(ByerrBufferLine_pls2[col-2] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col+2] - avg_G_q) , std_prec )))>>12;
                    /// Research FCC, checker removal
                    G3_std_q_rm1 = (int)(          min(abs(ByerrBufferLine_min2[col] - avg_G3_q_rm1) , std_prec )
                               + min(abs(ByerrBufferLine_min1[col-1] - avg_G3_q_rm1) , std_prec ) + min(abs(ByerrBufferLine_min1[col+1] - avg_G3_q_rm1) , std_prec )
                                            + min(abs(ByerrBufferLine[col] - avg_G3_q_rm1) , std_prec ) + 2)>>2;
                    G3_std_q_rp1 = (int)(          min(abs(ByerrBufferLine[col] - avg_G3_q) , std_prec )
                               + min(abs(ByerrBufferLine_pls1[col-1] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+1] - avg_G3_q) , std_prec )
                                            + min(abs(ByerrBufferLine_pls2[col] - avg_G3_q) , std_prec ) + 2)>>2;
                    G3_std_q_cm1 = (int)(          min(abs(ByerrBufferLine_min1[col-1] - avg_G3_q) , std_prec )
                               + min(abs(ByerrBufferLine[col-2] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine[col] - avg_G3_q) , std_prec )
                                            + min(abs(ByerrBufferLine_pls1[col-1] - avg_G3_q) , std_prec ) + 2)>>2;
                    G3_std_q_cp1 = (int)(          min(abs(ByerrBufferLine_min1[col+1] - avg_G3_q) , std_prec )
                               + min(abs(ByerrBufferLine[col] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine[col+2] - avg_G3_q) , std_prec )
                                            + min(abs(ByerrBufferLine_pls1[col+1] - avg_G3_q) , std_prec ) + 2)>>2;
                }
                else{
                    if(FC(row,col)== Red){
                        Gr_std_q = (682*(int)(min(abs(ByerrBufferLine_min2[col-1] - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+1] - avg_gr_q) , std_prec )
                                             +min(abs(ByerrBufferLine[col-1]      - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine[col+1]      - avg_gr_q) , std_prec )
                                             +min(abs(ByerrBufferLine_pls2[col-1] - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col+1] - avg_gr_q) , std_prec )))>>12;
                        Gb_std_q = (682*(int)(min(abs(ByerrBufferLine_min1[col-2] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+2] - avg_gb_q) , std_prec )
                                             +min(abs(ByerrBufferLine_pls1[col-2] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+2] - avg_gb_q) , std_prec )))>>12;
                    }
                    else{
                        Gr_std_q = (682*(int)(min(abs(ByerrBufferLine_min1[col-2] - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col]   - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+2] - avg_gr_q) , std_prec )
                                             +min(abs(ByerrBufferLine_pls1[col-2] - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col]   - avg_gr_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+2] - avg_gr_q) , std_prec )))>>12;
                        Gb_std_q = (682*(int)(min(abs(ByerrBufferLine_min2[col-1] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+1] - avg_gb_q) , std_prec ) 
                                             +min(abs(ByerrBufferLine[col-1]      - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine[col+1]      - avg_gb_q) , std_prec )
                                             +min(abs(ByerrBufferLine_pls2[col-1] - avg_gb_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col+1] - avg_gb_q) , std_prec )))>>12;
                    }
                    G3_std_q = (int)(          min(abs(ByerrBufferLine_min1[col] - avg_G3_q) , std_prec )
                               + min(abs(ByerrBufferLine[col-1] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine[col+1] - avg_G3_q) , std_prec )
                                            + min(abs(ByerrBufferLine_pls1[col] - avg_G3_q) , std_prec ) + 2)>>2;
                    G_std_q =   (341*(int)(min(abs(ByerrBufferLine_min2[col-1] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+1] - avg_G_q) , std_prec ) 
                        +min(abs(ByerrBufferLine_min1[col-2] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+2] - avg_G_q) , std_prec )
                                            +min(abs(ByerrBufferLine[col-1] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine[col+1] - avg_G_q) , std_prec )
                        +min(abs(ByerrBufferLine_pls1[col-2] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_pls1[col+2] - avg_G_q) , std_prec )
                                            +min(abs(ByerrBufferLine_pls2[col-1] - avg_G_q) , std_prec ) + min(abs(ByerrBufferLine_pls2[col+1] - avg_G_q) , std_prec )))>>12;
                    /// Research FCC, checker removal
                    G3_std_q_rm1 = (819*(int)(min(abs(ByerrBufferLine_min2[col-1] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine_min2[col+1] - avg_G3_q) , std_prec )
                                                                +min(abs(ByerrBufferLine_min1[col] - avg_G3_q) , std_prec ) 
                                                +min(abs(ByerrBufferLine[col-1] - avg_G3_q) , std_prec )  + min(abs(ByerrBufferLine[col+1] - avg_G3_q) , std_prec )))>>12;
                    G3_std_q_rp1 = (819*(int)(min(abs(ByerrBufferLine[col-1] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine[col+1] - avg_G3_q) , std_prec )
                                                                +min(abs(ByerrBufferLine_pls1[col] - avg_G3_q) , std_prec ) 
                                                +min(abs(ByerrBufferLine_pls2[col-1] - avg_G3_q) , std_prec )  + min(abs(ByerrBufferLine_pls2[col+1] - avg_G3_q) , std_prec )))>>12;

                    G3_std_q_cm1 = (819*(int)(min(abs(ByerrBufferLine_min1[col-2] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col] - avg_G3_q) , std_prec )
                                                                +min(abs(ByerrBufferLine[col-1] - avg_G3_q) , std_prec ) 
                                                +min(abs(ByerrBufferLine_pls1[col-2] - avg_G3_q) , std_prec )  + min(abs(ByerrBufferLine_pls1[col] - avg_G3_q) , std_prec )))>>12;

                    G3_std_q_cp1 = (819*(int)(min(abs(ByerrBufferLine_min1[col] - avg_G3_q) , std_prec ) + min(abs(ByerrBufferLine_min1[col+2] - avg_G3_q) , std_prec )
                                                                +min(abs(ByerrBufferLine[col+1] - avg_G3_q) , std_prec ) 
                                                +min(abs(ByerrBufferLine_pls1[col] - avg_G3_q) , std_prec )  + min(abs(ByerrBufferLine_pls1[col+2] - avg_G3_q) , std_prec )))>>12;

                }

                AvgG_std_q = (Gr_std_q + Gb_std_q + 1)>>1; 


                //// Diagonal 
                int metric_diag, metric_overshrpn2;
                int Y_est[3][3];
                int G1, G2, G3, G4;
                int cu_diag1_out, cu_diag2_out, cu_diag3_out;
                int ClpdW;

                ////////////////// STEP 1 ///////////////////

                /////////////////////////////////////////////////
                ////// Green clip weigh (Wgclp) calculation
                ////// calculated only in even rows like avg between even and odd rows
                /////////////////////////////////////////////////
                int G_std_q_array[7];           
                    // calculate w_gclp for each column independently based on central row and store it in array
                        for (int gl_shift_c = - 3 ;  gl_shift_c <= 3 ; ++gl_shift_c )
                        {
                            int col_w = col+gl_shift_c;
                                for(int shift_r = - 1 ;  shift_r <= 1 ; ++shift_r )
                                {
                                    for(int shift_c = - 1 ;  shift_c <= 1 ; ++shift_c )
                                    {
                                        int row_ind = max(min(row+shift_r,m_Height-2+abs(row+shift_r)& 1),abs(row+shift_r)& 1); 
                                        int col_ind = col + (shift_c+gl_shift_c);
                                    //         buf_r = min( max(row+1,abs((row+1)& 1)) , m_Height - 2 + abs((row+1)& 1) );buf_r = min( max(row-1,abs((row-1)& 1)) , m_Height - 2 + abs((row-1)& 1) );
                                        Y_est[shift_r+1][shift_c+1] =  ( pInImg->m_R[max(row_ind-1,abs(row_ind-1)&1)][col_ind-1] + pInImg->m_R[max(row_ind-1,abs(row_ind-1)&1)][col_ind]*2 + pInImg->m_R[max(row_ind-1,abs(row_ind-1)&1)][col_ind+1] 
                                                                       + pInImg->m_R[row_ind][col_ind-1]*2        + pInImg->m_R[row_ind][col_ind]*4          + pInImg->m_R[row_ind][col_ind+1]*2  
                                                                       + pInImg->m_R[min(row_ind+1,m_Height-2+abs(row_ind+1)&1)][col_ind-1] + pInImg->m_R[min(row_ind+1,m_Height-2+abs(row_ind+1)&1)][col_ind]*2 + pInImg->m_R[min(row_ind+1,m_Height-2+abs(row_ind+1)&1)][col_ind+1] + 8)>>4;
                                    }
                                }
                                metric_diag = min((abs(Y_est[0][0]-Y_est[1][1]) + abs(Y_est[1][1]-Y_est[2][2]) + abs(Y_est[0][1]-Y_est[1][2]) + abs(Y_est[1][0]-Y_est[2][1]) +2 )>>2, 
                                           (abs(Y_est[0][2]-Y_est[1][1]) + abs(Y_est[1][1]-Y_est[2][0]) + abs(Y_est[0][1]-Y_est[1][0]) + abs(Y_est[1][2]-Y_est[2][1]) +2 )>>2 );
                                metric_diag = min(metric_diag,(1<<HWP_DIAG_METR)-1);

                                // calculate metric_diag2
                                G1 = MAX( (abs(pInImg->m_R[max(row-2,abs(row-2)&1)][col_w-2] -  pInImg->m_R[row][col_w]) +  abs(pInImg->m_R[row][col_w - 2] -  pInImg->m_R[min(row + 2,m_Height-2+(row + 2)%1)][col_w]) + 
                                           abs(pInImg->m_R[row][col_w] - pInImg->m_R[min(row + 2,m_Height-2+(row + 2)%1)][col_w + 2]) + abs(pInImg->m_R[max(row - 2,abs(row-2)&1)][col_w] - pInImg->m_R[row][col_w + 2]) + (1<<(HWP_SH_DOWN+1)))>>(2+HWP_SH_DOWN) ,
                                          (abs(pInImg->m_R[max(row-2,abs(row-2)&1)][col_w] - pInImg->m_R[row][col_w - 2]) + abs(pInImg->m_R[max(row - 2,abs(row-2)&1)][col_w + 2] - pInImg->m_R[row][col_w])+
                                           abs( pInImg->m_R[row][col_w]-pInImg->m_R[min(row + 2,m_Height-2+(row + 2)%1)][col_w-2]) + abs(pInImg->m_R[row][col_w+2]-pInImg->m_R[min(row+2,m_Height-2+(row + 2)%1)][col_w]) + (1<<(HWP_SH_DOWN+1)))>>(2+HWP_SH_DOWN));
                                G2 = MAX( (abs(pInImg->m_R[max(row-1,abs(row-1)&1)][col_w-1] - pInImg->m_R[min(row + 1,m_Height-2+(row + 1)%1)][col_w+1]) + (1<<(HWP_SH_DOWN-1)))>>HWP_SH_DOWN,
                                          (abs(pInImg->m_R[max(row-1,abs(row-1)&1)][col_w+1] - pInImg->m_R[min(row + 1,m_Height-2+(row + 1)%1)][col_w-1]) + (1<<(HWP_SH_DOWN-1)))>>HWP_SH_DOWN);
                                G3 = MAX( (abs(pInImg->m_R[max(row-2,abs(row-2)&1)][col_w-1] - pInImg->m_R[row][col_w+1]) + abs(pInImg->m_R[row][col_w-1]-pInImg->m_R[min(row + 2,m_Height-1)][col_w+1]) + (1<<HWP_SH_DOWN) )>>
    ....
share|improve this question
1  
Get yourself familiarized with OOPS Design Patterns, if you haven't already. –  Kevindra Mar 20 at 20:06
    
Please do not remove the original code as the answers rely on them. You may post the updated version below the original. –  Jamal Mar 23 at 20:31
    
sure was about to update –  Gilad Mar 23 at 20:34

2 Answers 2

up vote 4 down vote accepted

It's difficult impossible to review for correctness ("Does it implement required functionality?") without knowing (or being able to recognize from past experience of image processing algorithms) what the required functionality is. Ideally there should be one or more comments which reference (perhaps with a hyperlink) the algorithms that are being implemented.

 void Algo::compute

Not a good class name Algo.

 int ind_r, ind_c, ind_app_r, ind_app_c , ind_g_r, ind_g_c;

Variables declared before they're initialized. Badly named. It's even more important to be told what data is supposed to be in a variable, than to be told what the code is doing (because you can read code to see what it's doing).

That might be the first change I'd make if I were trying to refactor it. For example, MSB, MSB2 and valinlog are declared at the top of the function. On inspection they're used within a single for loop later on: that (inside the for loop) is where they should be declared. Otherwise it's difficult to tell:

  • What they're used for (what they mean)
  • Whether they're initialized before they're used
  • Whether they're used again much later in the procedure (whereas if they're defined inside a small scope like a for loop, you know they're not re-used after they have gone out of scope)

Having so many at the top of the long procedure is akin to having global variables: it's difficult to tell where they're referenced.

int Gv_approx[7][7]

Magic numbers: 7 etc. (also 455, 819, etc.)

FILE *fp_log=fopen(m_OutputFileName.c_str(),"wt");

No immediate guarantee this file will be closed again.

Prepare_Config_Unit_DIAG1();

Maybe this and similar should be constructors of some class.

// produce corinng threshold calculation outside main loop

Woot: a comment! Too bad it's meaningless to me. Is corinng an identifier, or a typo?

// define rellevant lines' pointers.

Non-standard indentation (8 spaces instead of 4).

pInImg->m_R[min(row_ind+1,m_Height-2+abs(row_ind+1)&1)][col_ind-1] + pInImg-m_R[min(row_ind+1,m_Height-2+abs(row_ind+1)&1)][col_ind]*2 + pInImg-m_R[min(row_ind+1,m_Height-2+abs(row_ind+1)&1)][col_ind+1] + 8)>>4;

A nice long line; not designed to be read by a human.

            //if(DEBUG_MODE)
            //{

Doesn't inspire confidence. If it's debug mode why are subsequent lines of code unconditionally enabled?

share|improve this answer

Given that a good rule of thumb is to keep functions small enough to fit on a page, I want the kind of screen these coders have...

Seriously, function calls are cheap on modern hardware, and compilers can do the inlining for you today. I'd bet the code can gain some performance simply by chopping it into smaller functions.

You might get away with giving a variable a name like HH_h if there were <10 variables in the function, but for this monstrosity you'd need to have a more descriptive name than that, and also a comment for every variable, describing exactly what it represents.
Speaking of comments, if you can't be bothered to put stuff in functions, at least write a comment explaining what each loop is doing.
Finally, I know C isn't an object-oriented language at heart, but it does support structs. Use them to group related values together.

share|improve this answer
    
It looks like C code, but it's actually C++11 code. –  Morwenn Mar 20 at 23:57
    
@Morwenn: I don't see why this should be turned into a language war, nobody sane would say this is acceptable for C code. The suggestions I've made work for both C and C++. –  EOF Mar 21 at 8:45
    
It is because you said "I know C isn't an object-oriented language at heart". But this is C++, which is actually an OO language; my point was that you could strengthen this sentence. I never intended to turn this into a language war. –  Morwenn Mar 21 at 9:12
    
@Morwenn: Well, it seems to me like the whole question is meant to be used to beat C-programmers over the head saying: "Behold what C hath wrought!", when really it's not a matter of language, but of good programming practices older than dirt. Practically, if I were asked to convert this code into C++, I'd either do a full rewrite, or first turn it into usable C. If you go and design a few nice structs for the code, the only thing you need to do to turn them into classes is turning the functions that work on them into methods. –  EOF Mar 21 at 16:04

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.