pytorch / vision

Datasets, Transforms and Models specific to Computer Vision
https://pytorch.org/vision
BSD 3-Clause "New" or "Revised" License
16.16k stars 6.95k forks source link

Obscure error messages using VideoReader when PyAV version too old/not installed #8510

Open occipita opened 3 months ago

occipita commented 3 months ago

🐛 Describe the bug

When a sufficiently recent version of PyAV is not installed, the script vision/torchvision/io/video_reader.py initialises the variable av to an ImportError object that contains a description of the issue, either at line 38:

av = ImportError(
        """\
PyAV is not installed, and is necessary for the video operations in torchvision.
See https://github.com/mikeboers/PyAV#installation for instructions on how to
install PyAV on your system.
"""
    )

or on line 28 (code omitted for brevity, but is similar to the above). This is potentially very useful information that would make it easy to see why an application isn't working. Unfortunately, this error is never actually raised.

Instead, when a VideoReader object is created, the av variable is simply assumed to contain the PyAV module object. This is first used on line 159:

            self.container = av.open(src, metadata_errors="ignore")

As an ImportError object does not have a method called open, this results in a rather mystifying error condition being raised: AttributeError: 'ImportError' object has no attribute 'open'.

I suspect there should be a test immediately prior to line 159 which checks if av is an ImportError object and raises it if it is.

Versions

This bug is not related to specific versions, but can be seen by examination of the current version of the source code.

NicolasHug commented 3 months ago

Thanks for the report @occipita . And sorry for the weird user experience. AFAICT this was introduced in https://github.com/pytorch/vision/pull/6943. I have no idea why it was done this way, this part of the code-base is full of mysteries.

I can see how this can lead to a strange UX. We should try to make this better but unfortunately if we were to raise a loud error at import time this might lead to other issues: e.g. all users might potentially get an error even if they don't care about video decoding. This is sort of related to https://github.com/pytorch/vision/issues/8192.