Adding Huffman to JPEG encoder #3

ishan98 · 2017-08-31T12:44:08Z

This includes the addition of Huffman module to the JPEG encoder with Test Bench :

Huffman Core :
The module takes a serial input_data and generate the encoded data corresponding to the input by making the use of Huffman tables.
AC & DC ROM :
Provide the appropriate encoded values required for encoding the input data.
Table builder:
Makes the huffman tables by reading the .csv files in the directory.

Test Bench :
new_huffman_tb.py
This module is made for the testing of the Huffman Encoder.

Common.py :
Added input_data for testing the Huffman module and various functions such as setdata() to get the input and output in the required format.

After adding the above changes, the resultant repository contains the working model of the Huffman module along with the test suite.

mithro

Just some formatting and commenting fixes and we can merge this!

mithro · 2017-09-06T06:02:54Z

litejpeg/core/huffman/ac_cr_rom.py

+def ac_cr_rom(self,address1,address2,data_out_size,data_out_code):
+
+    code, size = build_huffman_rom_tables(
+    '/home/ishan/gsoc/environment/litejpeg-master/litejpeg/core/huffman/ac_cr_rom.csv')


Umm, this is a path to your computer :-P

You want to use file to find the path of this .py file. See https://docs.python.org/2/library/runpy.html and https://stackoverflow.com/questions/7116889/python-file-attribute-absolute-or-relative

Something like this should work

import os.path HUFFMAN_DIR = os.path.dirname(__file__) HUFFMAN_CSV = os.path.join(HUFFMAN_DIR, 'ac_cr_rom.csv')

mithro · 2017-09-06T06:05:33Z

litejpeg/core/huffman/ac_cr_rom.csv

@@ -0,0 +1,253 @@
+0b1010,4


It would be good if this file had some comments at the top describing what this file is used for, why it exists and what the values mean.

To read such a file, you'll need to do something like described here -> https://stackoverflow.com/questions/14158868/python-skip-comment-lines-marked-with-in-csv-dictreader or https://wuhrr.wordpress.com/2016/02/16/process-csv-with-comments/

mithro · 2017-09-06T06:09:25Z

litejpeg/core/huffman/ac_rom.csv

@@ -0,0 +1,253 @@
+0b1010,4


Same as the other .csv file.

mithro · 2017-09-06T06:09:47Z

litejpeg/core/huffman/dc_cr_rom.csv

@@ -0,0 +1,13 @@
+0b00,2


Same as other csv file.

mithro · 2017-09-06T06:11:53Z

litejpeg/core/huffman/ac_rom.py

+        address = Signal(len(self.address1)+len(self.address2))
+        raddr = Signal(len(self.address1)+len(self.address2))
+
+        self.sync += raddr.eq(address)


self.comb += [ address.eq(Cat(self.address1,self.address2)), rom_code_port.adr.eq(raddr), self.data_out_code.eq(rom_code_port.dat_r), rom_size_port.adr.eq(raddr), self.data_out_size.eq(rom_size_port.dat_r), ]

mithro · 2017-09-06T06:22:36Z

litejpeg/core/huffman/huffmancore.py

+        ]
+
+        # Handling FIFO write.
+        self.sync += [


The indenting here is a bit weird...

For the If statements can you follow the style I describe here -> timvideos/getting-started#43

mithro · 2017-09-06T06:23:37Z

litejpeg/core/huffman/huffmancore.py

+
+
+class HuffmanEncoder(PipelinedActor, Module):
+    """


Some extra newlines in this comment would be good.

mithro · 2017-09-06T06:26:22Z

test/common.py

+        These matrixes were taken from [`huffman_test_inputs.py` in cfelton's test_jpeg code]
+        (https://github.com/cfelton/test_jpeg/blob/master/test/huff_test_inputs.py).
+        The matrix is an example of what the quantization module might produce.
+        """


Indenting...

self.vli_test_y = [ 10, 1, 2, 4, 8, 16, 32, 65, 128, 256, 1000, 1, 2, 4, 8, 16, 32, 65, 128, 256, 1000, 1, 2, 4, 8, 16, 32, 65, 128, 256, 1, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 2, 2, 2, 2, 3, 3, 3, 2, ]

This doesn't look to be done?

@mithro, Do you want something else because I have already indented the values written in the matrix in proper order.
Kindly update if anything is missing.

See my example in the first comment here -> https://github.com/timvideos/litejpeg/pull/3/files#r137180177

This comment is redirecting to the same page, doesn't able to understand.

mithro · 2017-09-06T06:27:17Z

test/model/enc_frame.py

+            size.append(row[1])
+    code = tuple(code)
+    size = tuple(size)
+    return code, size


This function seems to appear twice.

@mithro , I am not able to find the function twice.

mithro · 2017-09-06T06:30:41Z

test/new_huffman_tb.py

+from litejpeg.core.common import *
+from litejpeg.core.huffman.huffmancore import HuffmanEncoder
+
+from common import *


Newline before the comment.

mithro

Almost there - just a few more style comments.

mithro · 2017-10-03T11:23:48Z

test/common.py

+        These matrixes were taken from [`huffman_test_inputs.py` in cfelton's test_jpeg code]
+        (https://github.com/cfelton/test_jpeg/blob/master/test/huff_test_inputs.py).
+        The matrix is an example of what the quantization module might produce.
+        """


See my example in the first comment here -> https://github.com/timvideos/litejpeg/pull/3/files#r137180177

mithro · 2017-10-03T11:24:51Z

litejpeg/core/huffman/tablebuilder.py

+import csv
+
+
+def build_huffman_rom_tables(csvfile):


This function is defined once here.
def build_huffman_rom_tables(csvfile):

mithro · 2017-10-03T11:25:30Z

test/model/enc_frame.py

 def rle_code(block):
    return list(chain(*[(symbol,len(list(g))) for symbol, g in groupby(block)]))

+def build_huffman_rom_tables(csvfile):


This function is defined again here....
def build_huffman_rom_tables(csvfile):

mithro · 2017-10-03T11:27:14Z

test/new_huffman_tb.py

+from common import *
+
+"""
+Testbanch for the Huffman module.


Testbanch.

Can you run spell check on all the comments? What do you use for an editor?

I am using sublime as my text editor, working on that.

mithro · 2017-10-03T11:27:55Z

test/new_rom_tb.py

+        print((yield dut.data),"  ",(yield dut.data_size))
+    yield
+    print((yield dut.data),"  ",(yield dut.data_size))
+


There are too many newlines between stuff here.

mithro · 2017-10-03T11:31:43Z

test/new_huffman_tb.py

+        "sys" :   [main_generator(tb),
+                   tb.streamer.generator(),
+                   tb.logger.generator()]
+    }


generators = { "sys": [ main_generator(tb), tb.streamer.generator(), tb.logger.generator(), ] }

mithro · 2017-10-03T11:33:43Z

litejpeg/core/huffman/huffmancore.py

+        ]
+
+        # Handling FIFO write.
+        self.sync += [


For the If statements can you follow the style I describe here -> timvideos/getting-started#43

mithro · 2017-10-03T11:35:08Z

litejpeg/core/huffman/huffmancore.py

+
+        ]
+
+        # Calculating the number of blocks to go as output.


The number of spaces for indenting in this file is wrong. You should always be using 4 spaces.

anuejn · 2019-02-28T20:13:41Z

what holds this from being merged?

ishan98 added 17 commits July 19, 2017 21:21

Adding Huffman tables and ROM

b2ea52a

Adding ROM Testbench

7c5930c

Updating ROM

0a2185f

Adding TableBuilder

9c6f993

Adding AC-DC ROM and TestBench

090a1de

Adding Huffman state machine

38ece29

Adding Test Bench for Huffman

0aed5fb

Updating Huffmancore

b4e992c

Adding changes to huffmancore

ebfa964

Adding Reference Huffman Model

d836556

Updating HuffmanEncoder

a2b22e1

Adding Documentation to Huffman

d785810

Code CleanUp

a5b0249

Update v1

93054dc

Updating Huffman V2

464b23d

Updating Common

577599f

Updating Huffman v2

0892da7

mithro reviewed Sep 6, 2017

View reviewed changes

ishan98 added 2 commits September 8, 2017 01:25

Updated Huffman V3

65b00f8

Updating Huffman V4

5e5d580

mithro reviewed Oct 3, 2017

View reviewed changes

Updating Huffman V3

a2c1394

Adding Huffman to JPEG encoder #3

Are you sure you want to change the base?

Adding Huffman to JPEG encoder #3

Conversation

ishan98 commented Aug 31, 2017

mithro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mithro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anuejn commented Feb 28, 2019 • edited

anuejn commented Feb 28, 2019 •

edited