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

Added ImageJPEG #30

Merged
merged 1 commit into from
Feb 22, 2016
Merged

Added ImageJPEG #30

merged 1 commit into from
Feb 22, 2016

Conversation

favreau
Copy link
Contributor

@favreau favreau commented Feb 11, 2016

No description provided.

for( size_t i = 0; i < NB_BYTES; ++i )
v.push_back(i);
imageJPEG.setData( v );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

No BOOST_CHECK in test!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops... fixed

@eile
Copy link
Contributor

eile commented Feb 12, 2016

+1 otherwise

const std::string expectedJSONWithData = "{\n"
" \"data\" : [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,"
" 10, 11, 12, 13, 14, 15 ]\n"
"}\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

the .fbs should use byte for data, which will cause the data to be emitted base64-encoded, not as array-of-uint8_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I understand, you want to rename 'data' into 'byte'? It's only an attribute name, how can this impact the generated code?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, the type of data should be byte.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, still don't get it, I changed data from ubyte to byte, but what I get is an array-of-int8_t instead of array-of-uint8_t. I do not see how base64 can be emitted there :-(

Copy link
Contributor

Choose a reason for hiding this comment

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

It works in the test: https://github.com/HBPVIS/ZeroBuf/blob/master/tests/jsonSerialization.cpp#L120

Not sure what's going wrong for you, but maybe you can find it following the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, now I understand, the code for the base64 generation is not in my branch, that's why it did not make any sense to me. Let me see what I can do.

@favreau favreau force-pushed the master branch 2 times, most recently from b02f164 to fa2a641 Compare February 22, 2016 09:19
favreau added a commit that referenced this pull request Feb 22, 2016
@favreau favreau merged commit 4a80db5 into HBPVIS:master Feb 22, 2016
@eile
Copy link
Contributor

eile commented Feb 22, 2016

Jenkins was red - don't merge then. Please fix asap, I now have activated the branch protection on this repo.

@favreau
Copy link
Contributor Author

favreau commented Feb 22, 2016

Yep, just realised. Working on it.

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.

2 participants