GNOME Bugzilla – Bug 715108
Include crystalhd plugin
Last modified: 2016-12-28 12:08:06 UTC
Attached patch adds the crystalhd plugin (for Broadcom's crystalhd chips) to gst-plugins-bad. The sources were once maintained at: git://linuxtv.org/jarod/crystalhd.git however the repo seems dead and I didn't get any replies on my forward port to the gstreamer 1.0 API. Since the kernel part of the above repo already lives in the linux staging tree I split out the gst plugin, the userspace library and firmware as well: https://github.com/agx/libcrystalhd https://github.com/agx/crystalhd-firmware https://github.com/agx/gst-plugins-bad There weren't any ABI changes yet so the gst plugin builds against the original source as well as the split out library (that's why it's not using pkgocnfig yet). Any chance this can be merged? There are lots of possible cleanups but it might make more sense to do this in tree to not lose the history.
Created attachment 261352 [details] [review] Crystalhd support using libcrystalhd
Review of attachment 261352 [details] [review]: What exactly is that CrystalHD hardware? Most things (everything?) in version.h and version_lnx.h seem to not be used. This needs quite a bit of working, porting to GstVideoDecoder as a first step. ::: .gitignore @@ +40,3 @@ Makefile.in Makefile +TAGS Unrelated ::: configure.ac @@ +2499,3 @@ ext/assrender/Makefile ext/apexsink/Makefile +ext/bcmdec/Makefile Is this specific to one platform? Probably belongs in sys/bcmdec then. ::: ext/bcmdec/gstbcmdec.c @@ +15,3 @@ + * This library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published + * by the Free Software Foundation, either version 2.1 of the License. We have to be careful to send this towards gst-p-ugly as it is 2.1, not 2.1 or later. @@ +74,3 @@ + + if (sts != BC_STS_SUCCESS) { + gst_buffer_map (buf, &info, GST_MAP_READ); No need to map it here, you're not reading anything from inside the buffer. @@ +146,3 @@ +static void +gst_bcm_dec_base_init (gpointer gclass) +{ Just put all of this into the class_init() @@ +193,3 @@ + g_object_class_install_property (gobject_class, PROP_SILENT, + g_param_spec_boolean ("silent", "Silent", + "Produce verbose output ?", FALSE, (GParamFlags) G_PARAM_READWRITE)); No need to typecast to GParamFlags, also add G_PARAM_STATIC_STRINGS @@ +697,3 @@ +static gboolean +gst_bcm_dec_src_event (GstPad * pad, GstObject * parent, GstEvent * event) +{ Useless, just remove it, let the default behavior happen. @@ +1466,3 @@ + BUF_MULT; + if (!bcmdec_get_buffer (bcmdec, size, &gstbuf)) { + usleep (30 * 1000); usleep().. sounds like there is a deeper problem. @@ +1741,3 @@ + pthread_attr_init (&thread_attr); + pthread_attr_setdetachstate (&thread_attr, PTHREAD_CREATE_JOINABLE); + pthread_create (&bcmdec->push_thread, &thread_attr, bcmdec_process_push, Why does this thread even exist? @@ +1804,3 @@ + pthread_attr_setdetachstate (&thread_attr, PTHREAD_CREATE_JOINABLE); + pthread_create (&bcmdec->recv_thread, &thread_attr, bcmdec_process_output, + bcmdec); You should probably use the GstTask of the src pad instead of creating separate thread. ::: ext/bcmdec/gstbcmdec.h @@ +94,3 @@ + guint8 stride; + +} OUTPARAMS; struct types should be in CamelCase @@ +150,3 @@ +struct _GstBcmDec +{ + GstElement element; Should be based on the GstVideoDecoder base class @@ +337,3 @@ + +// static gboolean +// bcmdec_alloc_mem_padbuf_que_pool(GstBcmDec *filter); No C++ // style comments. Also, put all of thew static definitions in the .c file ::: ext/bcmdec/parse.h @@ +90,3 @@ + +gint +parseAVC (Parse * parse, guint8 * pInputBuf, guint32 ulSize, guint32 * Offset); H.264 parser ? please use the parser in gst-plugins-bad/gst-libs/gst/codecparsers/gsth264parser.h instead of adding another one
> What exactly is that CrystalHD hardware? It's a hardware video decoder chip. Not uncommon in early mini-PCs and notebooks which were too low-powered to decode video in software (at reasonable resolutions).
(In reply to comment #3) > > What exactly is that CrystalHD hardware? > > It's a hardware video decoder chip. Not uncommon in early mini-PCs and > notebooks which were too low-powered to decode video in software (at reasonable > resolutions). Or with an old GPU that doesn't support doing that either :) Guido, I'd happy to test it out with an older CrystalHD once you've respun your plugin. CC:ing Fabiano who's also got some hardware now.
Great! I'm pushing the current progress to: https://github.com/agx/gst-plugins-bad/commits/master Porting to GstVideoDecoder will need some free time. I'll ping once this is done.
Don't know how relevant this hardware is still these days. Closing due to lack of apparent interest/activity. Please re-open if you plan on continuing work on this, thanks!