litex-hub / litevideo

Small footprint and configurable video cores (Deprecated)
Other
70 stars 25 forks source link

csc.ycbcr2rgb: Signed multiplication #35

Open gregdavill opened 4 years ago

gregdavill commented 4 years ago

I've started using the YCbCr2RGB along with YCbCr422_444 on a project converting FLIR Boson thermal camera to HDMI.

Hit some small issues I wanted to write down before I forget.

I'd like to look into these issue in a bit more detail, but don't have the cycles right now, I suspect these might be result of using a different verilog frontend during synthesis, vivado/yosys.

These are the changes I had to make:

diff --git a/litevideo/csc/ycbcr2rgb.py b/litevideo/csc/ycbcr2rgb.py
index 50b2789..360090d 100644
--- a/litevideo/csc/ycbcr2rgb.py
+++ b/litevideo/csc/ycbcr2rgb.py
@@ -70,17 +71,17 @@ class YCbCr2RGBDatapath(Module):
         # (cb - coffset)*bcoef
         # (cr - coffset)*ccoef
         # (cb - coffset)*dcoef
-        y_minus_yoffset = Signal((ycbcr_w + 1, True))
+        y_minus_yoffset = Signal((ycbcr_w + 4, True))
         cr_minus_coffset_mult_acoef = Signal((ycbcr_w + coef_w + 4, True))
         cb_minus_coffset_mult_bcoef = Signal((ycbcr_w + coef_w + 4, True))
         cr_minus_coffset_mult_ccoef = Signal((ycbcr_w + coef_w + 4, True))
         cb_minus_coffset_mult_dcoef = Signal((ycbcr_w + coef_w + 4, True))
         self.sync += [
             y_minus_yoffset.eq(ycbcr_delayed[1].y - coefs["yoffset"]),
-            cr_minus_coffset_mult_acoef.eq(cr_minus_coffset * coefs["acoef"]),
-            cb_minus_coffset_mult_bcoef.eq(cb_minus_coffset * coefs["bcoef"]),
-            cr_minus_coffset_mult_ccoef.eq(cr_minus_coffset * coefs["ccoef"]),
-            cb_minus_coffset_mult_dcoef.eq(cb_minus_coffset * coefs["dcoef"])
+            cr_minus_coffset_mult_acoef.eq(cr_minus_coffset * Signal((ycbcr_w, True), reset=coefs["acoef"]  )),
+            cb_minus_coffset_mult_bcoef.eq(cb_minus_coffset * Signal((ycbcr_w, True), reset=coefs["bcoef"]  )),
+            cr_minus_coffset_mult_ccoef.eq(cr_minus_coffset * Signal((ycbcr_w, True), reset=coefs["ccoef"]  )),
+            cb_minus_coffset_mult_dcoef.eq(cb_minus_coffset * Signal((ycbcr_w, True), reset=coefs["dcoef"]  ))
         ]

         # stage 3

Without y_minus_yoffset's width increased I was getting white pixels when Y saturated, instead of the correct colour. Without wrapping the coefs in Signed signals the signed multiplication was not being inferred correctly.

enjoy-digital commented 3 years ago

Thanks for the feedback @gregdavill, this will be useful for future use.