-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove AVFrameStream
struct and remove raw AVFrame*
pointers
#572
Conversation
AVFrameStream
structAVFrameStream
struct and remove raw AVFrame*
pointers
@@ -1302,7 +1301,7 @@ void VideoDecoder::convertAVFrameToFrameOutputOnCPU( | |||
} | |||
|
|||
int VideoDecoder::convertAVFrameToTensorUsingSwsScale( | |||
const AVFrame* avFrame, | |||
UniqueAVFrame& avFrame, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I'm misunderstanding, pretending the AVFrame*
was const
was a big lie, because of the call to sws_scale(..., avFrame->data, ...)
below. So I made the ref non-const.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it still makes sense for it to be const. We have a const reference to a unique pointer, which means that we can't change the contents of the unique pointer. That is, we can't call release()
or reset()
on the unique pointer. But we can make changes to the object the unique pointer points to.
This is similar to the difference between a "const pointer" and "pointer to const." A const pointer is a pointer that cannot change what it is pointing to. A pointer to const is a pointer to an object that cannot change - but you can change what the pointer points to.
Basically, anytime we don't want ownership of the unique pointer to change, we should use const.
|
||
int convertAVFrameToTensorUsingSwsScale( | ||
const AVFrame* avFrame, | ||
UniqueAVFrame& avFrame, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth reasoning through if all of these should be const references - see my above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll open an issue to keep track of this
Continuing the work towards making the decoder properly single-stream. This PR addresses #483 (comment).
The
AVFrameStream
struct is obsolete, we don't need to keep track of the stream - we know it'sactiveStreamIndex_
.Additionally, since this is fairly related, this PR removes all (but one) uses of
AVFrame*
and replaces those with[const] UniqueAVFrame&
.