Skip to content

Conversation

@NicolasHug
Copy link
Contributor

We haven't had this problem yet for videos, but as I'm developing audio support I started hitting some "bad optional access" errors. In my case, it was due to this bad access:

https://github.com/pytorch/torchcodec/blob/b7271fa66c0c1127144f2e4149c974a30ca75a3a/src/torchcodec/decoders/_core/VideoDecoder.cpp#L1594-L1595

We set the value here, when the decoder is created:

https://github.com/pytorch/torchcodec/blob/b7271fa66c0c1127144f2e4149c974a30ca75a3a/src/torchcodec/decoders/_core/VideoDecoder.cpp#L155-L158

Turns out that this value doesn't exist for our audio mp3 test file.

This PR changes the un-informative error message into something more informative. Note that we have to do the checks / enforcements at the very last moment instead of when the decoder is initiated (and when those fields are set), because that would prevent us from returning metadata in approximate mode.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 6, 2025
@scotts
Copy link
Contributor

scotts commented Mar 6, 2025

Note that we have to do the checks / enforcements at the very last moment instead of when the decoder is initiated (and when those fields are set), because that would prevent us from returning metadata in approximate mode.

Hmmm. That seems really unfortunate that we can't error as early as possible (construction time) in order to support a specific use-case. I originally called it "uncommon," but then I realized that I personally do it all the time. :) Let's merge this, and if users have difficulty that we're only erroring at decode time, we can revisit.

@NicolasHug NicolasHug merged commit e52d864 into meta-pytorch:main Mar 6, 2025
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants