0
\$\begingroup\$

Hi I am working on an image processing project in which I try to change brightness of a pixel. The code I wrote works but not in the way I want it to be. Here is the code segment I use to change brightness of the pixel. What happens here is that I check RGB values of a pixel in order to determine if those values are open to change. To dim the pixel, I substract 1 and to brigthen the pixel I add 1 to RGB values. However, this approach gives very ugly results. How can I modify the algorithm I am using? By the way I just shared the part used for brightness. Other part(Contrast) is not included here. That is the reason code seems incompleted. I am not expert nor knowledgable at image processing.

library IEEE;
use IEEE.STD_LOGIC_1164.all;
use IEEE.NUMERIC_STD.all;
use IEEE.std_logic_unsigned.all;
use IEEE.std_logic_arith.all;

entity Brightness_Contrast is
generic (
PICTURE_WIDTH_HEIGHT : integer := 250;
COLOR_BIT : integer := 4);
port (
clk_in : in  std_logic := '0';
operation : in std_logic_vector(1 downto 0);
rst: in std_logic;
data_in: in std_logic_vector(11 downto 0);
output_of_operation: out std_logic_vector(11 downto 0);
mode: in std_logic
);
end Brightness_Contrast;

architecture Behavioral of Brightness_Contrast is

signal check : std_logic_vector(11 downto 0) := (others => '0');

begin

       BRIGHTNESS_CONTRAST: process(rst,data_in,clk_in,operation,mode) is

       begin

       check <= data_in;

       if rst = '1' then

       check <= (others => '0');

       elsif rising_edge(clk_in) then --clk

              if operation = "00" then              

              if mode = '0' then 

              if check(3*COLOR_BIT-1 downto 2*COLOR_BIT) < "1110" then
              check(3*COLOR_BIT-1 downto 2*COLOR_BIT) <= ((check(3*COLOR_BIT- 
1 downto 2*COLOR_BIT)) + "0010");
              end if;

              if check(2*COLOR_BIT-1 downto COLOR_BIT) < "1110" then
              check(2*COLOR_BIT-1 downto COLOR_BIT) <= ((check(2*COLOR_BIT-1 
downto COLOR_BIT)) + "0010");
              end if;

              if check(COLOR_BIT-1 downto 0) < "1110" then
              check(COLOR_BIT-1 downto 0) <= ((check(COLOR_BIT-1 downto 0)) + 
"0010");
              end if;

              elsif mode = '1' then 

              if  check(3*COLOR_BIT-1 downto 2*COLOR_BIT) > "0001" then
              check(3*COLOR_BIT-1 downto 2*COLOR_BIT) <= ((check(3*COLOR_BIT- 
1 downto 2*COLOR_BIT)) - "0010");
              end if;

              if  check(2*COLOR_BIT-1 downto COLOR_BIT) > "0001" then
             check(2*COLOR_BIT-1 downto COLOR_BIT) <= ((check(2*COLOR_BIT-1 
downto COLOR_BIT)) - "0010");
              end if;

              if check(COLOR_BIT-1 downto 0) > "0001" then
              check(COLOR_BIT-1 downto 0) <= ((check(COLOR_BIT-1 downto 0)) - 
"0010");
              end if;   
 elsif operation = "01" then   

          if mode = '0' then  

          if   check(3*COLOR_BIT-1 downto 2*COLOR_BIT) < "1000" and  
check(3*COLOR_BIT-1 downto 2*COLOR_BIT) > "0001" then

          check(3*COLOR_BIT-1 downto 2*COLOR_BIT) <=  ((check(3*COLOR_BIT-1 
downto 2*COLOR_BIT)) - "0010"); 

          elsif  check(3*COLOR_BIT-1 downto 2*COLOR_BIT) > "0111" and 
check(3*COLOR_BIT-1 downto 2*COLOR_BIT) < "1110" then

          check(3*COLOR_BIT-1 downto 2*COLOR_BIT) <=  ((check(3*COLOR_BIT-1 
downto 2*COLOR_BIT)) + "0010") ;
          end if;

          if   check(2*COLOR_BIT-1 downto COLOR_BIT) < "1000" and  
check(2*COLOR_BIT-1 downto COLOR_BIT) > "0001" then

          check(2*COLOR_BIT-1 downto COLOR_BIT) <=  ((check(2*COLOR_BIT-1 
downto COLOR_BIT)) - "0010");

          elsif  check(2*COLOR_BIT-1 downto COLOR_BIT) > "0111" and  
check(2*COLOR_BIT-1 downto COLOR_BIT) < "1110" then

          check(2*COLOR_BIT-1 downto COLOR_BIT) <=  ((check(2*COLOR_BIT-1 
downto COLOR_BIT)) + "0010") ;
          end if;

          if   check(COLOR_BIT-1 downto 0) < "1000" and  check(COLOR_BIT-1 
downto 0) > "0001" then

          check(COLOR_BIT-1 downto 0) <=  ((check(COLOR_BIT-1 downto 0)) - 
"0010");

          elsif  check(COLOR_BIT-1 downto 0) > "0111" and  check(COLOR_BIT-1 
downto 0) < "1110" then

          check(COLOR_BIT-1 downto 0) <=  ((check(COLOR_BIT-1 downto 0)) + 
"0010") ;
          end if;                                      

          elsif mode = '1' then                    

          if  check(3*COLOR_BIT-1 downto 2*COLOR_BIT) < "1000" and 
check(3*COLOR_BIT-1 downto 2*COLOR_BIT) > "0010" then

          check(3*COLOR_BIT-1 downto 2*COLOR_BIT) <= ((check(3*COLOR_BIT-1 
downto 2*COLOR_BIT)) - "0011");

          elsif check(3*COLOR_BIT-1 downto 2*COLOR_BIT) > "0111" and 
check(3*COLOR_BIT-1 downto 2*COLOR_BIT) < "1101" then

          check(3*COLOR_BIT-1 downto 2*COLOR_BIT) <= ((check(3*COLOR_BIT-1 
downto 2*COLOR_BIT)) +"0011") ;
          end if;

          if  check(2*COLOR_BIT-1 downto COLOR_BIT) < "1000" and 
check(2*COLOR_BIT-1 downto COLOR_BIT) > "0010" then

          check(2*COLOR_BIT-1 downto COLOR_BIT) <= ((check(2*COLOR_BIT-1 
downto COLOR_BIT)) - "0011");

          elsif check(2*COLOR_BIT-1 downto COLOR_BIT) > "0111" and 
check(2*COLOR_BIT-1 downto COLOR_BIT) < "1101" then

          check(2*COLOR_BIT-1 downto COLOR_BIT) <= ((check(2*COLOR_BIT-1 
downto COLOR_BIT)) +"0011") ;
          end if;

          if  check(COLOR_BIT-1 downto 0) < "1000" and check(COLOR_BIT-1 
downto 0) > "0010" then

          check(COLOR_BIT-1 downto 0) <= ((check(COLOR_BIT-1 downto 0)) - 
"0011");

          elsif check(COLOR_BIT-1 downto 0) > "0111" and check(COLOR_BIT-1 
downto 0) < "1101" then

          check(COLOR_BIT-1 downto 0) <= ((check(COLOR_BIT-1 downto 0)) + 
"0011") ;
          end if;

          end if; --mode                      
          end if; --Operation                                                   
          end if; --CLK

          end process BRIGHTNESS_CONTRAST ;

          output_of_operation <= check;


 end Behavioral;
\$\endgroup\$
  • \$\begingroup\$ It's quite difficult to understand what your goal is. Could you specify what you mean by "a very ugly result"? What exactly do you do with your check signal? Is this a port that is connected to RGB LEDs? \$\endgroup\$ – po.pe May 7 '18 at 11:09
  • \$\begingroup\$ Where is the end of your process and what are you doing with the operation and mode signals? As VHDL code is "executed" concurrently having only parts of your code makes it difficult to get the big picture. \$\endgroup\$ – po.pe May 7 '18 at 11:16
  • \$\begingroup\$ This is a component and output_of_operation is connected to input of ram. Check is internal signal for data_in which comes from output of ram. I mean by ugly is that when I change brightness, the area whose brightness is changed turns to black or white with pink wavey fluctuations. From Ram Rgb leds are lighted \$\endgroup\$ – runo May 7 '18 at 11:17
  • \$\begingroup\$ I added rest of the code \$\endgroup\$ – runo May 7 '18 at 11:19
  • \$\begingroup\$ How do you make sure that your whole dimming is executed just once? \$\endgroup\$ – po.pe May 7 '18 at 11:22
1
\$\begingroup\$

This is not an answer per se, but I couldn't post in comments, so, a couple questions/comments for you... I have not spent a whole lot of time thinking about this, so take this FWIW, and apply it as you see fit:

  • Are you simulating this, or going directly to HW? I'm guessing HW, since you said "ugly"... you really need to simulate your design to see what is happening. You have no visibility in HW.

  • Every time you shift by a bit, you either multiply by 2 (SHL), or divide by 2 (SHR)... this is an implementation detail, but a fundamental concept in digital design... you could just as easily write (newValue = value*2 for SHL)

  • I recommend, before you write HDL that you draw this in HW first (gates, FFs, FSMs, memories, etc.) and make sure your design appears correct before writing HDL. Writing SW-centric code for HDL can get you in trouble... you can get away with it sometimes, but, if for a digital design class your professor won't be impressed... well, I wouldn't anyway.

  • Maybe pare it down... comment out the upper 8-bits, and just operate on the lower 4... get rid of all the generics and write something very simple, hard-coded and easier to see what you are doing to get confident with what you are doing

  • You have a boundary condition you have not dealt with... it is possible to overflow your 4-bit values when you add to them (and thus end up with a large delta in the result when you truncate the upper bits by your 4-bit assignment). (e.g. 0xff + 0x2 = 0x101 = 0x01 for you... which means you just went from "white" to "black") Also, you are using type integer which is 32-bit value, thus maybe getting negative values(?) you can either use std_logic_vectors, and ignore the overflow bits, or place a range for your variable that use type integer (e.g. range = 1 to 4)

  • as part of the previous bullet, do you really need all 3 math libraries? Maybe just use std_logic_unsigned to prevent any negative stuff(?) you have pointed to: use IEEE.NUMERIC_STD.all; use IEEE.std_logic_unsigned.all; use IEEE.std_logic_arith.all;

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks but there is no 32 bit value in the code. Where is it actually? \$\endgroup\$ – runo May 7 '18 at 18:58
  • \$\begingroup\$ Type integer is a 32-bit value.... but, nevermind, you didn't do what i was talking about in terms of using an integer. You do however still have an issue for overflowing your 4-bit values, and "rolling over" to a value that is not just an increase of 1.and, so, too, my example uses 8-bits.. anyway, same concept.. you can go from white to black in one addition. Think about saturation... if you are 0xf and add 1, stay at 0xf, not go to 0x0 which will happen since your result will be 0x10 and you only use the lower 4-bits. just a boundary condition, likely something else is going on too. \$\endgroup\$ – CapnJJ May 7 '18 at 19:09
  • \$\begingroup\$ But those many if statements are for checking if there will be a possible overflow. If number is 12, I can add 1. If number is 15, I cannot add 1. I do not see a reason for overflow. \$\endgroup\$ – runo May 7 '18 at 19:16
  • \$\begingroup\$ ok, my bad... sorry, i should have looked more closely. 10-4, good job. did you sim it? Maybe I should have sim'ed to keep it a more relevant response :/ \$\endgroup\$ – CapnJJ May 7 '18 at 19:17
1
\$\begingroup\$

To increase the brightness of a pixel, one simple method is to multiply R,G,B of a pixel by a constant. Similarly to decrease the brightness, divide by a constant.

Since we are working in VHDL, its convenient to choose the constant to be a power of 2.

\$C = 2^n\$

So that multiplication and division will be simply left or right shift operations by n places.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ what are left/right shift operations may I ask?? \$\endgroup\$ – runo May 7 '18 at 11:04
  • \$\begingroup\$ simple vector shift : >> or << ....you should know these shift operators \$\endgroup\$ – Mitu Raj May 7 '18 at 11:05
  • \$\begingroup\$ eg: multiplying binary '1000' by 4 = left shifting '1000' by 2 = 100000 \$\endgroup\$ – Mitu Raj May 7 '18 at 11:07
  • \$\begingroup\$ Traditionally, increasing the brightness in this context means adding an offset. Also, multiplying by 2^n, while cheap, is a very large change. \$\endgroup\$ – pipe May 7 '18 at 11:08
  • \$\begingroup\$ So left shifting means adding zeros before the most significant bit and right shifting means adding zeros after the least significant bit. This operation looks like concetination right? \$\endgroup\$ – runo May 7 '18 at 11:21
1
\$\begingroup\$

Be aware that a signal is always assigned at the end of a process and to the last statement of the process. This means in your case check <= data_in will only be assigned if operation != 00 as otherwise one of the other assignments will be the last one in the process. Further you say that operation and mode are controlled from external switches. Consider that the process you have is clocked at several MHz, means if you press the button for 100ms (what's already super quick) the process has been executed thousands of times. What you need is an edge detection on your button input. Can you show where operation comes from?

Regarding signal assignments:

proc : process(rst, clk)
begin
  if (rst) then
    a <= '0';
    b <= '0';
  elsif rising_edge(clk) then
    a <= '1';
    b <= '1';
    b <= 'a';
  end if;
end process;

So if you have a process like above, the signals a and b are assigned at the end of the process, this is not executed sequential as e.g. in Java. This means the assignment b <= '1' will always be ignored as the latest assignment is b <= 'a'.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You are right sir. I could not see that. I truly thank you for this answer which I will benefit a lot in the following days. \$\endgroup\$ – runo May 7 '18 at 11:40
  • \$\begingroup\$ Can you explain in more detail "Be aware that a signal is always assigned at the end of a process and to the last statement of the process. This means in your case check <= data_in will only be assigned if operation != 00 as otherwise one of the other assignments will be the last one in the process.". I am not good at if statements. I know java and think of these if statements in the same way as those in java \$\endgroup\$ – runo May 7 '18 at 11:45
  • \$\begingroup\$ These are very basic concepts of VHDL and you might wanna read some lecture regarding this topic, but I'll edit my answer to cover this as I otherwise see a lot more questions upcoming... \$\endgroup\$ – po.pe May 7 '18 at 11:48
  • \$\begingroup\$ thanks a lot again but why when operation =00 check <= data in is not assigning \$\endgroup\$ – runo May 7 '18 at 12:01
  • \$\begingroup\$ To be honest I don't even exactly know what the synthesizer would do as this is an asynchronous assignment - you should not do assignments like that. Try do assign everything inside the rising_edge(clk) if statement. \$\endgroup\$ – po.pe May 7 '18 at 12:09
0
\$\begingroup\$

For someone who knows absolutely nothing about either hardware design or image processing, you certainly have jumped directly into the deep end with a VHDL project to process video in real time.

The question-and-answer format used here on SE is simply not capable of giving you the education you need in order to succeed. You need to takes courses and/or read books on the following topics:

  • digital logic design
  • HDL programming
  • image processing

... after which, you might be capable of asking specific, focused questions that we could actually handle here.

I would recommend starting with the topic of image processing, and learning how to do the manipulations you want using software before trying to implement them in hardware. You may find that you don't need to have custom hardware at all.

Since you're a Java programmer(?), you might want to check out ImageJ. It's a Java-based set of tools for image manipulation. You can also use a more general tool such as Gnu Octave, which might give you more insight into how the low-level operations are coded.

| improve this answer | |
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.