Refactor context structs to use constructors - #869
Conversation
meta-cla
Bot
added
the
CLA Signed
| AVRational timeBase = {0, 0}; | ||
| UniqueAVBufferRef hwFramesCtx; | ||
|
|
||
| FiltersContext() = default; |
There was a problem hiding this comment.
Should default initializers be removed from the above then?
There was a problem hiding this comment.
Unfortunately, we can't do that as they are default-initialized as members of CpuDeviceInterface: https://github.com/pytorch/torchcodec/blob/da3eddaa6746d093b09ad1413a42d7244686c58e/src/torchcodec/_core/CpuDeviceInterface.h#L64-L67
Before we decode anything, they start out empty. We could instead make the members unique pointers, but I think that's more complexity than it's worth.
There was a problem hiding this comment.
Yeah, you are right. But in this case, I just realized that I forgot to add default initializers for SwsFrameContext struct. Can you help to fix within this PR?
There was a problem hiding this comment.
There was a problem hiding this comment.
You might wish to adjust a format as AV_PIX_FMT_NONE defined as -1 (https://github.com/FFmpeg/FFmpeg/blob/6891314db18c96afa35f5e3765d60f4e257fc665/libavutil/pixfmt.h#L72):
AVPixelFormat inputFormat = AV_PIX_FMT_NONE;
There was a problem hiding this comment.
@dvrogozh, oh, you mean for the individual fields. Yes, got it.
There was a problem hiding this comment.
Yes. Thank you for the change.
| AVRational timeBase = {0, 0}; | ||
| UniqueAVBufferRef hwFramesCtx; | ||
|
|
||
| FiltersContext() = default; |
There was a problem hiding this comment.
Yes. Thank you for the change.
3 participants
Because we're creating this in one place, with all fields always available, we should use a constructor. Note that the current code does not yet use
FiltersContext::hwFramesCtx, so we don't accept it in the constructor yet.