CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
QR code (encoding process) #17889
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
QR code (encoding process) #17889
Conversation
Thanks for your suggestions.+decode_error QRDecode::read_format(uint16_t& format , int which){ + /*version<=6*/ + int i; + uint16_t my_format = 0; + + Mat mat_format(1,FORMAT_LENGTH,CV_8UC1,Scalar(0));\I use cv::Mat only because it is easy to print by using "std::cout". And I did not add the usage in my code(I forget that...). I will add that later.
…------------------ Original ------------------
From: "ann"<notifications@github.com>;
Date: 2020年7月30日(星期四) 凌晨3:02
To: "opencv/opencv"<opencv@noreply.github.com>;
Cc: "413366511"<woaiwo6666@qq.com>; "Author"<author@noreply.github.com>;
Subject: Re: [opencv/opencv] WIP:QR code (decoding process) (#17889)
@APrigarina commented on this pull request.
In modules/objdetect/src/qrcode.cpp:
> +#define MAX_VERSION 40 +#define MAX_ALIGNMENT 7
These macros are already defined above
In modules/objdetect/src/qrcode.cpp:
> vector<Point2f> original_points; + + uint8_t orignal_data[MAX_PAYLOAD]; + uint8_t rearranged_data[MAX_PAYLOAD]; + uint8_t final[MAX_PAYLOAD];
it's not recommended to use keywords for variable names
In modules/objdetect/src/qrcode.cpp:
> + memset(final,0,sizeof(uint8_t)*MAX_PAYLOAD); + memset(payload,0,sizeof(uint8_t)*MAX_PAYLOAD); + payload_len=0; +} + +/* + * params @ format(uint16_t for returning the format bits) which(select from two different place) + * return @ can be correct or not + */ + +decode_error QRDecode::read_format(uint16_t& format , int which){ + /*version<=6*/ + int i; + uint16_t my_format = 0; + + Mat mat_format(1,FORMAT_LENGTH,CV_8UC1,Scalar(0));
what is the reason of use cv::Mat instead of std::vector and how is this variable used?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
modules/objdetect/src/qrcode.cpp
Outdated
version_index = -1; | ||
/**find the version according to the number of data codewords*/ | ||
for( i = version_begin ; i < version_end ; i++ ){ | ||
const BlockParams * tmp_ecc_params = &(version_info_database[i].ecc[ecc_level]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you really need pointer in this case?
It is good way if you avoid pointers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think pointers maybe take less space and can be used in a faster way, (although may be not that safe..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use shared pointers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find that I can't 'make' normally when I use "share_ptr".It says " error: ‘shared_ptr’ in namespace ‘std’ does not name a template type
std::shared_ptr version_info ;
"
So what should I do now ? Looking forword to your help!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/home/truth/github/opencv/modules/objdetect/src/qrcode.cpp:2290:10: error: ‘shared_ptr’ in namespace ‘std’ does not name a template type
std::shared_ptr version_info ;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can try smart pointers in opencv (here)
I don't think it is necessary to use smart pointers here, because they are pointing to static variables and are not involved with memory allocation and free. So can I just use the normal pointers?
…------------------ 原始邮件 ------------------
发件人: "ann"<notifications@github.com>;
发送时间: 2020年9月28日(星期一) 晚上8:33
收件人: "opencv/opencv"<opencv@noreply.github.com>;
抄送: "413366511"<woaiwo6666@qq.com>; "Mention"<mention@noreply.github.com>;
主题: Re: [opencv/opencv] WIP:QR code (decoding process) (#17889)
@APrigarina commented on this pull request.
In modules/objdetect/src/qrcode.cpp:
> + int versionAuto(const std::string & input_str); + +}; +/**findVersionCapacity + * @param input_length + * @param ecc_level + * @param mode + * @func find the version automatically + * @return the version index + * */ +int findVersionCapacity(const int &input_length ,const int &ecc_level ,const int & version_begin, const int & version_end ){ + int i,version_index,data_codewords; + version_index = -1; + /**find the version according to the number of data codewords*/ + for( i = version_begin ; i < version_end ; i++ ){ + const BlockParams * tmp_ecc_params = &(version_info_database[i].ecc[ecc_level]);
you can try smart pointers in opencv (here)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@ZhengQiushi Do you have any progress with the patch? The PR still does not pass CI checks. |
Hi, @asmorkalov. At the moment, I and @APrigarina are working on this PR. Some QR codes modes don't work correctly, we are trying to fix it |
jenkins cn please retry a build |
f55c4bb
to
d650c14
Compare
@allnes, @rayonnant14 please review |
fnc1_first = false; | ||
fnc1_second = false; | ||
fnc1_second_AI = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These variables are not used
std::string counter = decToBin(str_len, bits); | ||
loadString(counter, output, true); | ||
int i; | ||
for (i = 0; i < str_len; i += 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (i = 0; i < str_len; i += 2) | |
for (i = 0; i < str_len - 1; i += 2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@APrigarina Thank you for update!
Could you please add links/information somewhere about used format specification?
int version = 0, int correction_level = CORRECT_LEVEL_L, | ||
int mode = QR_MODE_AUTO, int structure_number = 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameters should be split on 2 parts:
- properties of algorithm - should go into constructor/
create()
methods - sample processing properties (output_size)
This is useful to unify algorithms and provide one interface.
@param structure_number The optional number of QR codes to generate in Structured Append mode. | ||
@param output_size The optional size of generated QR code (by default is small and depends on version). | ||
*/ | ||
CV_WRAP bool generate(String input, OutputArray output, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generate => encode
remove unnecessary double spaces.
|
||
#include "precomp.hpp" | ||
#include "opencv2/objdetect.hpp" | ||
#include "opencv2/calib3d.hpp" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need calib3d?
#include "precomp.hpp" | ||
#include "opencv2/objdetect.hpp" | ||
#include "opencv2/calib3d.hpp" | ||
#include <iostream> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iostream
should not be used in OpenCV modules.
Use logging instead.
Or properly target headers, like sstream
.
std::string decToBin(const int &format, const int &total); | ||
int eccLevelToCode(int level); | ||
uint8_t gfPow(uint8_t x, int power); | ||
uint8_t gfMul(const uint8_t& x, const uint8_t& y); | ||
Mat gfPolyMul(const Mat& p, const Mat& q); | ||
Mat gfPolyDiv(const Mat& dividend, const Mat& divisor, const int& ecc_num); | ||
Mat polyGenerator(const int& n); | ||
int getBits(const int& bits, const vector<uint8_t>& payload ,int& pay_index); | ||
void loadString(const std::string& str, vector<uint8_t>& cur_str, bool is_bit_stream); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static to make them local
QRCodeEncoder::QRCodeEncoder() : p(new Impl) {} | ||
QRCodeEncoder::~QRCodeEncoder() {} | ||
|
||
bool QRCodeEncoder::generate(cv::String input, cv::OutputArray output, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool
result is not informative from the user perspective.
How to troubleshot the false
result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cv::
should be removed.
return; // done | ||
} | ||
} | ||
std::cerr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FAIL() <<
QRCodeEncoder encoder; | ||
Mat result ; | ||
bool success = encoder.generate(original_info, result); | ||
ASSERT_TRUE(success) << "Can't generate qr image :" << name_test_image; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<< name_test_image
not necessary as this is a test parameter.
Mat src = imread(image_path, IMREAD_GRAYSCALE), straight_barcode; | ||
ASSERT_FALSE(src.empty()) << "Can't read image: " << image_path; | ||
|
||
bool eq = countDiffPixels(result, src) == 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NORM_L1 or _INF
std::string image_path = findDataFile(root +"/"+ encode_qrcode_images_name[i]); | ||
|
||
/**read from test set*/ | ||
Mat src = imread(image_path, IMREAD_GRAYSCALE), straight_barcode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
straight_barcode
Avoid multiple declarations on the single line (allowed for simple cases without initialization only).
Must be defined near its usage.
Sure, here is ISO standard and wiki useful information |
c6ca15a
to
2663910
Compare
@alalek Please review |
@param encoded_info Input string to encode. | ||
@param qrcode Generated QR code. | ||
*/ | ||
CV_WRAP virtual void encode(String encoded_info, OutputArray qrcode) = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String
"const reference"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const String&
should be used for input parameters.
here and below
output_bits.insert(output_bits.end(), bin_number.begin(), bin_number.end()); | ||
} | ||
|
||
static int eccLevelToCode(int level) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int => QRCodeCorrectionLevel
size_t mode_count = static_cast<size_t>(mode_list.size()); | ||
ASSERT_GT(mode_count, 0u) << "Can't find validation data entries in 'test_images': " << dataset_config; | ||
|
||
int modes[] = {1, 2, 4}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
magic numbers or QREncodeMode
?
CV_WRAP Params(); | ||
CV_PROP_RW int version; | ||
CV_PROP_RW int correction_level; | ||
CV_PROP_RW int mode; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int correction_level
int mode
Can we use enums here?
const Size fixed_size = Size(200, 200); | ||
const float border_width = 2.0; | ||
|
||
int establishCapacity(int mode, int version, int capacity) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
int mode
Please use enum type
EncodingMethods result = strategy[strategy.size() - 1]; | ||
for (size_t i = 0; i < result.blocks.size(); i++) | ||
{ | ||
AutoEncodePerBlock result_block = result.blocks[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EncodingMethods result
AutoEncodePerBlock result_block
reference or const reference
for (size_t j = 0; j < str_len; j++) | ||
{ | ||
previous = strategy[j]; | ||
std::string sub_string = cur_string.substr(j, str_len - j); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.substr(j, str_len - j)
The second argument is not needed here: https://en.cppreference.com/w/cpp/string/basic_string/substr
EncodingMethods previous; | ||
EncodingMethods new_method; | ||
new_method.len = ERROR_MODE_OCCUR; | ||
for (size_t j = 0; j < str_len; j++) | ||
{ | ||
previous = strategy[j]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EncodingMethods previous;
Not used outside of for
body, so it should be declated inside.
} | ||
else | ||
{ | ||
size_t str_len = cur_string.length(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cur_string.length()
should be equal to i
.
enum ECIEncodings { | ||
ECI_UTF8 = 26 | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
ecc_level = parameters.correction_level; | ||
mode_type = parameters.mode; | ||
struct_num = parameters.structure_number; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not all variables are initialized in ctor:
int version_size;
int mask_type;
uint32_t eci_assignment_number;
uint8_t parity;
uint8_t sequence_num;
uint8_t total_num;
@param encoded_info Input string to encode. | ||
@param qrcode Generated QR code. | ||
*/ | ||
CV_WRAP virtual void encode(String encoded_info, OutputArray qrcode) = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const String&
should be used for input parameters.
here and below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@APrigarina Great job!
Merge with extra: opencv/opencv_extra#807
ISO standard and wiki useful information.