rmhsawyer/ec601-final-project-mapping_user_face_to_emoji

Project Description: an App that translates user's expression into emoji based on Android

ec601-final-project-mapping_user_face_to_emoji's Issues

Comments needed

https://github.com/rmhsawyer/EC601-Final-Project/blob/master/live-plotting.py#L11

This function could use a comment or two describing what it does.

comment needed

https://github.com/rmhsawyer/EC601-Final-Project/blob/874055bd07443f7a51d2ca022c200a1cf506b154/real-time.py#L38

This could use a comment to explain what is going on with faces and why the values used were chosen

real-time.py "magic" numbers

Lines 38-55 contains a lot of code that is not commented. A lot of numbers are hard coded here without any explanation/reason for the numbers.

Starting and Stopping buttons

Tested program (on 11/12/2017) and found that starting the program properly initialized the webcam and worked fine. However, after hitting stop and then start again, webcam would not reappear. Reset button only helped some of the time, but did not always fix this issue.

Tested on Windows 10, Firefox 57.0 (64-bit).

getting an error at this line when trying to run your code

https://github.com/rmhsawyer/EC601-Final-Project/blob/874055bd07443f7a51d2ca022c200a1cf506b154/real-time.py#L35

I am getting the following error when I try to run your code:

cv2.error: /opt/conda/conda-bld/opencv_1491943970124/work/opencv-3.1.0/modules/imgproc/src/color.cpp:7456: error: (-215) scn == 3 || scn == 4 in function ipp_cvtColor

Is it maybe because you need to give a time delay between when you start the videocapture and when you call the while loop? Or you can also try this:

if ret is True:
img_gray = cv2.cvtColor(frame, cv2.COLOR_BGR2GRAY)
else:
continue

Can you comment why you have chosen 1.1 as a scale factor?

https://github.com/rmhsawyer/EC601-Final-Project/blob/874055bd07443f7a51d2ca022c200a1cf506b154/emoji.py#L92

Please comment about this line why you have chosen this particular scaling factor? The same question stands for the next 2 lines as well

Comment on line caffe_functions.py L89

Some comments could be placed on this line to explain why the values for X, Y , C were choosen

https://github.com/rmhsawyer/EC601-Final-Project/blob/8c57c4bb40c89cf912777cd5e6e161a14927a879/caffe_functions.py#L89

same comments in your codes

emotion 2 not used

https://github.com/rmhsawyer/EC601-Final-Project/blob/874055bd07443f7a51d2ca022c200a1cf506b154/TFDroid-master/emoji.py#L72

You can delete this line . Its declared and not returned

Emoji Me app, android version?

Should list which version of android this is programmed for.

Combine lines 206 and line 202

The initialization of these variables could all be on the same line.

https://github.com/rmhsawyer/EC601-Final-Project/blob/8c57c4bb40c89cf912777cd5e6e161a14927a879/caffe_functions.py#L206

process_dataset.py

Hi in your process.py file the start up script may have an improvement in your start up scripts by having single_face and useMean set to true outside the if statements.

Emojis File

In the Emoji file if the .png files are imported from some where and you did not make them yourself you may want to credit where the artwork came from.

Code Review #2

I am glad to see that you have resolved most of the issues that I had bought up previously. There are still a few issues that I found that needs to be rectified.

1. You have improved your GitHub Readme from before. However, it still needs to explain the project
   pipeline and major technologies being used properly.
2. In the file emoji.py, you have used a threshold of 0.8. However, it is not clear as to how or why you
   used this threshold.
3. In the same file emoji.py, you have used the scaling factor to be 1.1. Why have you used this value?
   Also, shouldn’t the scaling factor be dynamic, so that it can be viewed on different screens? I have
   the same issue with the next 2 lines of code as well, where you have arbitrarily assigned 5 to
   minNeighbors without explaining why, and used the size of 30 by 30 for minSize.
4. When I try to run your code using the commandline command python real-time.py haar-
   cascade_frontalface_default.xml, it gives me an error saying: error: (-215) scn == 3 || scn == 4 in
   function ipp_cvtColor. Is it maybe because you need to give a time delay between when you start
   the videocapture and when you call the while loop? Or you can also try this:
   if ret is True:
   img_gray = cv2.cvtColor(frame, cv2.COLOR_BGR2GRAY)
   else:
   continue

Overall, I think your project is very interesting and you have done a very good job in carrying it out. I am looking forward to see your finished project during the poster presentation. Good luck!

Jaffe Dataset

This project uses the Jaffe dataset, however it does not link to this set nor mentions any licensing/usage terms for this dataset. Could be problematic depending on their usage terms.

opencv_functions.py if else statement around Line 114

https://github.com/rmhsawyer/EC601-Final-Project/blob/8c57c4bb40c89cf912777cd5e6e161a14927a879/opencv_functions.py#L114

This H and W values can be declared outside the IF Else to save space (2 less lines of code)

Line 208 comment section of code

This loop could use a description of what the code is trying to accomplish

https://github.com/rmhsawyer/EC601-Final-Project/blob/8c57c4bb40c89cf912777cd5e6e161a14927a879/caffe_functions.py#L208

Code Review Two

I am happy to see that most of the problems I raised last time were fixed and the python code you now have is very clean.

I was unable to get your app to run in andriod studio so I did not focus on that as there are no directions saying how I would go about that (maybe you could put some if you would like feel back on that.

As the issues I raised individually state the problems in your code are limited this time around

1 https://github.com/rmhsawyer/EC601-Final-Project/blob/master/live-plotting.py#L11

This function could use a comment or two describing what it does so its not as hard to understand

2 in the Emoji.py file You can delete line 72. Its declared and not returned. It looks like it was going to be used but was commented out.

3 In the realtime.py file the faces varible could use a comment to explain what is going on with faces and why the values used were chosen

Overall this is a very neatly organized program with good code and you guys did a really good job at making a fun program for communicating with friends.

what is the threshold of 0.8 you are using?

https://github.com/rmhsawyer/EC601-Final-Project/blob/874055bd07443f7a51d2ca022c200a1cf506b154/emoji.py#L63

Can you explain as a comment why you are using the value threshold of 0.8 here?

Packages required and versions?

Tried to run real-time.py but could not figure out which combinations of packages were needed to run it. Should list in README.md which versions of Python, Keras, OpenCV, etc. are required to run.

landmarkrecognition.py line 77

In landmarkrecognition.py, line 77

face_name = []

This line is intending to empty the list declared in line 61. This actually declares a new empty list and binds it to variable face_name. Might be better to use:

face_name[:] = []

This will empty the list of values without creating a new empty list.

Summary of code review by Harish N Sathishchandra

Well done. Your project looks great. However, I found some isuues which needs some improvement.

1. Your GitHub Readme should be more detailed. Explain more about your project here and also
   explain what each module of your project does.
2. The file organization in your GitHub page needs improvement. For instance, the OpenCV and Caffe
   functions can be grouped into one folder. The dataset related files can also be grouped into a
   single folder. The same goes for all the WAV files as well.
3. The codes in general are well commented. However, I found the function listfile.py needs to be
   better commented to make it more understandable.
4. In caffe_functions.py, line 30 it isn’t clear why you have chosen vmin=0 and vmax=0.2.