[Bug 745144] gstvalue: Make sure GST_FOURCC_ARGS produces printable characters

[GStreamer (GNOME Bugzilla)]

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

Tim-Philipp Müller <t.i.m at zen.co.uk> changed:

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

--- Comment #2 from Tim-Philipp Müller <t.i.m at zen.co.uk> ---
Comment on attachment 297853
  --> https://bugzilla.gnome.org/attachment.cgi?id=297853
gstvalue: Make sure GST_FOURCC_ARGS produces printable characters

>-#define GST_FOURCC_ARGS(fourcc) \
>-        ((gchar) ((fourcc)     &0xff)), \
>-        ((gchar) (((fourcc)>>8 )&0xff)), \
>-        ((gchar) (((fourcc)>>16)&0xff)), \
>-        ((gchar) (((fourcc)>>24)&0xff))
>+
>+#define GST_PRINT_CHAR(fourcc, shift) \
>+  g_ascii_isprint((fourcc)>>shift) ? (fourcc)>>shift : '.'
>+#define GST_FOURCC_ARGS(fourcc)          \
>+    GST_PRINT_CHAR(fourcc, 0),           \
>+    GST_PRINT_CHAR(fourcc, 8),           \
>+    GST_PRINT_CHAR(fourcc, 16),          \
>+    GST_PRINT_CHAR(fourcc, 24)

To me, passing the shift argument to GST_PRINT_CHAR seems a bit weird. If
you're making this a publically usable define with the name _CHAR in it, it
should just take a single char/uchar IMHO. We also seem to be dropping the
explicit &0xff which I think would be good to keep.

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