Hi @binliunls ,

I think this great PR almost achieved all the necessary features of the video loading part.
Please take a look and maybe test it locally in your use case.

Thanks in advance.

Hi @rijobro,
I had a few tests on this videodataset this week. However my program stuck when I try to use dataloader whose num_workers equals to 1 or above to load data with the videodataset. I am currently finding the reason why this happened. And I think maybe this would be helpful for you too.

Hi @binliunls ,

Do you mean DataLoader or ThreadDataLoader with use_thread_workers=True?
I think this line in this PR is not thread safe:

self.cap.set(cv2.CAP_PROP_POS_FRAMES, index)

And GPU transforms can't work in multi-processing in your use case.

So if num_workers > 0, both multi-thread and multi-processing workers should not work in theory here.
@rijobro right?

Thanks.

Hi @binliunls ,

Do you mean DataLoader or ThreadDataLoader with use_thread_workers=True? I think this line in this PR is not thread safe:

self.cap.set(cv2.CAP_PROP_POS_FRAMES, index)

And GPU transforms can't work in multi-processing in your use case.

So if num_workers > 0, both multi-thread and multi-processing workers should not work in theory here. @rijobro right?

Thanks.

Hi @Nic-Ma :
I tried DataLoader and ThreadDataLoader. They both stuck when set the number workers >= 1. @rijobro Could you please double check this problem, in case I did something wrong with the environment or the code.

Hi @binliunls ,

Are you execute transforms on GPU? It should not work with num_workers > 0 in DataLoader.

Thanks.

Hi @binliunls ,

Are you execute transforms on GPU? It should not work with num_workers > 0 in DataLoader.

Thanks.

I tried to move the transform from GPU to CPU and the program still stuck.

Hi @binliunls ,

I think your multi-processing issue may be similar to:
https://www.pythonfixing.com/2022/05/fixed-python-opencv-multiprocessing.html
Let's keep using num_workers=0 with ThreadDataLoader and GPU transforms so far.
@rijobro Could you please help confirm the issue? If True, maybe we need to highlight it in the doc-string of VideoDataset?

Thanks in advance.

Hi @binliunls ,

I think your multi-processing issue may be similar to: https://www.pythonfixing.com/2022/05/fixed-python-opencv-multiprocessing.html Let's keep using num_workers=0 with ThreadDataLoader and GPU transforms so far. @rijobro Could you please help confirm the issue? If True, maybe we need to highlight it in the doc-string of VideoDataset?

Thanks in advance.

Hi @Nic-Ma:
The link below solved my problem. It may cause problems when the main process and children process share the same obeject, which is the video "cap" object in our case. I moved the "open_video" function call from the init function to functions like "get_frame", where the "cap" is needed . The new program can run.
https://stackoverflow.com/questions/62126858/python-multiprocessing-and-opencv-videocapture-read-error

Hi @binliunls ,

If you move the "open_video" call to every get frame, will it be slower as it frequently opens and closes?

Thanks.

Hi @binliunls ,

If you move the "open_video" call to every get frame, will it be slower as it frequently opens and closes?

Thanks.
Hi @Nic-Ma
Yes it will be 1.5x slower than @rijobro one if num_worker=0&batch_size=4 for both videodatasets. However @rijobro old videodataset will be 1.5x slower if I set num_worker=4 and keep the batch_size=4 for the new one.
Thanks.

Ok, so I will put a multiprocessing argument into the constructor, which is False by default. This will do open_video at construction and only perform once. This will not be threadsafe and the user should use num_workers==0. If True, we do open_video each time a new frame is requested. This will be threadsafe, useful if num_workers>0 but slower if num_workers==0. Does that sound reasonable?

Hi @rijobro ,

I think it's OK to add this multiprocessing, the solution looks good to me.
But I think it's "process safe" not "thread safe", the VideoDataset still can't work with ThreadDataLoader if set use_thread_workers=True. It's not a problem so far.

Thanks.

@binliunls I've made the various changes (including the RGB option you requested here). Could you give it a go and let me know if it all works as expected?

@Nic-Ma @wyli I'm seeing circular import errors. Is this specific to this PR or are we seeing this elsewhere? I can't see changes in this code affecting the Convolution class it's mentioning.

I think it's in this PR only, and I couldn't find out the reason...most likely from the unit tests?

@binliunls I've made the various changes (including the RGB option you requested here). Could you give it a go and let me know if it all works as expected?

Hi @rijobro ,
I will run some tests on this new dataset. Meanwhile I think maybe the channel to front operation in the link below is not necessary, since users can do this by adding "AsChannelFirst" to their transforms. And this will be a block to users who just want to see what the frames look like, because they are going to get a channel first image and have to move the axis again.
https://github.com/rijobro/MONAI/blob/2d52bfe27e7dd2f238103cc32f8f1837c1fa8214/monai/data/video_dataset.py#L108

Hm, I think for pytorch based applications, we always want the channel in front of spatial dimensions. Granted, matplotlib and opencv expect them at the end. I'll put this as an option, too. It's true that we could leave it for the transforms, but I think that it's going to be needed frequently enough to have it done by default.

Hi @rijobro ,

Would you like to fix the CI issue and let's merge this PR then?

Thanks in advance.

Will wait to see if this passes CI, added .avi example video in case .mp4 codec missing. If both missing, skip. Also silence stderr error from OpenCV C++.

/build

@rijobro ,

I checked the blossom error, seems some code is not correct:

2022-08-12T14:30:57.0337947Z [2022-08-12T14:29:35.551Z] ======================================================================
2022-08-12T14:30:57.0338225Z [2022-08-12T14:29:35.551Z] ERROR: test_dataset (tests.test_video_datasets.TestVideoFileDataset)
2022-08-12T14:30:57.0338497Z [2022-08-12T14:29:35.551Z] ----------------------------------------------------------------------
2022-08-12T14:30:57.0338703Z [2022-08-12T14:29:35.551Z] Traceback (most recent call last):
2022-08-12T14:30:57.0339057Z [2022-08-12T14:29:35.551Z]   File "/home/jenkins/agent/workspace/MONAI-premerge/monai/tests/test_video_datasets.py", line 131, in test_dataset
2022-08-12T14:30:57.0339340Z [2022-08-12T14:29:35.551Z]     super().test_dataset(self.known_num_frames, self.known_fps)
2022-08-12T14:30:57.0339648Z [2022-08-12T14:29:35.551Z] AttributeError: 'TestVideoFileDataset' object has no attribute 'known_num_frames'

Thanks.

Will wait to see if this passes CI, added .avi example video in case .mp4 codec missing. If both missing, skip. Also silence stderr error from OpenCV C++.

Hi @rijobro ,
I tried your test case. And when both "mp4" and "avi" codec don't exist. The "test_dataset" in class TestVideoDataset will still go. And this may lead to a problem since the "know_fps" and "know_num_frames" are both uninitialized since no condec is avaliable.

there is a version of opencv in our docker image release, it's probably from the base image and compiled without ffmpeg. In order to use the video dataset, what would be the installation steps from a docker image user's perspective?

HI @wyli ,
I have been there in monai docker. 1, Remove the dir /opt/conda/lib/python3.8/site-packages/cv2 manually by "rm -r .../cv2". 2,"conda install opencv" to reinstall opencv. 3, run "apt-get install ffmpeg libsm6 libxext6 -y" to install some extra libs.

there is a version of opencv in our docker image release, it's probably from the base image and compiled without ffmpeg. In order to use the video dataset, what would be the installation steps from a docker image user's perspective?

HI @wyli , I have been there in monai docker. 1, Remove the dir /opt/conda/lib/python3.8/site-packages/cv2 manually by "rm -r .../cv2". 2,"conda install opencv" to reinstall opencv. 3, run "apt-get install ffmpeg libsm6 libxext6 -y" to install some extra libs.

thanks, I'd suggest let's raise this issue on monday's meeting to discuss the user experience.

I have updated to set the known_num_frames etc to None when no codecs are found. This should raise the SkipTest as self.video_source is None in this case.

/build

[2022-08-15T14:12:14.896Z] coverage
[2022-08-15T14:12:19.081Z] No source for code: '/home/jenkins/agent/workspace/MONAI-premerge/monai/config-3.8.py'.
[2022-08-15T14:12:19.082Z] Aborting report output, consider using -i.
script returned exit code 1

looks like opencv is creating new config-3.8.py which blocks the tests

Hi @rijobro @wyli ,

If we skip the tests, CI of codecov will not pass: codecov/project — 49.76% (target 70.00%)
Any idea?

Thanks.

[2022-08-15T14:12:14.896Z] coverage
[2022-08-15T14:12:19.081Z] No source for code: '/home/jenkins/agent/workspace/MONAI-premerge/monai/config-3.8.py'.
[2022-08-15T14:12:19.082Z] Aborting report output, consider using -i.
script returned exit code 1

looks like opencv is creating new config-3.8.py which blocks the tests

This issue is blocking the codecov https://github.com/Project-MONAI/MONAI/runs/7847212644?check_suite_focus=true it doesn't happen on other PR's tests

/build

Unrelated error in the CI: https://github.com/Project-MONAI/MONAI/runs/7853900108?check_suite_focus=true, will re-run jobs.

/build