Hello! I am preparing to submit a patch for ff_h2645_extract_rbsp in libavcodec/h2645_parse.c. I believe there are sufficient performance gains to be made by tweaking the large-stride RBSP escape code detection logic. This would be my first contribution to ffmpeg, so I have several questions about best practice. Please forgive me if this is the incorrect venue for these questions.
The current approach uses unaligned 64-byte loads and some bit tricks to detect
zero bytes. Because RBSP escape sequences are 0x00, 0x00, 0x03, you only need to
scan every other byte. This is paired with a 9-byte stride, to minimize the
number of bytes inspected.
My own testing suggests this is shortsighted -- most platforms that support
unaligned 64-byte loads appear to be performing two aligned loads and masking
them together. Combined with the odd stride, this means we are preforming
roughly twice the required amount of loads. My assumptions were validated by
some anecdotal testing on my own CPU, but I am not confident that my statement
is
true for all targets.
Updating the scan to seek to an 8-byte offset, and then proceed in aligned
8-byte strides, appears to confer a significant performance improvement. My
tests
showed a 32% performance improvement, but this was a custom harness that just
does RBSP depayloading, and again, my CPU (AMD EPYC 9754) is not necessarily
a representative example.
In general, though, I expect the performance improvements to hold for _modern_
machines, but could hurt performance of 'older' targets. My rough understanding
is that lookahead fetch engines became common ~2010. Targets without robust
lookahead fetchers may see a slight performance hit.
My questions are twofold:
1. What validation work of the performance improvement should I preform before
writing and submitting a patch? Assuming I am confident that the patch yields
a performance improvement on most modern targets, would ffmpeg welcome this
change? Or is the performance stability of legacy platforms too important?
2. How should I approach big-endian support? As my patch would involve a lot of
64-bit literals, I could either:
- (1) prioritize code size, providing one implementation, wrapping loads with
a macro that converts BE to LE as needed. This would be cleanest imho,
but incur a slight penalty on big-endian targets.
- (2) prioritize performance, providing two mostly-identical implementations,
one for each endianess.
Thanks for the time,
Carson Riker
signature.asc
Description: OpenPGP digital signature
_______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
