[Vxl-maintainers] RE: vidl_avicodec

[Matt L. <ml...@le...>]

Gehua,      

   I made a very quick and rough conversion from vil1 to vil in the
vidl code.  Most of the changes were scripted, and then I manually
fixed what was needed to get everything to compile and work. I agree
that most of the AVI code is outdated.  This could be done much more
efficiently using some of the features of vil.  Apparently there is
some better code in contrib/mul/mvl2 that also supports AVI in linux
using the avifile library.  I didn't have the time to look into using a
similar approach to update vidl_avicodec.

   As a result, if anything in vidl_avicodec or vidl_mpegcodec looks
incorrect it probably is.  Feel free to make it better.

Thanks,
Matt Leotta


         First problem:
         In vidl_avicodec::make_dib(), there are lines:
           // 1st, Get the datas from the video frame
           vil_image_view<vxl_byte> image(frame->get_view());
           byte* TjSection = (byte*) image.top_left_ptr();
           ......
           // Get rid of the original datas
           delete[] TjSection;
         
        
         It does NOT make any sense to free the image memory from outside! 
         
         Before revision 1.22, the TjSection was an self-allocated array of bytes and therefore
        needs deletion. But now, as it is changed to point to the buffer inside imag_view, the
        situation is totally different. As far as I can see, it should NOT delete. 
         
        
        
         Second:
         Also inside this function, as it copied the contents of image_view<byte> into Windows DIB
        format, it used the following code with TjSection defined as above:
         
              // Store the TargetJr data in a Bitmap way, DIB is a flipped upside down
               for (j=frame->height()-1; j>=0;j--)
               {
                 db = TjSection+ (j*frame->width())*frame->get_bytes_pixel();
                 DIB = newbits + (frame->height()-j-1)*line_length;
                 for (i=0; i<frame->width(); i++) {
                   *DIB = *(db+2);
                   *(DIB+1) = *(db+1);
                   *(DIB+2) = *(db);
                   DIB+=3;
                   db+=3;
                 }
               }
         
          
         It seems that it tried to improve performance by assuming a lot of things in
        vil_image_view. But I think it is unnecessary. One idea is to create a view using
        vil_flip_ud(), and then apply a straight-forward copying. 
         
         Please correct me if I make any mistakes or misunderstand. It is very likely since I only
        started looking at this code 10 minutes ago. As I never wrote a line of code in this library
        and does not have the big picture in mind, I don't want to do sth look like a quick fix but
        actually break the code. 
         
         Best Regards,
         Gehua Yang