Skip to content

Testing pyav#2745

Closed
fmassa wants to merge 25 commits intopytorch:masterfrom
fmassa:test-av
Closed

Testing pyav#2745
fmassa wants to merge 25 commits intopytorch:masterfrom
fmassa:test-av

Conversation

@fmassa
Copy link
Copy Markdown
Member

@fmassa fmassa commented Oct 2, 2020

Let's see when it breaks

@andfoy
Copy link
Copy Markdown
Contributor

andfoy commented Oct 5, 2020

It seems that other test is causing video ones to fail:
imagen

Also, order don't seem to affect:
imagen

@andfoy
Copy link
Copy Markdown
Contributor

andfoy commented Oct 5, 2020

test_io_opt seems to be the culprit for the failure of the tests, I don't know what is the intent of that test

imagen

Edit: The error occurs when set_video_backend('video_reader') is called. If set_video_backend('pyav') is selected, then all tests pass

@andfoy
Copy link
Copy Markdown
Contributor

andfoy commented Oct 5, 2020

The only effect that set_video_backend('video_reader') has over test_io.py is related to the codecs used to encode/decode the test videos:

if video_codec is None:

That means that under the current circumstances test_io is only testing the video_reader backend, rather than the PyAV one.

@fmassa
Copy link
Copy Markdown
Member Author

fmassa commented Oct 6, 2020

@andfoy this is great debugging, thanks!

So, test_io_opt is there to test the video_reader backend of read_video, without having to resort to copy-pasting all the tests. If it is negatively interfering with test_io, we should do something to change that.

It is great that we now know the reason why this wasn't working before, now we need to understand why the partial reading of files is not working with the video_reader backend.

@bjuncek you have used the video_reader backend for partial reads recently, right? Can you check again if those tests are passing or not in your system?

@bjuncek
Copy link
Copy Markdown
Contributor

bjuncek commented Oct 6, 2020

@fmassa I pulled your branch and run the tests in my PR. They are passing locally for me but that has always been the case so lets see what CI says

@andfoy
Copy link
Copy Markdown
Contributor

andfoy commented Oct 6, 2020

I think we can close this one

@andfoy andfoy closed this Oct 6, 2020
@fmassa fmassa deleted the test-av branch October 6, 2020 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants