Pyav backend for VideoReader API #6598
Conversation
jdsgomes
left a comment
jdsgomes
left a comment
There was a problem hiding this comment.
Thanks for working on it. In general looks good, so I just left a few comments
| ], | ||
| ) | ||
| def test_metadata(self, video_file): | ||
| torchvision.set_video_backend("cuda") |
There was a problem hiding this comment.
can we run this just once for all tests in this file since they all supposed to run with the cud backend?
There was a problem hiding this comment.
Good question. Let me try out :)
There was a problem hiding this comment.
For some reason that failed (putting it in a class header) so I'll keep it as is, and then re-do the tests later.
| device = torch.device("cuda") | ||
| self._c = torch.classes.torchvision.GPUDecoder(path, device) | ||
| return | ||
| if not _has_video_opt(): |
There was a problem hiding this comment.
we still need this inside the elif self.backend == "video_reader" bellow right?
| temp_str = self.container.streams.get(**self.pyav_stream)[0] | ||
| offset = int(round(time_s / temp_str.time_base)) | ||
| if not keyframes_only: | ||
| warnings.warn("Accurate seek is not implemented for pyav backend") |
There was a problem hiding this comment.
Not related to this PR, but do you have plans to add accurate seek in PyAV?
There was a problem hiding this comment.
Yeah, but in a roundabout way.
(I need to do it manually)
|
FYI @YosuaMichael will be cutting the release branch on Monday. Is your intention to try to merge prior? |
|
@datumbox let's not do it, it is missing accurate seek so it would make more sense to do it for the next one (and hopefully sort some other woes along the way). |
|
@bjuncek Agreed. Makes sense to me to delay to avoid release issues. |
bjuncek
changed the title
|
Hi @jdsgomes . If this looks ok, let's try to merge it. |
jdsgomes
left a comment
jdsgomes
left a comment
There was a problem hiding this comment.
LGTM I agree that precise seek can be added in a follow up PR
|
Checked the latest changes looks good to me. |
|
Hey @jdsgomes! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
This reverts commit 2e83352.
This reverts commit 2e83352.
5 participants
Draft implementation for the VideoReader API.
This would allow the usage of the new API without the necessity to have own FFMPEG installation, but instead would only rely on having PyAV installed.