-
Notifications
You must be signed in to change notification settings - Fork 354
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
base: master
Are you sure you want to change the base?
Conversation
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. |
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. |
Hi @abratchik, that sounds awesome! I'll try your fork out when I'm home. |
great work! thx a lot. |
@easytarget push for this ;-) |
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. |
@abratchik how about an auto reboot option? |
@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 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. |
@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 |
@abratchik I can confirm that /setup page is now loading. But changes done are not save, even by pressing save button. 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 After editing the incomplete conn.json camara is connecting to wifi. Camera defaults are still not loaded and all fields are blank. Please check again. |
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 Problem with |
@abratchik /setup is broken again. Did you test yourself before? /system and /dump look ok. |
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 :) |
@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. Starting stream is turning light on, although light is set to 0. 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. |
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 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. |
@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. |
Sure, lets have a call :) I cannot speak right now, in 45 min ok? Are you using Zoom? |
ok, send you an email |
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. |
@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. 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. |
@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. |
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 {
"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. |
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. |
@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? |
@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, |
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:
|
@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. |
Add a check whether the WebSocket buffer has space.
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:
./Docs/html
folder.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