GNOME Bugzilla – Bug 506549
fbdevsink patch
Last modified: 2008-05-16 22:25:08 UTC
added support for linux framebuffer video sink
Created attachment 101888 [details] [review] patch to add fbdevsink
Generally looks good from a short look but why didn't you use GstVideoSink as base class? Would probably save you some lines of code :)
Also you should add the acceptable ranges of bpp, depth, etc to the pad template. And it might make sense to set the device field of the instance struct in the init method already to the default value.
- extend GstVideoSink, this will set the default video properties correctly, like QoS and max-latency. - use start()/stop() to open close the device. Make getcaps only return the device specific info when it's opened. - be as specific as you can in the caps of the template, I think you can add some more properties.
Created attachment 101931 [details] [review] updated patch, use patch -p1 I think I am extending from GstVideoSink now, I am new to gclass stuff so it is hard to tell. Some of the other drivers use GstVideoSink and GstBaseSink similarly (ximagesink) I am using start() and stop() for opening and closing now. I return FALSE if there is an error with open, ioctl, or mmap. I am not sure if this is correct handling or not, it seems to cause the pipeline to fail to pause. I am more specific with the template of the caps now. I specify
Committed, with a few minor modifications. Moved to sys/, added <stdint.h>, and changed a malloc/strcpy to g_strdup().
Some comments: * Please move declarations to the top of blocks. It's the gstreamer style. * Use the glib macros for endianness changing. * Add a test for big endian, and a g_warning() asking the BE user to report feedback on whether it works or not. * Please add checks for failed opens and mmaps in gst_fbdevsink_stop()