After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 715108 - Include crystalhd plugin
Include crystalhd plugin
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2013-11-24 12:57 UTC by Guido Günther
Modified: 2016-12-28 12:08 UTC
See Also:
GNOME target: ---
GNOME version: ---

Crystalhd support using libcrystalhd (129.38 KB, patch)
2013-11-24 13:06 UTC, Guido Günther
needs-work Details | Review

Description Guido Günther 2013-11-24 12:57:06 UTC
Attached patch adds the crystalhd plugin (for Broadcom's crystalhd chips) to gst-plugins-bad. The sources were once maintained at:


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:

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.
Comment 1 Guido Günther 2013-11-24 13:06:01 UTC
Created attachment 261352 [details] [review]
Crystalhd support using libcrystalhd
Comment 2 Olivier Crête 2013-12-05 23:50:34 UTC
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 @@


@@ +2499,3 @@

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;

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 @@
+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
Comment 3 Tim-Philipp Müller 2013-12-06 00:18:05 UTC
> 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).
Comment 4 Bastien Nocera 2013-12-14 18:17:10 UTC
(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.
Comment 5 Guido Günther 2013-12-15 11:44:08 UTC
Great! I'm pushing the current progress to:

Porting to GstVideoDecoder will need some free time. I'll ping once this is done.
Comment 6 Tim-Philipp Müller 2016-12-28 12:08:06 UTC
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!