# HG changeset patch
# User Steve Freeland <[EMAIL PROTECTED]>
# Date 1181086752 -3600
# Node ID cde74537c27744822caa70a9efbbfc0b653a2bd0
# Parent  c682c2e968e1a19f2b0fc3bf6450609f5195b36c
[PATCH] video_out_fb crash
I discovered some problems with the framebuffer output driver.  The first
problem I had was a segfault when trying to play a 480x360 clip on a 640x480
display.  I traced it to the yuv420_rgb16() color conversion function, which
was overrunning the input buffer (the "y" part of the image).  The reason
was that it was being called downstack from fb_frame_proc_slice(), multiple
times for each 16-pixel high horizontal slice of the image.  When it got to
the last slice, only 8 pixels were left to the bottom but it still tried to
process a 16-pixel high slice.

Nosing around a bit, I compared the configuration of the color converter as
used by the fb driver to the xshm driver and found some oddities:

1) The color converter was configured with a "source height" of 16 pixels no
matter what the size of the image, and a "dest height" based on what was
referred to within video_out_fb.c as a "stripe" -- essentially an input
slice scaled up or down as required by the output size.
2) Apparently to prevent the above from causing problems, the position in
the output buffer was managed by special code -- see the "stripe_incr"
variable.
3) The xshm driver calls yuv2rgb_next_slice() with a NULL argument at the
beginning of each frame to allow the color converter to reset its tracking
of the slice-by-slice progress through the image; the fb driver does not.

I'm not sure exactly why it was done that way, but my best guess would be
that whoever coded it didn't know about the need to call
yuv2rgb_next_slice() with a NULL argument, and the rest was built up to get
it to mostly work without that.

The attached patch changes the behaviour to match that of the xshm driver,
and also removes the reset_dest_pointers() function, replacing its single
invocation with one to fb_frame_field(), which is identical after removing
the "stripe" management.

It fixed my crash.  Can anyone see if I've misunderstood what was going on?
If not, it should probably be applied to the official version.

diff -r cde74537c27744822caa70a9efbbfc0b653a2bd0 -r 
c682c2e968e1a19f2b0fc3bf6450609f5195b36c src/video_out/video_out_fb.c
--- a/src/video_out/video_out_fb.c      Wed Jun 06 00:39:12 2007 +0100
+++ b/src/video_out/video_out_fb.c      Tue Jun 05 19:45:56 2007 +0100
@@ -99,7 +99,6 @@ typedef struct fb_frame_s
   yuv2rgb_t         *yuv2rgb;  /* yuv2rgb converter for this frame */
   uint8_t           *rgb_dst;
   int                yuv_stride;
-  int                stripe_height, stripe_inc;
   
   int                bytes_per_line;
 
@@ -182,7 +181,6 @@ static void fb_frame_proc_slice(vo_frame
   else
     frame->yuv2rgb->yuy22rgb_fun(frame->yuv2rgb,
                                 frame->rgb_dst, src[0]);
-  frame->rgb_dst += frame->stripe_inc; 
 }
 
 static void fb_frame_field(vo_frame_t *vo_img, int which_field)
@@ -193,21 +191,18 @@ static void fb_frame_field(vo_frame_t *v
   {
   case VO_TOP_FIELD:
       frame->rgb_dst    = frame->data;
-      frame->stripe_inc = 2*frame->stripe_height *
-                         frame->bytes_per_line;
     break;
                        
   case VO_BOTTOM_FIELD:
       frame->rgb_dst    = frame->data +
                          frame->bytes_per_line ;
-      frame->stripe_inc = 2*frame->stripe_height *
-                         frame->bytes_per_line;
     break;
                        
   case VO_BOTH_FIELDS:
       frame->rgb_dst    = frame->data;
     break;
   }
+  frame->yuv2rgb->next_slice (frame->yuv2rgb, NULL);
 }
 
 static void fb_frame_dispose(vo_frame_t *vo_img)
@@ -304,11 +299,11 @@ static void setup_colorspace_converter(f
       frame->yuv2rgb->
        configure(frame->yuv2rgb,
                  frame->sc.delivered_width,
-                 16,
+                 frame->sc.delivered_height,
                  2 * frame->vo_frame.pitches[0],
                  2 * frame->vo_frame.pitches[1],
                  frame->sc.output_width,
-                 frame->stripe_height,
+                 frame->sc.output_height,
                  frame->bytes_per_line * 2);
       frame->yuv_stride = frame->bytes_per_line * 2;
       break;
@@ -317,38 +312,13 @@ static void setup_colorspace_converter(f
       frame->yuv2rgb->
        configure(frame->yuv2rgb,
                  frame->sc.delivered_width,
-                 16,
+                 frame->sc.delivered_height,
                  frame->vo_frame.pitches[0],
                  frame->vo_frame.pitches[1],
                  frame->sc.output_width,
-                 frame->stripe_height,
+                 frame->sc.output_height,
                  frame->bytes_per_line);
       frame->yuv_stride = frame->bytes_per_line;
-      break;
-  }
-}
-
-static void reset_dest_pointers(fb_frame_t *frame, int flags)
-{
-  switch(flags)
-  {
-    case VO_TOP_FIELD:
-      frame->rgb_dst = frame->data;
-      frame->stripe_inc = 2 * frame->stripe_height *
-                         frame->bytes_per_line;
-      break;
-
-    case VO_BOTTOM_FIELD:
-      frame->rgb_dst = frame->data +
-                      frame->bytes_per_line ;
-      frame->stripe_inc = 2 * frame->stripe_height *
-                         frame->bytes_per_line;
-      break;
-
-    case VO_BOTH_FIELDS:
-      frame->rgb_dst = frame->data;
-      frame->stripe_inc = frame->stripe_height *
-                         frame->bytes_per_line;
       break;
   }
 }
@@ -457,8 +427,6 @@ static void fb_update_frame_format(vo_dr
 
     frame_reallocate(this, frame, width, height, format);
 
-    frame->stripe_height = 16 * frame->sc.output_height /
-                          frame->sc.delivered_height;
     if(this->use_zero_copy)
       frame->bytes_per_line = this->fb_bytes_per_line;
     else
@@ -468,7 +436,7 @@ static void fb_update_frame_format(vo_dr
     setup_colorspace_converter(frame, flags);
   }
 
-  reset_dest_pointers(frame, flags);
+  fb_frame_field(frame_gen, flags);
 }
 
 static void fb_overlay_clut_yuv2rgb(fb_driver_t *this,

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Xine-cvslog mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/xine-cvslog

Reply via email to