Opened 14 years ago

Closed 14 years ago

#1079 closed defect (fixed)

frame corruption in preview playbackbox

Reported by: jwestfall@… Owned by: danielk
Priority: minor Milestone: 0.19
Component: mythtv Version: head
Severity: low Keywords:
Cc: Ticket locked: no

Description

Hi

There is some pretty obvious frame corruption occuring in the preview playbackbox. It looks like its being cause by a lack of locking on the frame. The decoder is overwriting frame->buf with a different frame while we are still doing yuv2rgba conversion. The result is an image that consists of 2 parts of 2 different frames.

jim

Attachments (1)

get-frame-timing.patch (12.3 KB) - added by danielk 14 years ago.
Shows timing differences between scaling image as YUV vs. RGB

Download all attachments as: .zip

Change History (5)

comment:1 Changed 14 years ago by danielk

Milestone: 0.20
Resolution: worksforme
Status: newclosed

I'm not sure what you are experiencing, but the previewPixmap is only updated within a qApp->lock()/qApp->unlock().

Is this reproducable on your end? If so how?

comment:2 Changed 14 years ago by jwestfall@…

Resolution: worksforme
Status: closedreopened

The issue is with the video preview not the static pixmap. The code in PlaybackBox::updateVideo is doing

       VideoFrame *frame = nvp->GetCurrentFrame(w, h);
        
        if (w == 0 || h == 0 || !frame || !(frame->buf))
        {   
            nvp->ReleaseCurrentFrame(frame);
            return;
        }   
            
        unsigned char *yuv_buf = frame->buf;
        if (conv_rgba_size.width() != w || conv_rgba_size.height() != h)
        {   
            if (conv_rgba_buf)
                delete [] conv_rgba_buf;
            conv_rgba_buf = new unsigned char[w * h * 4];
            conv_rgba_size = QSize(w, h);
        }
  
        conv_yuv2rgba(conv_rgba_buf,
                      yuv_buf, yuv_buf + (w * h), yuv_buf + (w * h * 5 / 4),
                      w, h, w * 4, w, w / 2, 0);

        nvp->ReleaseCurrentFrame(frame);
        
        QImage img(conv_rgba_buf, w, h, 32, NULL, 65536 * 65536,
                   QImage::LittleEndian);
        img = img.scale(videoRect.width(), videoRect.height());
            
        p->drawImage(videoRect.x(), videoRect.y(), img);
    }

for each frame to be displayed in the preview video box. There is no locking on the frame and frame->buf is being passed to conv_yuv2rgba(). This leaves the decoder thread open to overwrite frame->buf while its being processed by yuv2rgba(). It may not be noticeable in the mmx version of yuv2rgba(), but you can definitely see the issue in the c version which is ultra slow.

comment:3 Changed 14 years ago by danielk

Milestone: 0.200.19
Owner: changed from Isaac Richards to danielk
Status: reopenednew

Ok, I see what the problem is. I'll fix this tomorrow.

Changed 14 years ago by danielk

Attachment: get-frame-timing.patch added

Shows timing differences between scaling image as YUV vs. RGB

comment:4 Changed 14 years ago by danielk

Resolution: fixed
Status: newclosed

(In [8736]) Fixes #1079, and speeds up scaling.

This grabs the preview from within the output loop, so that it is impossible for a slow scaler/yuv2rgb converter to get parts of two frames. This also speeds up the process 4 to 5 fold by scaling the frame before converting it to RGB, instead of the other way around. This is on my machine with MMX, I wrote the fix with the RGB conversion before the YUV scaling as well for timing purposes and that version is attached to the ticket, so anyone without MMX can test both approaches.

Note: See TracTickets for help on using tickets.