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

Add Base64 support for FileStorage #6697

Merged
merged 11 commits into from Jun 30, 2016
Merged

Add Base64 support for FileStorage #6697

merged 11 commits into from Jun 30, 2016

Conversation

wiryls
Copy link
Contributor

@wiryls wiryls commented Jun 18, 2016

[GSoC 2016] FileStorage.

What does this PR change?

  • Add Base64 support for reading and writing XML\YML file.
// The two new functions for writing data:
void cvWriteRawData_Base64(cv::FileStorage & fs, const void* _data, int
len, const char* dt);
void cvWriteMat_Base64(cv::FileStorage & fs, cv::String const & name,
cv::Mat const & mat);
  • Change YML file header form YAML:1.0 to YAML 1.0. (standard format)
  • Add test for Base64 part.

[GSoC] FileStorage:
Add base64 support for reading and writting XML\YML file.
The two new functions:
```
void cvWriteRawData_Base64(cv::FileStorage & fs, const void* _data, int
len, const char* dt);
void cvWriteMat_Base64(cv::FileStorage & fs, cv::String const & name,
cv::Mat const & mat);
```
1. Add Base64 support for reading and writing XML\YML file.

The two new functions for writing:

```cpp
void cvWriteRawData_Base64(cv::FileStorage & fs, const void* _data, int
len, const char* dt);
void cvWriteMat_Base64(cv::FileStorage & fs, cv::String const & name,
cv::Mat const & mat);
```

2. Change YML file header form `YAML:1.0` to `YAML 1.0`. (standard
format)

3. Add test for Base64 part.
@vpisarev vpisarev self-assigned this Jun 18, 2016
@vpisarev
Copy link
Contributor

thanks! I will check it. Quick question - you've changed the header in YAML files written by OpenCV. Does the decoder support both the new and the old format? We want to be able to read all the previously stored .yml files.

@wiryls
Copy link
Contributor Author

wiryls commented Jun 18, 2016

Yes, it supports both the new and the old format.
I plan to add a test file for yml compatibility testing.

Two other test are still needed.

1. Verify the Base64 data.
2. Read an old YML file for compatibility test.
Two test are still needed:

1. Verify the Base64 data.
2. Read an old YML file for compatibility test.
@@ -1238,6 +1238,11 @@ inline String::String(const FileNode& fn): cstr_(0), len_(0) { read(fn, *this, *

//! @endcond


CV_EXPORTS void cvWriteRawData_Base64(::cv::FileStorage & fs, const void* _data, int len, const char* dt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, change the API the first parameter to "CvFileStorage* fs"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! About changing this API, there is one problem to be discussed.

cvWriteRawData_Base64 cannot be continually called, like:

// in `icvWriteMat`
for( y = 0; y < size.height; y++ )
    cvWriteRawData( fs, mat->data.ptr + (size_t)y*mat->step, size.width, dt );

When doing something like this, the output data cannot be merged into a single one. To solve this, I might need to add a private variable to CvFileStorage for recording status. Meanwhile, users must call cvStartWriteStruct(fs, name, CV_NODE_SEQ, "binary") themselves. It doesn't look good.

Another way, I can change the parameters to (CvFileStorage * fs, const char* name, const void* _data, int len, const char* dt) and treat it as a complete data type. But then this function must output all the data at once. Is this ok?

```cpp
cvWriteMat_Base64(::cv::FileStorage & fs, ::cv::String const & name,
::cv::Mat const & mat)
```
becomes:
```cpp
CV_EXPORTS void cvWriteMat_Base64(::CvFileStorage* fs, const char* name,
const ::CvMat* mat);
CV_EXPORTS void
cvWriteMatND_Base64(::CvFileStorage* fs, const char* name, const
::CvMatND* mat);
```
@vpisarev
Copy link
Contributor

@wiryls, probably, we should have 3 functions then: cvStartWriteRawData_Base64(), cvWriteRawData_Base64() and cvEndWriteRawData_Base64()

@wiryls
Copy link
Contributor Author

wiryls commented Jun 24, 2016

@vpisarev Ok, and can I add a member variable to CvFileStorage?

Because I need something to hold the length of binary data and data whose length is not a multiple of 3 (the part that temporarily should not be encoded). Or is there any better way to achieve it?

@vpisarev
Copy link
Contributor

@wiryls, yes, sure, add all the necessary data to CvFileStorage

The three new functions:

```cpp
void cvStartWriteRawData_Base64(::CvFileStorage * fs, const char* name,
int len, const char* dt);
void cvWriteRawData_Base64(::CvFileStorage *
fs, const void* _data, int len);
void
cvEndWriteRawData_Base64(::CvFileStorage * fs);
```

Test is also updated. (And it's remarkable that there is a bug in
`cvWriteReadData`.)
I could not find the cause of the error:

```
C:\builds_ocv\precommit_opencl\opencv\modules\ts\src\ts_perf.cpp(361):
error: The difference between expect_max and actual_max is
8445966.0000002384, which exceeds eps, where

expect_max evaluates to 0.9999997615814209,

actual_max evaluates to 8445967, and

eps evaluates to 1.0000000000000001e-005.

Argument "dst0" has unexpected maximal value
```

Hope this is a false alarm.
@wiryls
Copy link
Contributor Author

wiryls commented Jun 24, 2016

@vpisarev . I've done that.

cvWriteRawData_Base64 now become three functions:

void cvStartWriteRawData_Base64(::CvFileStorage * fs, const char* name, int len, const char* dt);
void cvWriteRawData_Base64(::CvFileStorage * fs, const void* _data, int len);
void cvEndWriteRawData_Base64(::CvFileStorage * fs);

By the way, I've also encountered a strange error when building on OpenCL (maybe it's a bug). See commit 677d4d20ce9de6b548887a05bbdd0ffb207281b0. And build success after I retry.

@vpisarev
Copy link
Contributor

👍

@opencv-pushbot opencv-pushbot merged commit df5a7c8 into opencv:master Jun 30, 2016
@wiryls wiryls deleted the FileStorageBase64 branch July 3, 2016 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants