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 627768 - add NV12 support to textoverlay
add NV12 support to textoverlay
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal blocker
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-08-23 20:41 UTC by Rob Clark
Modified: 2010-08-30 22:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to add NV12 support (4.60 KB, patch)
2010-08-23 20:42 UTC, Rob Clark
committed Details | Review
fix Cb/Cr inversion that previously existed with NV12/NV21/UYVY (2.76 KB, patch)
2010-08-30 20:23 UTC, Rob Clark
committed Details | Review

Description Rob Clark 2010-08-23 20:41:35 UTC
patch attached
Comment 1 Rob Clark 2010-08-23 20:42:54 UTC
Created attachment 168590 [details] [review]
patch to add NV12 support
Comment 2 Sebastian Dröge (slomo) 2010-08-24 07:26:40 UTC
commit 1b6918709c4bb843a6dbd932f292d45519d15dcf
Author: Rob Clark <rob@ti.com>
Date:   Mon Aug 23 13:59:38 2010 -0500

    textoverlay: add NV12 support
    
    Fixes bug #627768.
Comment 3 Sebastian Dröge (slomo) 2010-08-24 17:51:22 UTC
Apparently something is wrong here with the chroma. If you choose a color that has cb!=cr you'll see that the chroma planes are both swapped. This should be blue, not red:

gst-launch-0.10 videotestsrc ! "video/x-raw-yuv,format=(fourcc)NV12,width=800,height=600" ! clockoverlay color=0xff0000ff font-desc="Sans 30" ! ffmpegcolorspace ! ximagesink
Comment 4 Sebastian Dröge (slomo) 2010-08-24 17:52:19 UTC
Unfortunately I don't see the mistake here, it seems to be written in the correct order and it's done in the same order as for I420 where it works.
Comment 5 Rob Clark 2010-08-24 18:09:35 UTC
hmm, maybe offset to first pixel of U is off by one?  Ok, well, I'll check into this in the evening
Comment 6 Rob Clark 2010-08-25 02:43:35 UTC
well, I can reproduce same issue.. with above pipeline I get red (whereas I420 I get blue)..  I can't help but think there is an off by one somewhere, which would show up this way as inverted Cr/Cb with NV12/NV21 but may be less noticed with I420.. but I'll have to debug tomorrow.
Comment 7 Sebastian Dröge (slomo) 2010-08-25 13:39:54 UTC
Marking as blocker as this should get fixed before 0.10.31 or reverted
Comment 8 Rob Clark 2010-08-30 20:23:36 UTC
Created attachment 169124 [details] [review]
fix Cb/Cr inversion that previously existed with NV12/NV21/UYVY

In case of odd values for xpos or ypos, the division by two in CbCr
plane would result in an off-by-one error, which in the case of NV12,
NV21, or UYVY would cause inversion of blue and red colors.  (And
would be not so easily noticed for I420 as it would just cause the
chroma to be offset slightly from the luma.)

This patch also fixes a silly typo from the earlier patch which
added NV12 support that broke UYVY support.
Comment 9 Sebastian Dröge (slomo) 2010-08-30 20:56:19 UTC
commit 54f4aa28c227825e48fd7940b3566630a10cffe9
Author: Rob Clark <rob@ti.com>
Date:   Mon Aug 30 14:59:22 2010 -0500

    textoverlay: fix Cb/Cr inversion for colored text overlays
    
    In case of odd values for xpos or ypos, the division by two in CbCr
    plane would result in an off-by-one error, which in the case of NV12,
    NV21, or UYVY would cause inversion of blue and red colors.  (And
    would be not so easily noticed for I420 as it would just cause the
    chroma to be offset slightly from the luma.)
    
    This patch also fixes a silly typo from the earlier patch which
    added NV12 support that broke UYVY support.
Comment 10 David Schleef 2010-08-30 22:56:45 UTC
As an aside, is there a reason for using NV12?