Issues :: JPEG Encoder

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 <= (others => '0'); size_wr <= '0'; hr_we <= '0'; hr_data <= (others => '0'); hr_waddr <= (others => '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 => '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" =>
        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" =>
        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" =>
        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" =>
        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 --> done.
      when others =>
        null;
    end case;
    size_wr_cnt <= size_wr_cnt + 1;
    hr_we <= '1';
  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

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;

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;

Browse

JPEG Encoder