Improve the video decoder errors #5491
Conversation
- adds the source info to the errors in the video decoder operator - makes sure the video decoder operator raises the right error instead of just crashing the process Signed-off-by: Janusz Lisiecki <[email protected]>
JanuszL
force-pushed
the
improve_video_decoder_error
branch
from
c51083b to
ba0e516
Compare
|
!build |
|
CI MESSAGE: [15399757]: BUILD STARTED |
|
CI MESSAGE: [15399757]: BUILD FAILED |
|
CI MESSAGE: [15399757]: BUILD PASSED |
| Filename(), | ||
| "due to", | ||
| "\", due to", |
There was a problem hiding this comment.
| "\", due to", | |
| "\", due to ", |
| : av_state_(std::make_unique<AvState>()), | ||
| filename_(source_info.size() ? std::optional<const std::string>(source_info) : std::nullopt), |
There was a problem hiding this comment.
This whole std::optional<const std::string> affair seems unnecessarily complex. I don't think it's possible to have a meaningful empty filename - so we could just use empty string to denote the "memory file" (see frames_decoder.h:202, the only usage of filename_.value). This would simplify the code here, too.
| @@ -206,8 +206,9 @@ FramesDecoder::FramesDecoder(const std::string &filename) | |||
|
|
|||
|
|
|||
| FramesDecoder::FramesDecoder(const char *memory_file, int memory_file_size, bool build_index, | |||
| bool init_codecs, int num_frames) | |||
| bool init_codecs, int num_frames, const std::string &source_info) | |||
There was a problem hiding this comment.
You can use std::string_view, which will not create a copy of the argument nor will it construct a temporary string.
Alternatively, since we use it to construct a string member, we could pass source_info by value and move it to filename_. This would save one copy if the constructor call does not use a reference to an existing string object.
There was a problem hiding this comment.
we could pass source_info
Done
move it to filename_
Do we have to move explicitly or the compiler will silently do so?
There was a problem hiding this comment.
No, compiler won't move out a parameter in any scenario other than when directly returning it, e.g.:
string foo(string a, string b) {
if (a.find("nemo") != string::npos)
return a; // copy elision
else
return b; // copy elision
}
| @@ -117,7 +117,7 @@ class DLL_PUBLIC FramesDecoder { | |||
| * `memory_file_size` arguments cover the entire video file, including the header. | |||
| */ | |||
| FramesDecoder(const char *memory_file, int memory_file_size, bool build_index = true, | |||
| bool init_codecs = true, int num_frames = -1); | |||
| bool init_codecs = true, int num_frames = -1, const std::string &source_info = {}); | |||
There was a problem hiding this comment.
| bool init_codecs = true, int num_frames = -1, const std::string &source_info = {}); | |
| bool init_codecs = true, int num_frames = -1, std::string_view source_info = {}); |
There was a problem hiding this comment.
Pass it by value as suggested earlier.
| frame_buffer_(num_decode_surfaces_), | ||
| stream_(stream) { | ||
| int num_frames, | ||
| const std::string &source_info): |
There was a problem hiding this comment.
| const std::string &source_info): | |
| std::string_view source_info): |
There was a problem hiding this comment.
Pass it by value as suggested earlier.
|
!build |
|
CI MESSAGE: [15513826]: BUILD STARTED |
|
CI MESSAGE: [15513826]: BUILD FAILED |
Signed-off-by: Janusz Lisiecki <[email protected]>
JanuszL
force-pushed
the
improve_video_decoder_error
branch
from
3890aec to
dd95dc0
Compare
|
!build |
|
CI MESSAGE: [15514517]: BUILD STARTED |
|
CI MESSAGE: [15514517]: BUILD PASSED |
| bool init_codecs, int num_frames, const std::string source_info) | ||
| : av_state_(std::make_unique<AvState>()), | ||
| filename_(source_info), |
There was a problem hiding this comment.
| bool init_codecs, int num_frames, const std::string source_info) | |
| : av_state_(std::make_unique<AvState>()), | |
| filename_(source_info), | |
| bool init_codecs, int num_frames, std::string source_info) | |
| : av_state_(std::make_unique<AvState>()), | |
| filename_(std::move(source_info)), |
...or, as suggested earlier, use std::string_view.
Signed-off-by: Janusz Lisiecki <[email protected]>
|
!build |
|
CI MESSAGE: [15521941]: BUILD STARTED |
|
CI MESSAGE: [15521941]: BUILD PASSED |
of just crashing the process
Category:
Bug fix (non-breaking change which fixes an issue)
Description:
of just crashing the process
Additional information:
Affected modules and functionalities:
-frames decoder
Key points relevant for the review:
-NA
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: N/A