[Bug 722345] directfb: video sink crashes when used with ext_surface

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=722345
  GStreamer | gst-plugins-bad | 1.2.2

--- Comment #3 from Eric <etrousset at awox.com> 2014-01-16 15:17:46 UTC ---
(In reply to comment #2)
> Review of attachment 266475 [details]:
> 
> :::
> /home/etrousset/source_ext/gst-1.2.2/gst-plugins-bad-1.2.2.modified/ext/directfb/dfbvideosink.c
> @@ +235,3 @@
> +  if(!dfbpool->dfbvideosink->ext_surface) {
> +      ret = dfbpool->dfbvideosink->dfb->CreateSurface
> (dfbpool->dfbvideosink->dfb,
> +                                                       &s_dsc, &surface);
> 
> Please run gst-indent on this and create a new patch :)
> 
> @@ +284,2 @@
>    /* Release our internal surface */
> +  if (meta && meta->surface) {
> 
> Why would we ever have no meta? Sounds like we should just fail in that case
> 

I think that in the case where we use ext_surface, the code just allocate a
standard GstBuffer (not 100% sure) have to look deeper at the code 

> @@ +435,3 @@
>       this is an optimisation for memory allocation */
> +  if (meta->pixel_format != DSPF_UNKNOWN) {
> +      alloc_size = meta->width * meta->height *
> DFB_BYTES_PER_PIXEL(meta->pixel_format);
> 
> What about stride here?
To be honest, I don't know what is the stride ^^
> 
> @@ +1130,3 @@
> +      case GST_VIDEO_FORMAT_BGR:
> +      case GST_VIDEO_FORMAT_RGB:
> +      pixel_format = DSPF_RGB24;
> 
> Can you put this part in a separate patch? Also I don't think it's correct.
> It's probably GST_VIDEO_FORMAT_BGR on little endian, and RGB on big endian.

Yes, this is a quit fix for my case of use to work, but this certainly needs
some tests of this find :)

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.