OpenCores

Quantization header missing update

Back to bugtracker overview.

Information:
Type :: BUG
Status :: OPENED
Assigned to :: nobody

Description:
In the latest SVN revision, there is a potential to corrupt the header data stored in JFIFGen. Specifically, the recommended initialization procedure is to write the image size, followed by the two quantization tables. However, the JFIFGen state machine is busy for five clock cycles after updating the image size, while it stores the new size information. If the first quantization table entry is written immediately afterward, then the state machine will still be busy when the new data arrives, and the update will be lost. The result is an incorrect entry in the final JPEG header.

My proposed fix is to prioritize incoming quantization table data, and write the updated size information while the memory is otherwise idle. See below:

Change #1: Shorten size_wr_cnt from 3 bits to 2 bits
signal size_wr_cnt : unsigned(1 downto 0);

Change #2: Prioritize updated size information
p_host_wr : process(CLK, RST)
begin
if RST = '1' then
size_wr_cnt '0');
size_wr hr_we hr_data '0');
hr_waddr '0');
elsif CLK'event and CLK = '1' then
hr_we
-- Note the fact that size has updated. Write the four bytes
-- during idle periods, so we don't accidentally clobber changes
-- to the quantization tables, etc.
if image_size_reg_wr = '1' then
size_wr_cnt '0');
size_wr end if;

-- Write any quantization table updates immediately.
if qwren = '1' then
-- luminance table select
if qwaddr(6) = '0' then
hr_waddr ( resize(unsigned(qwaddr(5 downto 0)),hr_waddr'length) +
to_unsigned(C_QLUM_BASE,hr_waddr'length));
else
-- chrominance table select
hr_waddr ( resize(unsigned(qwaddr(5 downto 0)),hr_waddr'length) +
to_unsigned(C_QCHR_BASE,hr_waddr'length));
end if;
hr_we hr_data -- Write the new image size when otherwise idle.
elsif size_wr = '1' then
case size_wr_cnt is
-- height H byte
when "00" =>
hr_data hr_waddr -- height L byte
when "01" =>
hr_data hr_waddr -- width H byte
when "10" =>
hr_data hr_waddr -- width L byte
when "11" =>
hr_data hr_waddr size_wr done.
when others =>
null;
end case;
size_wr_cnt hr_we end if;
end if;
end process;


This is just long enough that it can overlap with the first quantization table entry, and the updated table entry is lost. If the new quantization table differs from the

If This is enough time to receive the first entry in a new quantization table. a new size i

If a quantization table entry is written immediately after the image size is changed, then the JFIFGen RAM may miss the first entry or tw

Comments:

ness, ooter Aug 23, 2013
Reformatting, try #3:

  p_host_wr : process(CLK, RST)
  begin
    if RST = '1' then
      size_wr_cnt <= (others =%gt; '0');
      size_wr     <= '0';
      hr_we       <= '0';
      hr_data     <= (others =%gt; '0');
      hr_waddr    <= (others =%gt; '0');
    elsif CLK'event and CLK = '1' then
      hr_we <= '0';
    
      -- Note the fact that size has updated.  Write the four bytes
      -- during idle periods, so we don't accidentally clobber changes
      -- to the quantization tables, etc.
      if image_size_reg_wr = '1' then
        size_wr_cnt <= (others =%gt; '0');
        size_wr     <= '1';
      end if;
      
      -- Write any quantization table updates immediately.
      if qwren = '1' then
        -- luminance table select
        if qwaddr(6) = '0' then
          hr_waddr <= std_logic_vector
                        ( resize(unsigned(qwaddr(5 downto 0)),hr_waddr'length) + 
                          to_unsigned(C_QLUM_BASE,hr_waddr'length));
        else
          -- chrominance table select
          hr_waddr <= std_logic_vector
                        ( resize(unsigned(qwaddr(5 downto 0)),hr_waddr'length) + 
                          to_unsigned(C_QCHR_BASE,hr_waddr'length));
        end if;
        hr_we   <= '1';
        hr_data <= qwdata;
      -- Write the new image size when otherwise idle.
      elsif size_wr = '1' then
        case size_wr_cnt is
          -- height H byte
          when "00" =%gt;
            hr_data  <= image_size_reg(15 downto 8);
            hr_waddr <= std_logic_vector(to_unsigned(C_SIZE_Y_H,hr_waddr'length));
          -- height L byte
          when "01" =%gt;
            hr_data <= image_size_reg(7 downto 0);
            hr_waddr <= std_logic_vector(to_unsigned(C_SIZE_Y_L,hr_waddr'length));
          -- width H byte
          when "10" =%gt;
            hr_data <= image_size_reg(31 downto 24);
            hr_waddr <= std_logic_vector(to_unsigned(C_SIZE_X_H,hr_waddr'length));
          -- width L byte
          when "11" =%gt;
            hr_data <= image_size_reg(23 downto 16);
            hr_waddr <= std_logic_vector(to_unsigned(C_SIZE_X_L,hr_waddr'length));
            size_wr  <= '0';  -- Last byte --%gt; done.
          when others =%gt;
            null;
        end case;
        size_wr_cnt <= size_wr_cnt + 1;
        hr_we <= '1';
      end if;
    end if;
  end process;
  
ness, ooter Aug 23, 2013
Source code reformatted with HTML escape characters:

  p_host_wr : process(CLK, RST)
  begin
    if RST = '1' then
      size_wr_cnt %lt;= (others =%gt; '0');
      size_wr     %lt;= '0';
      hr_we       %lt;= '0';
      hr_data     %lt;= (others =%gt; '0');
      hr_waddr    %lt;= (others =%gt; '0');
    elsif CLK'event and CLK = '1' then
      hr_we %lt;= '0';
    
      -- Note the fact that size has updated.  Write the four bytes
      -- during idle periods, so we don't accidentally clobber changes
      -- to the quantization tables, etc.
      if image_size_reg_wr = '1' then
        size_wr_cnt %lt;= (others =%gt; '0');
        size_wr     %lt;= '1';
      end if;
      
      -- Write any quantization table updates immediately.
      if qwren = '1' then
        -- luminance table select
        if qwaddr(6) = '0' then
          hr_waddr %lt;= std_logic_vector
                        ( resize(unsigned(qwaddr(5 downto 0)),hr_waddr'length) + 
                          to_unsigned(C_QLUM_BASE,hr_waddr'length));
        else
          -- chrominance table select
          hr_waddr %lt;= std_logic_vector
                        ( resize(unsigned(qwaddr(5 downto 0)),hr_waddr'length) + 
                          to_unsigned(C_QCHR_BASE,hr_waddr'length));
        end if;
        hr_we   %lt;= '1';
        hr_data %lt;= qwdata;
      -- Write the new image size when otherwise idle.
      elsif size_wr = '1' then
        case size_wr_cnt is
          -- height H byte
          when "00" =%gt;
            hr_data  %lt;= image_size_reg(15 downto 8);
            hr_waddr %lt;= std_logic_vector(to_unsigned(C_SIZE_Y_H,hr_waddr'length));
          -- height L byte
          when "01" =%gt;
            hr_data %lt;= image_size_reg(7 downto 0);
            hr_waddr %lt;= std_logic_vector(to_unsigned(C_SIZE_Y_L,hr_waddr'length));
          -- width H byte
          when "10" =%gt;
            hr_data %lt;= image_size_reg(31 downto 24);
            hr_waddr %lt;= std_logic_vector(to_unsigned(C_SIZE_X_H,hr_waddr'length));
          -- width L byte
          when "11" =%gt;
            hr_data %lt;= image_size_reg(23 downto 16);
            hr_waddr %lt;= std_logic_vector(to_unsigned(C_SIZE_X_L,hr_waddr'length));
            size_wr  %lt;= '0';  -- Last byte --%gt; done.
          when others =%gt;
            null;
        end case;
        size_wr_cnt %lt;= size_wr_cnt + 1;
        hr_we %lt;= '1';
      end if;
    end if;
  end process;

Post a comment:
Login to post comments!

Back to bugtracker overview.

© copyright 1999-2019 OpenCores.org, equivalent to Oliscience, all rights reserved. OpenCores®, registered trademark.