On Wed, 3 May 2023, myais wrote:
Hello,
- I splited this patch, Do I need to resubmit or just attach them as
attachments? (I attached those patches. If I need to resummit, please let me
know.)
The attached form here is fine with me.
I didn't review it in detail yet, but I think it roughly looks ok, but
there's a few issues.
The "fate-hevc" tests as part of our testsuite fails with these patches
applied - please test that and make sure it passes.
The assembly fails to build with MSVC, with errors like these:
libavcodec\aarch64\hevcdsp_qpel_neon.o.asm(2278)
: error A2173: syntax error in expression
add x2, x2, x3, lsl 1
The immediate constant 1 here should be prefixed with #, the same thing
goes in a lot of other places in the same file.
If compiling only patch 1, it fails to build due to a mismatched #endif in
the c file.
The second patch adds code that git flags as "No newline at end of file";
please don't do that, please make sure your editor saves the file as usual
with a trailing newline.
The patches have some cases of trailing whitespace, please make sure you
don't have time.
In the second patch, you're inconsistently using "#if
__ARM_FEATURE_DOTPROD" and "#if defined(__ARM_FEATURE_DOTPROD)".
Dot product is a new feature we haven't taken advantage of on aarch64
before. None of my toolchains/environments have this enabled by default.
It would be good if you'd provide examples of where you're testing it and
how you configure the build to enable it.
Because right now, most of this assembly runs untested unless building in
a specific configuration that explicitly enables that extension.
For such features, we generally would want to always compile/assemble the
feature (as long as the toolchain supports assembling it, with some extra
flag), and use runtime checks for detecting whether the cpu feature is
supported. But I guess adding support for that is a bigger separate
project, and this approach with build time ifdefs if the toolchain
mandates support for it, is tolerable for now.
If there's not a huge benefit from the dot product instructions, maybe it
would be best to just not use them, so the whole wide audience can benefit
from the optimizations?
// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
