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 710392 - geometrictransform: fix setting black background for AYUV buffers
geometrictransform: fix setting black background for AYUV buffers
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.2.0
Other All
: Normal normal
: 1.2.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-10-17 13:38 UTC by Antonio Ospite
Modified: 2013-11-09 09:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
geometrictransform: fix setting black background for AYUV buffers (1.82 KB, patch)
2013-10-17 13:38 UTC, Antonio Ospite
needs-work Details | Review
[v2] geometrictransform: fix setting black background for AYUV buffers (1.85 KB, patch)
2013-10-31 10:06 UTC, Antonio Ospite
committed Details | Review

Description Antonio Ospite 2013-10-17 13:38:36 UTC
Created attachment 257539 [details] [review]
geometrictransform: fix setting black background for AYUV buffers

When the frame buffer is AYUV writing all zeros does not set it to black, in YUV colorspace 0x80 is the black level for chrominance and 0x10 is the black level for luminance.

The attached patch fixes setting the black background when the out_frame format is AYUV; in all the other supported formats zeroing the data with memset is still the right thing to do.

The issue can be reproduced with this command line:

gst-launch-1.0 videotestsrc ! video/x-raw,format="AYUV",width=640,height=480 ! rotate angle="0.79" ! autovideoconvert ! autovideosink

The background looks greenish while a black one is expected.

After the patch the same command line gives the expected result.

BTW, does GStreamer have a more general way to set colors depending on the pixelformat?


A more complete test can be performed with a script like:

#!/bin/sh

set -e

FORMATS="ARGB BGR BGRA BGRx RGB RGBA RGBx AYUV xBGR xRGB GRAY8 GRAY16_BE GRAY16_LE"
for format in $FORMATS;
do
  echo "Testing format: $format"
  GST_PLUGIN_PATH=$(pwd) \
  gst-launch-1.0 videotestsrc ! video/x-raw,format="$format",width=640,height=480 ! rotate angle="0.79" ! autovideoconvert ! autovideosink > /dev/null 2>&1
done
Comment 1 Sebastian Dröge (slomo) 2013-10-30 17:35:57 UTC
Review of attachment 257539 [details] [review]:

::: gst/geometrictransform/gstgeometrictransform.c
@@ +246,3 @@
+     * 0x10 is black for Y; components order is VUYAVUYA... */
+    for (int i = 0; i < out_frame->map[0].size; i += 4)
+      *((guint32 *) (out_data + i)) = 0x808010ff;

This is not going to work properly on big endian systems. Use GST_WRITE_UINT32_BE(0xff008080) here instead
Comment 2 Antonio Ospite 2013-10-31 10:06:18 UTC
Created attachment 258639 [details] [review]
[v2] geometrictransform: fix setting black background for AYUV  buffers

Here is a second iteration of the patch.

Changes since v1:
  - use GST_WRITE_UINT32_BE()
  - adjust the comment to match the BE order of the components
  - adjust the commit message to mention luminance before chrominance,
    and add the link to the bug report

Thanks,
   Antonio
Comment 3 Sebastian Dröge (slomo) 2013-10-31 14:03:50 UTC
commit ae2231624c92953ea3dfb32a36a8ff2fb927da20
Author: Antonio Ospite <ospite@studenti.unina.it>
Date:   Thu Oct 17 12:53:31 2013 +0200

    geometrictransform: Fix setting black background for AYUV buffers
    
    When the frame buffer is AYUV writing all zeros does not set it to
    black, in YUV colorspace 0x10 is the black level for luminance and 0x80
    is the black level for chrominance.
    
    Fix setting the background to black when the out_frame format is AYUV;
    in all the other supported formats zeroing the data with memset is still
    the right thing to do.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=710392