[Bug 766236] rtp j2k payload/depayload messes up colours in sample pattern

[GStreamer (GNOME Bugzilla)]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #328326|none                        |reviewed
             status|                            |

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

::: gst/rtp/gstrtpj2kcommon.h
@@ +52,3 @@
+#define GST_RTP_J2K_YBR420    "YCbCr-4:2:0"
+#define GST_RTP_J2K_YBR410    "YCbCr-4:1:0"
+#define GST_RTP_J2K_GRAYSCALE "GRAYSCALE"

These here are the standard ones, maybe in the header of the other one with
YBRA we should specifically mark that one as non-standard (putting it at the
bottom there, maybe having the defines contain a marker but not the string
values).

::: gst/rtp/gstrtpj2kdepay.c
@@ +57,3 @@
     GST_STATIC_CAPS ("application/x-rtp, "
+        "media = (string) \"video\", " "clock-rate = (int) 90000, "
+        GST_RTP_J2K_SAMPLING_LIST "," "encoding-name = (string) \"JPEG2000\"")

This breaks ABI as it won't work with older versions anymore. Which might be OK
as streams were just not standards compliant at all

Needs a second opinion but I think it's ok. If we don't require the sampling,
then we can only invent a colorspace (sYUV as before) and have absolutely no
idea for the sampling.

::: gst/rtp/gstrtpj2kpay.c
@@ +200,3 @@
+  guint32 num_components;
+  guint32 dx;
+  guint32 dy;

You only initialize these but never set them to anything meaningful

@@ +260,3 @@
+          guint32 len = gst_rtp_j2k_pay_header_size (data, offset);
+          GST_LOG_OBJECT (pay, "found SIZ at %u", offset);
+          state->num_components = GST_READ_UINT16_BE (data + offset + (2 +
34));

The value of num_components seems to be unused. You just set it

@@ +397,3 @@
+  state.dx = 1;                 /* no subsampling in x direction */
+  state.dy = 1;                 /* no subsamplingin y direction */
+  state.num_components = 3;     /* 3 components */

Maybe initialize this with the values from the caps?

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