[vpdq] Refactor video hashing

[ianwal]

Summary

Refactor video hashing. The combined class of hashing and decoding frames was complicated and difficult to understand. This PR splits it up into smaller classes, which also allows different frame types to be used in the future with minimal changes.

• Moved some FFmpeg custom types into separate files for better readability.

• Hashing is split into two new classes: VpdqHasher and FFmpegHasher.

  • VpdqHasher takes care of all hashing operations and just runs the generic hashing algorithm as frames are added.
    • A template is used as the frame type in VpdqHasher as opposed to an abstract class with derived frame types to avoid needing to dynamically allocate every frame. A generic frame is added that works without FFmpeg as an example, but is not used for anything currently. Using the direct FFmpeg data pointer is significantly faster than copying the data into the generic frame buffer.
  • New class FFmpegHasher that feeds FFmpeg frames to the VpdqHasher.

• Changed FFmpegHasher::processPacket to take in the data by reference instead of the full unique_ptr.

  • This is recommended by Herb Sutter GotW 91 if ownership is not being transferred.

• Minor change to matchTwoHashBrute to pass data by const& instead of copying for better performance.

I did not notice a performance impact from these changes across multiple runs of regtest.

No public API changes other than matchTwoHashBrute.

Most of these changes are just moving common functions into grouped common modules. No major logic or anything is rewritten. I would try to split up this PR but most of it is easier to understand in the context of the full changes.

Test Plan

regtest passes successfully.

[Dcallies]

Thanks again @ianwal!

Will try and review this later today!

Just a gut check on your end, do you think the existing regtests are reasonably comprehensive, or should we consider expanding the suite in the future? If so, what kind of changes would make you confident for refactors like this one?

[ianwal]

Just a gut check on your end, do you think the existing regtests are reasonably comprehensive, or should we consider expanding the suite in the future? If so, what kind of changes would make you confident for refactors like this one?

I trust the existing regtest for testing the "all good" cases, but more sample files with different codecs and containers and some bad data would be useful in general. Also unit testing and static analysis couldn't hurt. If this was a directly consumable CPP library, which would be very cool but somewhat impractical due to the pains of CPP package management, then I would definitely want some unit tests for public APIs across supported platforms.

For this PR specifically, this is almost entirely just moving stuff around and organizing so I think the regtest is good enough. And with how good the sanitizers are I would be surprised if there were any concurrency issues in the good cases.

[Dcallies]

Sorry, I left you hanging! Will attempt to complete review tomorrow.

[Dcallies]

Cool, thanks for the explanation, and looks good to go.