Skip to content

Conversation

scotts
Copy link
Contributor

@scotts scotts commented Dec 16, 2024

In the C++ layer, in VideoDeocder::initializeDecoder(), we were calling std::vector::resize(), adding 1 each time, and then calling std::vector::back() to get a reference to the last element and fill it out. That is not how vectors are designed to be used. Instead, in the loop, we just create a struct on the stack, fill it with values, and call std::vector::push_back().

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Dec 16, 2024
@scotts scotts marked this pull request as ready for review December 16, 2024 16:48
@scotts scotts merged commit 9653969 into meta-pytorch:main Dec 16, 2024
36 of 37 checks passed
@scotts scotts deleted the refactor_init branch December 16, 2024 16:48
Comment on lines -237 to -240
// Some formats don't store enough info in the header so we read/decode a few
// frames to grab that. This is needed for the filter graph. Note: If this
// takes a long time, consider initializing the filter graph after the first
// frame decode.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if we should keep that comment, or what was the reason to remove it? It might be helpful to keep (especially the "Note" part), particularly so that the reader knows that some frame decoding does happen within avformat_find_stream_info()?

I'm finding this from the FFmpeg docs

Some formats do not have a header or do not store enough information there, so it is recommended that you call the avformat_find_stream_info() function which tries to read and decode a few frames to find missing information.

Copy link
Contributor Author

@scotts scotts Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I removed the comment because of the latter part about lazy initialization - we're doing that now. But the other stuff is both relevant and informative. I'll submit a PR which revives the relevant stuff.

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