Master al #8
Master al #8
Conversation
| #endif | ||
| #if defined(ESP32) | ||
| # include <FreeRTOS.h> | ||
| # include <AsyncTCP.h> |
There was a problem hiding this comment.
This means, this library won't compile when AsyncTCP library is not installed.
There was a problem hiding this comment.
AsyncTCP is part of Espressif arduino-esp32 (https://github.com/espressif/arduino-esp32/), so it will be there. AsyncTCP is the base class used by ESPAsyncWebServer (https://github.com/me-no-dev/ESPAsyncWebServer) which is much faster / better than default espressif WebServer.
There was a problem hiding this comment.
Does the library makes sense to be compiled for ESP8266 at all? If not the code can be reduced to just
# include <FreeRTOS.h>
# include <AsyncTCP.h>I am using your library in a project that compiles on ESP8266 and EPS32, thats where I copied the include code from.
src/internal/frame.hpp
Outdated
| @@ -46,17 +55,18 @@ class Frame | |||
| * \retval true writing completed. | |||
| * \retval false writing disrupted by timeout. | |||
| */ | |||
| bool | |||
| writeTo(Print& os, int timeout = 10000); | |||
| bool writeTo(Print& os, int timeout = 10000); | |||
There was a problem hiding this comment.
Do not change code style.
I use the NDN style: https://named-data.net/doc/ndn-cxx/current/code-style.html
| @@ -29,6 +29,103 @@ class CameraClass | |||
| bool | |||
| changeResolution(const Resolution& resolution, int sleepFor = 500); | |||
|
|
|||
| /** | |||
| * must be -2 to +2, default 0 | |||
There was a problem hiding this comment.
This isn't proper Doxygen syntax.
There was a problem hiding this comment.
Fixed with commit -a -amend, please verify if it is correct now - have not used doxygen yet
alorbach
force-pushed
the
master-al
branch
3 times, most recently
from
7f217b3
to
ac8a23e
Compare
|
have you confirmed the changes I made ? |
There was a problem hiding this comment.
The code lacks examples. The async functionality needs to have examples to demonstrate how it works.
#9 (comment)
fixed code style
- Add function to change frequence - Add function to overwrite frame data buffer which I need to convert JPG to BMP using PSRAM!
- white pixel correction - black pixel correction - denoise control - lens correction - raw gamma - changeFrequence of Cam Sensor from 10 to 20 mhz - AutoExposurecontrol - Dcw - AsyncClient: Handle Client Disconnect from Outside - fix merge conflicts
|
I will keep my branch rebased on your master as long as possible. |
In order to get more details on API failure reason, the return error code needs to be stored and evaluated.
Use bAbort flag to cancel streaming from outside task
| * \param iEnable must be 0(disable) or 1 (enable) | ||
| */ | ||
| bool | ||
| changAutoExposurecontrol(int iEnable); |
There was a problem hiding this comment.
Some drive-by comments while I was looking at OV2640 libraries...
There are a bunch of methods named chang.... I think you mean change...
| bool | ||
| changDenoise(int iEnable); | ||
|
|
||
| /** \brief Enable/Disable Lenc Correction |
There was a problem hiding this comment.
Some questions will be answered here:
https://github.com/espressif/esp32-camera/blob/e689c3b082985ee7b90198be32d330ce51ac5367/sensors/ov2640.c
And some you need to research in ov2640 sensor doc (Or whatever sensor you have)
https://www.uctronics.com/download/cam_module/OV2640DS.pdf
I remember the lenc and rawgma had big impact on how low light is being handeled.
There was a problem hiding this comment.
And some infos can be found here and here:
https://github.com/espressif/esp32-camera/blob/e689c3b082985ee7b90198be32d330ce51ac5367/sensors/private_include/ov2640_settings.h
https://github.com/espressif/esp32-camera/blob/e689c3b082985ee7b90198be32d330ce51ac5367/sensors/private_include/ov2640_regs.h
| * \param iEnable must be 0(disable) or 1 (enable) | ||
| */ | ||
| bool | ||
| changRawGMA(int iEnable); |
| bool | ||
| changRawGMA(int iEnable); | ||
|
|
||
| /** \brief Enable/Disable DCW |
| * \param iGainCeiling must be between GAINCEILING_2X and GAINCEILING_128X | ||
| */ | ||
| bool | ||
| changGainceilingSensor(int iGainCeiling); |
There was a problem hiding this comment.
Wouldn't that be changeGainCeilingSensor to match the iGainCeiling parameter?
Can GAINCEILING_nX be any integer between 2 and 128, in which case shouldn't it just be an integer? If it's an enum, change the parameter description to reference the enum?
And maybe drop Sensor entirely? Having sensor in the name is either redundant because it's implicit that you're manipulating some of the sensor settings, or it's wrong because it suggests that there is some kind of a sensor that measures something about the gain, like there might be temperature sensor.
There was a problem hiding this comment.
Possible that some settings did not work at all.
I wanted to have as many sensor options available for testing as possible,
Made many changes to add support for more camera sensor properties, AsyncClient support (ESPAsyncWebServer) and much more. Feel free to use my branch, it's last rebase is from 2020-12-06.
See espressif/esp32-camera@ae32d52#diff-a00662a8f0e0727f691f0ff27d0dd00a3721195193dad7b9b3dffb3f4d8591cc
For now I will maintaine rebase support for my branch as long as possible.