[Bug 767908] jpeg2000parse: use enums for colorspace and sampling, rather than strings

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=767908

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

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

--- Comment #10 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 330147:
 --> (https://bugzilla.gnome.org/review?bug=767908&attachment=330147)

Incomplete review, but all other things go along the same line

::: ext/openjpeg/gstopenjpegdec.h
@@ +25,3 @@
 #include <gst/gst.h>
 #include <gst/video/video.h>
+#include "../../gst-libs/gst/codecparsers/gstjpeg2000sampling.h"

<gst/codecparsers/...>

::: ext/openjpeg/gstopenjpegenc.c
@@ +118,3 @@
         "num-components = (int) [1, 4], "
         GST_RTP_J2K_SAMPLING_LIST ","
+        GST_JPEG2000_SAMPLING_COLORSPACE_LIST "; "

This might be worth moving to the library too... but the naming should all be
more consistent. GST_JPEG2000_* maybe, and call it in the style of
GST_VIDEO_FORMATS_ALL

::: gst-libs/gst/codecparsers/gstjpeg2000sampling.c
@@ +21,3 @@
+#include "gstjpeg2000sampling.h"
+
+static const gchar *GstRtpJ2kSamplingStrings[] = {

snake_case_for_variable_names :)

@@ +37,3 @@
+gst_rtp_j2k_get_sampling (const gchar * sampling_string)
+{
+  GstRtpJ2kSampling i;

g_return_val_if_fail() guards for all the non-static functions

::: gst-libs/gst/codecparsers/gstjpeg2000sampling.h
@@ +26,3 @@
+
+/**********************************************************************************************/
+/* Sampling values from RF 5371 for JPEG 2000 over RTP :
https://datatracker.ietf.org/doc/rfc5371/C

Make this a proper gtk-doc documentation comment, see various headers in
gstreamer core for examples

@@ +61,3 @@
+  GST_RTP_J2K_GRAYSCALE,
+  GST_RTP_J2K_YBRA4444_EXT,
+  GST_RTP_J2K_NO_SAMPLING

No sampling should move to the very beginning

@@ +65,3 @@
+
+
+#define GST_RTP_J2K_SAMPLING_LIST "sampling = (string) {\"RGB\", \"BGR\",
\"RGBA\", \"BGRA\", \"YCbCr-4:4:4\", \"YCbCr-4:2:2\", \"YCbCr-4:2:0\",
\"YCbCr-4:1:1\", \"GRAYSCALE\" , \"YCbCrA\"}"

YCbCrA should probably get -4:4:4:4 added

@@ +86,3 @@
+} GstJPEG2000SamplingColorSpace;
+
+const gchar *gst_get_color_space_string (GstJPEG2000SamplingColorSpace

This needs to be namespaced properly. gst_jpeg2000_*

@@ +88,3 @@
+const gchar *gst_get_color_space_string (GstJPEG2000SamplingColorSpace
+    colorspace);
+GstJPEG2000SamplingColorSpace gst_get_color_space (const gchar *

What we usually do for these two kinds of functions is from_string / to_string.

@@ +119,3 @@
+#define GST_J2K_SIZE_OF_MARKER      4
+#define GST_J2K_SIZE_OF_BOX_ID      4
+#define GST_J2K_SIZE_OF_BOX_LEN        4

JPEG2000 instead of J2K. All needs to be made more consistent :)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.