[Bug 715108] Include crystalhd plugin

[GStreamer (bugzilla.gnome.org)]

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

Olivier Crete (Tester) <olivier.crete> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #261352|none                        |needs-work
             status|                            |

--- Comment #2 from Olivier Crete (Tester) <olivier.crete at ocrete.ca> 2013-12-05 23:50:34 UTC ---
Review of attachment 261352:
 --> (https://bugzilla.gnome.org/review?bug=715108&attachment=261352)

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

-- 
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.