[RFC] Decoder-native transforms design

[scotts]

Sketching a design and implementation plan for #526.

[NicolasHug]

It will be probably be transforms dependent, but I agree with your call that in general this will be per-video. I think that ideally this would be "per-scene", but that's not in scope.

[NicolasHug]

make_params() is mostly relevant for random transforms, either those that are randomly applied, or those that do random sampling. Resize isn't random. The relevant parameters are just stored as public attributes on the Transform object.

[NicolasHug]

Three things come to mind:

• For non-random transforms make_transforms() doesn't exist, the relevant attributes are just attributes on the Transform object (mentioned below as well). That's not an issue, we can easily work around that.
• make_params() in its current state requires the input frames to be passed (flat_inputs). Those are expected to be materialized tensors, which we definitely won't have by the time we need to call make_params() in TorchCodec. Most of the time, all of what's actually needed from flat_inputs are the H and W of the frames. So we should be able to get around that by slightly modifying the torchvision part, and accept H and W instead of flat_input as input to make_params() (or similar new hook). We'll probably want to keep this private for the time being.
• the logic in make_param() is often dependent on the H and W of the input frames, as described above. For most streams that's not an issue because H and W are constant. For the streams with variable resolution, that quickly becomes very hairy. Do we call make_params() again on the new H and W? Where? We're risking sampling new random values that may lead to nonsensical transforms for a given video!

[scotts]

@NicolasHug, on the three things:

• We can work around it, but it does mean we'll probably need to do a lot of if isinstance(t, Resize), as that means there is no uniform way to discover a transforms parameters. We'll need to know that for transform X it has params Y and Z.
• Noted. I think we can start the implementation with Resize, and punt on these issues until we do RandomCrop.
• FFmpeg filters such as crop actually accept expressions, not just number values: https://ffmpeg.org/ffmpeg-filters.html#crop. Some are evaluated per-frame, so we may be able to use those to deal with variable resolution videos. For first versions, I also think it's reasonable to say variable resolution videos are not fully supported.

[NicolasHug]

Sounds reasonable to stick to filtergraph for this, but I wonder how this affect our current use of libswscale for color-conversion? Maybe we'll have to declare that any use of the transforms parameter means filtergraph is used for color-conversion, despite libswscale being faster according to our past benchmarks?

[scotts]

I think we can do a bit of both. Some transforms (such as Resize) we can support on libswscale. I assume there's a lot of more involved transforms that are not going to work on libswscale. We can add a virtual member function to this C++ class that returns a libswscale string, if possible. If all transforms are supportable on libswscale and all other its other criteria are met, we use that. If not, we have to use filtergraph.

We can also start with just filtergraph, and do libswscale as an optimization.

[Dan-Flores]

This is a good solution for Design Consideration 3. Do you foresee any issues from adding a dependency on torchvision in torchcodec?

[scotts]

Good call out. I'd like to explore if we can avoid an explicit dependency and instead go a duck-typing route. We absolutely want to accept TorchVision transforms. But it would be great if we could avoid an explicit dependency from TorchCodec to TorchVision.