Implement CenterCrop #1094
Implement CenterCrop #1094
Conversation
meta-cla
bot
added
the
CLA Signed
| :template: dataclass.rst | ||
|
|
||
| DecoderTransform | ||
| RandomCrop |
There was a problem hiding this comment.
I think this is #1081 's driveby, we also need CenterCrop here now :)
| std::string coordinates = x_.has_value() | ||
| ? (":" + std::to_string(x_.value()) + ":" + std::to_string(y_.value())) | ||
| : ""; | ||
| return "crop=" + std::to_string(outputDims_.width) + ":" + | ||
| std::to_string(outputDims_.height) + ":" + std::to_string(x_) + ":" + | ||
| std::to_string(y_) + ":exact=1"; | ||
| std::to_string(outputDims_.height) + coordinates + ":exact=1"; |
There was a problem hiding this comment.
Just to check my understanding, it's a filtergraph built-in behavior that passing "crop=w:h" leads to a center crop?
There was a problem hiding this comment.
Yes, exactly. I can update the comment to make that clear. From the docs:
x
The horizontal position, in the input video, of the left edge of the output video. It defaults to (in_w-out_w)/2. This expression is evaluated per-frame.y
The vertical position, in the input video, of the top edge of the output video. It defaults to(in_h-out_h)/2. This expression is evaluated per-frame.
| CropTransform(const FrameDims& dims, int x, int y); | ||
|
|
||
| // Becomes a center crop if x and y are not specified. | ||
| CropTransform(const FrameDims& dims); |
There was a problem hiding this comment.
To follow-up on your main comment about using CropTransform for both RandomCrop and CenterCrop, I think what you have here, and the rationale behind it, make sense.
2 participants
Depends on #1081.
Implementation of
torchcodec.transforms.CenterCrop.The only potential surprise here is the change to the C++
Croptransform, where we are using it to implement bothCenterCropandRandomCrop. My rationale here is that the C++ objects have a 1:1 mapping to the FFmpeg filters, and the objects intorchcodec.transforms.DecoderTransformcan map to either a TorchVision transform or directly to an FFmpeg filter. (We don't have any of the latter yet.) The code incustom_ops.cppis the bridge between the two.I'm open to instead making the C++ side not share crop implementations and define a C++
CenterCropthat takes no x and y coordinates.