Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Major refactoring proposal #280

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

abratchik
Copy link

@abratchik abratchik commented Dec 2, 2022

Hi, @easytarget , I'd like to propose these changes in order to further expand the idea of can web server based on ESP32 CAM board and its clones.

Key changes in this PR:

  1. Using SD card or built-in LittleFS storage for hosting the web server pages (HTML, CSS, JS) instead of byte arrays in memory. This makes the WebUI development much more convenient; standard development IDE for Web apps can be used. Besides, WebUI part can be swapped just by replacing the SD card with different UI design.
  2. Replaced myconfig.h configuration approach with JSON-based configs, similar to the one used for camera settings. Details are in README.md
  3. Refactored Web UI while keeping the original design and concept. The new Web UI is also easily customizable and very compact so that it can fit the LittleFS storage.
  4. Refactored the code from C to C++ object hierarchy, eliminating extensive use of global variables. Root abstract class CLAppComponent has been introduced. Key application components - CAppCam (camera manager), CLAppHttpd (webserver) and CLAppConn (network manager) inherit from CLAppComponent. This allows to encapsulate the global variables based on their purpose and ensure proper visibility and protection.
  5. Ported Web Server to ESPAsyncWebServer library. This library is light-weight and very powerful and flexible, allowing to reduce the required amount of HTTP server boilerplate code dramatically.
  6. Ported JSON parsing to Espressive native json_parser and json_generator libs. They are very light weight but allow to parse and generate JSON quite easily and effectively.
  7. Ported video streaming to Web Socket (thanks to ESPAsyncWebServer). This allows to eliminate the requirement of two HTTP ports - just one is sufficient. This reduce the load on the web server and allows to achieve maximum possible frame rates and resolution on the given hardware.
  8. Refactored the /dump page so it is no longer refreshing the whole page but just pulls the JSON data from the server instead. This allows to increase the refresh rate without significant load on the web server.
  9. Updated documentation (README, API). I apologize for dropping some of photos but I think it is better to make this example less personalized since it can be used in many other projects.
  10. Added doxygen-generated documentation in the ./Docs/html folder.
  11. Added configuration of the WiFi/Network parameters through the web page.
  12. Added support of PWM on GPOI pins - - this can be very useful for implementing some of the use cases (for example, PTZ servo control)
  13. Added multi-stream support (viewing video in 2 or more separate browser sessions, depending on the configuration)
  14. Support basic user authentication.
  15. Support of newer ESP32-S3 SoC chip (tested on LILYGO T-SIMCAM development board)

Fixes
#272 - fixed. AP configuration is perfrmed in JSON config files, see README
#279 - internal CLAppConn class does the same job but is within the project class hierarchy thus making the code more compact and managed under the project.
#158 - fixed
merges PRs:
#249 - changes proposed in the PR ported to the new UI.
#192 - implemented, see API documentation.
#51 - implemented
#54 - implemented

@abratchik abratchik changed the title Major refactroring proposal Major refactoring proposal Dec 2, 2022
@daschr
Copy link

daschr commented Dec 2, 2022

Great work! This looks like a great progress to a more stable project. Out of curiosity, what framerates do you get while streaming the video? For me, the multistream-feature is really the most desired one, would be great if it gets finally implemented as default in the firmware.

@abratchik
Copy link
Author

Hi @daschr thank you. Maximum resolution (1600x1200) on 12 fps is what I was getting. I found that the camera module gets heated and starts responding with delays if you set the FPS and resolution too high. Besides, the color noise increases, which is expected. In my projects, 800x600 on 25 fps is good enough.
Multi-stream feature can be also implemented as the same frame can be broadcasted to all websocket clients, this is a standard feature. I just don't need it for my project, may be we can add a bit later.

@daschr
Copy link

daschr commented Dec 2, 2022

Hi @abratchik, that sounds awesome! I'll try your fork out when I'm home.

@TungstenE2
Copy link

TungstenE2 commented Dec 22, 2022

great work! thx a lot.
did you consider #249 as well?

@TungstenE2
Copy link

@easytarget push for this ;-)

@abratchik
Copy link
Author

great work! thx a lot. did you consider #249 as well?

Hi @TungstenE2, I checked the PR #249 - will have to merge it manually as there is no more HTML in C/C++ code. All the HTML/CSS/JS moved to the /data folder, which needs to be copied either to the SD card or to the internal storage formatted as LittleFS.

The changes you proposed will go to the /data/js/cam.js - ths is where all camera-related parameter controls are defined.

@TungstenE2
Copy link

@abratchik how about an auto reboot option?
See: #266

@TungstenE2
Copy link

@abratchik does your new version also support OTA updates as the old version did? I have 2 cams installed in places that are not easy to access for wired updating.

@abratchik
Copy link
Author

abratchik commented Dec 26, 2022

@abratchik does your new version also support OTA updates as the old version did? I have 2 cams installed in places that are not easy to access for wired updating.

of course. Moreover, you can also configure it via GUI. Navigate to the http://<your-ip:your-port>/setup - OTA is configured right there.
However, there is a little problem. You can update the sketch but not the data (which is hosting the front end). For that, some development still needs to happen.

On the other hand, you can keep your front end UI anywhere on your network, not necessarily on the device itself. Basically, you just need JSON API calls and Websocket operations for the image and video streaming. The front end could be good for demo and trial purposes but not for production, due to security considerations. I'd say security is one of the biggest issues in the current implementation. We will need to lock JSON and Websocket and change HTTP to HTTPS, otherwise this implementation will not be good for production. Same problem was also valid for the previous versions.

@TungstenE2
Copy link

@abratchik tried to follow instructions on https://github.com/abratchik/esp32-cam-webserver/tree/8f39567ff1b99d146bea02aacda1163ca8be5341

Get 2 error:

Mehrere Bibliotheken wurden für "WiFi.h" gefunden
In file included from C:\Users\xxx\Documents\Arduino\esp32-cam-webserver-8f39567ff1b99d146bea02aacda1163ca8be5341\esp32-cam-webserver\esp32-cam-webserver.ino:6:
Benutzt: C:\Users\xxx\AppData\Local\Arduino15\packages\esp32\hardware\esp32\2.0.6\libraries\WiFi
C:\Users\xxx\Documents\Arduino\esp32-cam-webserver-8f39567ff1b99d146bea02aacda1163ca8be5341\esp32-cam-webserver\src\app_httpd.h:9:10: fatal error: ESPAsyncWebServer.h: No such file or directory
#include "ESPAsyncWebServer.h"
^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
Nicht benutzt: C:\Program Files (x86)\Arduino\libraries\WiFi
exit status 1
Fehler beim Kompilieren für das Board ESP32 Dev Module.

@TungstenE2
Copy link

TungstenE2 commented Jan 1, 2023

@abratchik I can confirm that /setup page is now loading. But changes done are not save, even by pressing save button.
I did all changes to connect to my wifi, provided host name and TZ. Pressed save button and cofirmed.
Clicked the System Button which tries to load /dump page using https instead of http (already reported some days ago) Manually changing to http loaded /dump page.

Going back to /setup or sever restart is loading with defauld AP mode, and not in wifi mode.

conn.json is created, but not all parameters provided are saved.

mdns_name ok
ssid missing
pass incomplete (special character % in pass is missing)

After editing the incomplete conn.json camara is connecting to wifi.

Camera defaults are still not loaded and all fields are blank.
Setting them all and saving&confirm.
Doing reboot, all cam settings are still blank.
Seems to be new bug, as this was working before.
Seems cam.json is not created on sd card.

grafik

Please check again.

@abratchik
Copy link
Author

Hi @TungstenE2 thanks again.

The latest fix should help with the issues you reported.

The only issue I could not pinpoint yet is supporting '%' symbol in the password. Somehow this character is dropped when saving to JSON, need further research. There is a workaround though - whenever you need to enter '%' in the password,entering it twice gives proper result. For example, if the password is 'abcd%' then entering 'abcd%%' will lead to saving just one '%'.

Issues with saving /conn.json were related to the bug in saving special characters to JSON. Now it should work fine.

Problem with /cam.json should be fixed as well. I tested on my setup - seems to be ok.

@TungstenE2
Copy link

@abratchik /setup is broken again. Did you test yourself before?

grafik

/system and /dump look ok.

grafik
grafik

@abratchik
Copy link
Author

@abratchik /setup is broken again. Did you test yourself before?

apparently not enough, sorry for that. I was testing on proxy so proxy cash had been masking the problem, my mistake. Hopefully it will be better this time :)

@TungstenE2
Copy link

TungstenE2 commented Jan 4, 2023

@abratchik ok, seems to work now, but I found new bugs....

Doing initial setup incl changing cam paramters will create conn.json, cam.json and http.json.
Edit conn.json to add % in wifi password.
After reboot cam will not start up and LED is solid red.
Deleted cam.json and http.json, LED still solid red.
Deleted conn.json as well, LED will blink and cam starts in AP mode.
Just configured wifi in AP mode including %% in password and rebooting worked. Cam starts up normal.

Starting stream is turning light on, although light is set to 0.
Need to set lamp on and off again to turn it of. This is reproducible each time stream is started and was not the case before.

The click flow for the buttons and their namings and the page names is iritating and confusing. This should be improved for better UX.

Other bugs reported before are also still unfixed.

@abratchik
Copy link
Author

abratchik commented Jan 4, 2023

Hi @TungstenE2, thanks again for testing!

Regarding the '%', as I already mentioned, still need some research. The workaround is to use double %% whenever necessary. Other symbols in passwords should not break anything.

The boot process is can be only impacted by the conn.json. Other config files have no effect on it. The design of this process is as follows: during the start, the sketch checks if the config is present. If it is then it loads the data from it and tries to initiate the WiFi. If the board is configured by the user as AP (accesspoint=true) then it will skip WiFi LAN and go directly to AP mode. However, if the board was configured for WiFi (accesspoint=false and SSID/password provided) then it will be trying to connect to the known WiFi in the loop. The loop will continue indefinitely, and the board will not start until a successful LAN connection is made. So, the behavior you observed is correct, except the "percent" problem, will fix it later.

Regarding the flash lamp, there are 2 sliders now. One is "Light" and the other is "Flash Light". The fist one is an immediate change of the lamp (so you can switch it on and off and regulate light intensity real time). The second one is a level at which the flash is activated when the "auto flash" mode is on. So, if the flash level is set to non-zero, the flash lamp will trigger during picture snap and will turn on during the stream start, regardless of the Light level setting. Once the stream is over, the lamp should go off as well. is it consistent with the behavior you are observing?

On the UI, I'm not the usability expert, just tried to keep the UI as close as possible to the previous version look and feel. However, one of the key reasons of this refactoring was to de-couple the frontend UI from the server code. Now it is super easy- you can change the web UI part the way you like, it is stored in the flash drive as normal HTML/CSS/JS code. If you have specific recommendations on the UI improvement, please suggest.

What other bugs are you referring to, could you please summarize? I know one you mentioned before related to MJPEG support. I will probably look into it later but cannot promise it will be supported. Good news are that the AsyncWebServer does support chunked responses but I still need to play around a bit with it. The way it was implemented in the previous version is - the chunked response for the video stream was processed in a loop with delay() calls between the frames. This approach is not good from the overall stability and performance standpoint, and will not work for with the AsyncWebServer.

@TungstenE2
Copy link

@abratchik I know about the % issue and added it manually to the file. This workes before. But somehow this broke the bootup this time.

Also I know about the 2 sliders, but the behaviour of the light slider broke in the latest fix. Please check.

May be we should do a video call, this might be more efficient than reporting here.

I like the idea of decoupeling but currently there are to many small bugs so I do not recomment @easytarget to pull your new version as v5.0.

@abratchik
Copy link
Author

Sure, lets have a call :) I cannot speak right now, in 45 min ok? Are you using Zoom?

@TungstenE2
Copy link

ok, send you an email

@abratchik
Copy link
Author

Update: small UI issues fixed as requested. Experiments with MJPEG were not very successful, unfortunately. I was able to make it working but frame rates were much worse, compared even to the original version (before refactoring). I believe the reason is: the old version was sending frames in one chunk while the AsyncTCP/AsynWebServer doesn't allow that (max chunk buffer length is around 5k). As a result, frames on higher resolution are sent in chunks, which makes the streaming very slow and unstable.

Summarizing, I think MJPEG is to be dropped. It is obsolete anyways, uses low level protocol (TCP) as the transport, no stream control etc. Switching to websocket is much better.

@TungstenE2
Copy link

@abratchik thx for the fixes and effort.

Altough the refactoring seems to be a good move, I see in my use cases missing functionality I am depending on. eg a dedicated video streaming URL which can be called by monitoring clients like TinyCam app. As already stated MJPEG is not a must as other protocolls are also available, but seems like this is not doable in the refactored version but is availabe in current version v4 by @easytarget.
Also Auto lamp function was working just fine before the refactoring and is currently not usable. The lastest fix now turns on the lamp 100% each time the stream is started. The logic from @easytarget was much better.

Multi client streaming will also not be available, which would have been a big improvement and was not only requested by me.

Host name is not properly supported and device name is not showing up properly on router.

Device name can not be set and to be used in html page titel, to easily identify the right browser tab.

Ideally host name and device name should be identical.

In general I like the refactoring, but for me I see no benefit as I will not gain new or better functionality, altough the code has been improved.

So all in all I do not recommend to merge the project by @easytarget as to many things are differnt from the current v4 version. Code updates are allways a good idea, but some of the logic is completly changed and existing functionality is missing and will not be supported anymore. Video stream will only be availabe in browser. May be both projects/version should stay independent, so users can choose the version on their needs, but it should be noted in documentation what the differences are, so people can understand and decide, based on use cases.

A good improvement I see in the latest fix, the stream is not freezing that often any more.

@daschr
Copy link

daschr commented Jan 10, 2023

@abratchik capacitors are already on all my ESP32 cams

@daschr did you test already as well? Do get to display /setup and /system dring initial wifi setup? For me it is not working.

@TungstenE2 No, I did not test it since I need multiclient streaming anyway (the reason I made #237).

I'm also worried whether this whole project is still maintained by @easytarget, since there was few progress the last half year.

@abratchik
Copy link
Author

abratchik commented Jan 10, 2023

@abratchik capacitors are already on all my ESP32 cams
@daschr did you test already as well? Do get to display /setup and /system dring initial wifi setup? For me it is not working.

@TungstenE2 No, I did not test it since I need multiclient streaming anyway (the reason I made #237).

I'm also worried whether this whole project is still maintained by @easytarget, since there was few progress the last half year.

good timing :) Multi-stream support just released, you can test. By the way, what was the maximum frame rate you were getting in your solution (#237)? I was able to stream 640x48x12FPS on two windows without any noticeable impact on performance.

The flash lamp is obviously a problem. The logic in my version is the following - the first client triggers the flash lamp and starts the stream. The second and subsequent ones are just connecting to the existing stream started by the 1st client, and inherit its settings, including the flash lamp state. If the first client terminates, the others can still continue - the lamp will be shut down only once the last client is disconnected.

@abratchik
Copy link
Author

Hi @TungstenE2 thanks for the feedback and testing anyways.

Regarding MJPEG - agree with you, if it is a mandatory requirement in your setup then you probably should stick to the old version. Unfortunately, there is no way for MJPEG to be added back. The way this protocol is working will not allow effective multi-streaming. Big advantage of MJPEG is compatibility, of course, since the protocol itself is very old. But on the flip side, it takes a significant toll on the server resources , which is critical for embedded SoC systems like ESP32. One can say that MJPEG is like a PS/2 port - it was a mainstream standard but you cannot find it anymore, since better interface alternatives are available ))

I checked the flash lamp, indeed here was a bug, thanks for catching, fixed.

Multi-streaming is just released, you are welcome to test ;)

Host/device name - good question. In fact, I did not change the code from @easytarget, which sets this name, but it seems to be not working. Does it work for you in the version 4? I did a quick research on the code, it should not work for old version either.

Page title is set in the httpd.json, parameter my_name:

{
    "my_name": "MY_NAME",
    "lamp":0,
    "autolamp":true,
    "flashlamp":100,
    "pwm": [{"pin":4, "frequency":50000, "resolution":9, "default":0}],
    "mapping":[ {"uri":"/img", "path": "/www/img"},
                {"uri":"/css", "path": "/www/css"},
                {"uri":"/js", "path": "/www/js"}],
    "debug_mode": false
}

Works ok, using it in my project.

@daschr
Copy link

daschr commented Jan 10, 2023

@abratchik capacitors are already on all my ESP32 cams
@daschr did you test already as well? Do get to display /setup and /system dring initial wifi setup? For me it is not working.

@TungstenE2 No, I did not test it since I need multiclient streaming anyway (the reason I made #237).
I'm also worried whether this whole project is still maintained by @easytarget, since there was few progress the last half year.

good timing :) Multi-stream support just released, you can test. By the way, what was the maximum frame rate you were getting in your solution (#237)? I was able to stream 640x48x12FPS on two windows without any noticeable impact on performance.

The flash lamp is obviously a problem. The logic in my version is the following - the first client triggers the flash lamp and starts the stream. The second and subsequent ones are just connecting to the existing stream started by the 1st client, and inherit its settings, including the flash lamp state. If the first client terminates, the others can still continue - the lamp will be shut down only once the last client is disconnected.

I am getting the same or even higher rate with more parallel clients. I'll test your pull request tomorrow. But I think I'll fork this project and patch it to fit my own needs anyway.

@TungstenE2
Copy link

@daschr what kind of clients are you taling about? Browser clients or an application client? What kind of streaming protocol are you using? Also blob as implemted in this version?

@daschr
Copy link

daschr commented Jan 11, 2023

@TungstenE2 I use browsers (mostly chrome) and the VLC mediaplayer. The streaming protocol is the same like in the master branch, so content-replace with jpeg images.

@daschr
Copy link

daschr commented Jan 11, 2023

@TungstenE2 I use browsers (mostly chrome) and the VLC mediaplayer. The streaming protocol is the same like in the master branch, so content-replace with jpeg images.

Oh, and you can also play/record the stream directly with ffmpeg f.e. using ffplay http://:81/ which is also quite handy for video surveillance.

@TungstenE2
Copy link

TungstenE2 commented Jan 11, 2023

@TungstenE2 I use browsers (mostly chrome) and the VLC mediaplayer. The streaming protocol is the same like in the master branch, so content-replace with jpeg images.

Oh, and you can also play/record the stream directly with ffmpeg f.e. using ffplay http://:81/ which is also quite handy for video surveillance.

@daschr so did I in the past, but with latest changes, this is no longer supported. See previous communication,

@Kenajcrap
Copy link

Kenajcrap commented Sep 12, 2023

Oh, and you can also play/record the stream directly with ffmpeg f.e. using ffplay http://:81/ which is also quite handy for video surveillance.

how were you able to play the contents of this fork with ffmpeg? Trying to to input it in a similar way to the mjpeg stream of the original repo (but adding http basic auth) yielded:

[http @ 0x559798cf00] Last chunk received, closing conn
2023-09-11 21:12:18.750965942  [AVIOContext @ 0x5597989d50] Statistics: 5164 bytes read, 0 seeks
2023-09-11 21:12:18.750979775  http://[REDACTED_USER]:[REDACTED_PASS]@[REDACTED_IP]/view?mode=stream: Invalid data found when processing input```

@easytarget
Copy link
Owner

@abratchik @TungstenE2 @daschr and Co..

Apologies for going offline here, I got rather overwhelmed by life issues and this Project was one of the things I had to give up.. I should have archived it or something but I was thinking entirely correctly. ESP completely breaking, the re-breaking the Camera code didn't help my sense of humour either.

Looking through this it seems obvious to me that I should really be pointing people to @abratchik 's fork and making it clear that I am not really maintaining this. I'm cleaning some stuff up at the moment..

Thoughts? is that A good idea, Is there an even better fork I should consider? Is there a better page where all the different firmwares are discussed?..

In the meantime I intend to try this PR and see how it goes, but I want to point new arrivals at a better fork.

Mike Pichl and others added 2 commits March 31, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants