Add support for memory video file in FramesDecoder #4184
Conversation
Signed-off-by: Albert Wolant <awolant@nvidia.com>
Signed-off-by: Albert Wolant <awolant@nvidia.com>
|
!build |
|
CI MESSAGE: [5707917]: BUILD STARTED |
|
CI MESSAGE: [5707917]: BUILD FAILED |
|
CI MESSAGE: [5707917]: BUILD PASSED |
Signed-off-by: Albert Wolant <awolant@nvidia.com>
| void FramesDecoderGpu::InitGpuDecoder() { | ||
| nvdecode_state_ = std::make_unique<NvDecodeState>(); | ||
|
|
||
| InitBitStreamFilter(); | ||
|
|
||
| filtered_packet_ = av_packet_alloc(); | ||
| DALI_ENFORCE(filtered_packet_, "Could not allocate av packet"); | ||
|
|
||
| auto codec_type = GetCodecType(); | ||
|
|
||
| // Create nv decoder | ||
| CUVIDDECODECREATEINFO decoder_info; | ||
| memset(&decoder_info, 0, sizeof(CUVIDDECODECREATEINFO)); | ||
|
|
||
| decoder_info.bitDepthMinus8 = 0; | ||
| decoder_info.ChromaFormat = cudaVideoChromaFormat_420; | ||
| decoder_info.CodecType = codec_type; | ||
| decoder_info.ulHeight = Height(); | ||
| decoder_info.ulWidth = Width(); | ||
| decoder_info.ulMaxHeight = Height(); | ||
| decoder_info.ulMaxWidth = Width(); | ||
| decoder_info.ulTargetHeight = Height(); | ||
| decoder_info.ulTargetWidth = Width(); | ||
| decoder_info.ulNumDecodeSurfaces = num_decode_surfaces_; | ||
| decoder_info.ulNumOutputSurfaces = 2; | ||
|
|
||
| CUDA_CALL(cuvidCreateDecoder(&nvdecode_state_->decoder, &decoder_info)); | ||
|
|
||
| // Create nv parser | ||
| CUVIDPARSERPARAMS parser_info; | ||
| memset(&parser_info, 0, sizeof(CUVIDPARSERPARAMS)); | ||
| parser_info.CodecType = codec_type; | ||
| parser_info.ulMaxNumDecodeSurfaces = num_decode_surfaces_; | ||
| parser_info.ulMaxDisplayDelay = 0; | ||
| parser_info.pUserData = this; | ||
| parser_info.pfnSequenceCallback = detail::process_video_sequence; | ||
| parser_info.pfnDecodePicture = detail::process_picture_decode; | ||
| parser_info.pfnDisplayPicture = nullptr; | ||
|
|
||
| CUDA_CALL(cuvidCreateVideoParser(&nvdecode_state_->parser, &parser_info)); | ||
|
|
||
| // Init internal frame buffer | ||
| // TODO(awolant): Check, if continuous buffer would be faster | ||
| for (size_t i = 0; i < frame_buffer_.size(); ++i) { | ||
| frame_buffer_[i].frame_.resize(FrameSize()); | ||
| frame_buffer_[i].pts_ = -1; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I added new constructor in this PR. This is a part that is common for both constructors, so I extracted it to a separate method.
|
!build |
|
CI MESSAGE: [5717738]: BUILD STARTED |
|
CI MESSAGE: [5717738]: BUILD PASSED |
| bool is_vfr_ = false; | ||
|
|
||
| std::string filename_ = ""; |
There was a problem hiding this comment.
How about making this string also optional? The rationale would be that when we have a video from memory it would not have a filename. Unless of course the existing implementation assumes, that the filename need to always be there, in that case I wouldn't bother with such "enhancement".
There was a problem hiding this comment.
Done. After the constructor we use it only to report errors to let user know which file caused the error. I refactored this code a bit, so now we have a function that returns name of the file or "memory file" instead.
| av_state_->ctx_ = avformat_alloc_context(); | ||
| DALI_ENFORCE(av_state_->ctx_, "Could not alloc avformat context"); | ||
|
|
||
| const int av_io_buffer_size = 32768; |
There was a problem hiding this comment.
Where does this number come from?
And maybe write it as (1 << 15) so it's less magical?
There was a problem hiding this comment.
This was the value that FFMPEG sets as a default when you do not try to change that. I extracted it as a const field in the class, changed the value and added some comment.
| return to_read; | ||
| } | ||
|
|
||
| int64_t MemoryVideoFile::Seek(int64_t new_position, int origin) { |
There was a problem hiding this comment.
Why is this arg named origin? As I understand it's a seeking method.
There was a problem hiding this comment.
Changed it to mode. Yes, this is the seeking method.
|
|
||
| int Read(unsigned char *buffer, int buffer_size); | ||
|
|
||
| int64_t Seek(int64_t new_position, int origin); |
There was a problem hiding this comment.
Any short docu explaining what is the second argument?
| * @param memory_file Pointer to memory with video file data. | ||
| * @param memory_file_size Size of memory_file. | ||
| */ | ||
| explicit FramesDecoderGpu(char *memory_file, int memory_file_size, cudaStream_t stream = 0); |
There was a problem hiding this comment.
How about using span instead of passing the pointer and size separately? (up to you)
There was a problem hiding this comment.
We actually do have our own implementation of span, so you could use that one. But it's up to you.
There was a problem hiding this comment.
This code is not dependent on DALI other then some error checking and reporting and I think it would be good to keep it this way. It might be useful to others, I think.
Signed-off-by: Albert Wolant <awolant@nvidia.com>
|
!build |
|
CI MESSAGE: [5722994]: BUILD STARTED |
|
CI MESSAGE: [5722994]: BUILD FAILED |
Signed-off-by: Albert Wolant <awolant@nvidia.com>
|
!build |
|
CI MESSAGE: [5729182]: BUILD STARTED |
|
CI MESSAGE: [5729182]: BUILD FAILED |
Signed-off-by: Albert Wolant <awolant@nvidia.com>
|
!build |
|
CI MESSAGE: [5729429]: BUILD STARTED |
|
CI MESSAGE: [5729429]: BUILD FAILED |
|
CI MESSAGE: [5729429]: BUILD PASSED |
Signed-off-by: Albert Wolant awolant@nvidia.com
Category:
New feature
Description:
Add support for video files kept in memory for
FramesDecoderandFramesDecoderGpu.Additional information:
Affected modules and functionalities:
FramesDecoder,FramesDecoderGpuand video testsKey points relevant for the review:
Is this functionality enough for video inference plan?
Tests:
Checklist
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-2957