Basic Video Support #74
Conversation
luisremis
force-pushed
the
lr/feature_video
branch
from
dafbdc6 to
9fdc0dc
Compare
luisremis
force-pushed
the
lr/feature_video
branch
from
9fdc0dc to
566df36
Compare
| @@ -103,21 +103,21 @@ int AddImage::construct_protobuf(PMGDQuery& query, | |||
| } | |||
|
|
|||
| std::string img_root = _storage_tdb; | |||
| VCL::ImageFormat vcl_format = VCL::TDB; | |||
| VCL::Image::Format vcl_format = VCL::Image::Format::TDB; | |||
There was a problem hiding this comment.
Seems like this file could totally use a
using namespace VCL; IT appears often enough to avoid doing VCL:: everywhere
There was a problem hiding this comment.
I agree. For now we just update the changes in the enums. We can change namespacing in a future refactor, I already have some other things in mind that we can do better.
src/QueryHandler.cc
Outdated
| @@ -243,7 +253,15 @@ void QueryHandler::cleanup_query(const std::vector<std::string>& images) | |||
| { | |||
| for (auto& img_path : images) { | |||
| VCL::Image img(img_path); | |||
| img.delete_image(); | |||
| img.delete_image(); // TODO this should be common | |||
There was a problem hiding this comment.
Common to what? And if it should be, maybe just do it?
src/VideoCommand.cc
Outdated
| results["list"].append(VDMS_VID_PATH_PROP); | ||
| } | ||
|
|
||
| std::cout << "hello" << std::endl; |
There was a problem hiding this comment.
gosh, not sure how it got there. It will make people think I don't use gdb (which I don't).
| ifile.seekg(0, std::ios::end); | ||
| size_t encoded_size = (long)ifile.tellg(); | ||
| ifile.seekg(0, std::ios::beg); | ||
|
|
There was a problem hiding this comment.
Isn't this equivalent to asking VCL to read video? Seems like code repeat somehow
There was a problem hiding this comment.
It is not the same. VCL will run decoding when reading, and in this case we don't need that, we just want the blob to be sent without any decoding. The same is true for images, an inefficiency we , but it is least critical(I think there is an issue already, if not, it's been in my mind for some time already). Decoding video is much more compute intensive.
There was a problem hiding this comment.
In that case I would have added a function to VCL to allow the same thing so all data manipulation stayed in one library as much as possible but too late I support. FIle an issue?
| @@ -452,8 +521,8 @@ | |||
| "properties": { | |||
| "_ref": { "$ref": "#/definitions/refInt" }, | |||
| "link": { "$ref": "#/definitions/blockLink" }, | |||
| "operations": { "$ref": "#/definitions/blockOperations" }, | |||
| "format": { "$ref": "#/definitions/formatString" }, | |||
| "operations": { "$ref": "#/definitions/blockImageOperations" }, | |||
There was a problem hiding this comment.
Aren't these changes part of the previous commit? It probably breaks some test in the previous commit.
There was a problem hiding this comment.
nope, this is a result of adding video that we have to split operations block in two.
| props = {} | ||
| props["name"] = "simg_update_0" | ||
|
|
||
| img_params = {} |
There was a problem hiding this comment.
Do you need me to review tests? I am hoping you have put checks and balances here to make sure videos get stored and rertieved correctly without losing out data :)
luisremis
force-pushed
the
lr/feature_video
branch
from
566df36 to
2d2432d
Compare
Add basic support for video. Updates Image formats definitions.