On Sat, 16 Jul 2022, Michael Niedermayer wrote:

On Sat, Jul 16, 2022 at 03:30:23PM +0300, Martin Storsjö wrote:
On Sat, 16 Jul 2022, Michael Niedermayer wrote:

On Sat, Jul 16, 2022 at 12:25:37AM +0300, Martin Storsjö wrote:
On Fri, 15 Jul 2022, Michael Niedermayer wrote:

On Fri, Jul 15, 2022 at 10:56:03PM +0300, Martin Storsjö wrote:
On Fri, 15 Jul 2022, Swinney, Jonathan wrote:

If the max height is just 16, then this should be fine. I assumed that h
could have a much higher value (>1024), but if that is not the case,
then this is a useful optimization.

At least according to the me_cmp.h header, which says:

/* Motion estimation:
 * h is limited to { width / 2, width, 2 * width },
 * but never larger than 16 and never smaller than 2.
 * Although currently h < 4 is not used as functions with
 * width < 8 are neither used nor implemented. */

These rules where written with support for encoding of all
standard formats in mind at the time that was written.
today it may make sense to extend these rules to cover the
things which where created since then

Right, but if that suddenly changes, such a change also must expect that it
might need updates to all assembly implementations that implement that
interface currently. Right now, both the defacto case (any callers in the
codebase) and the explicit documentation says that it can't be called with
parameters outside of that range.

What i meant was that newly added functions should be more flexible than
these old rules. That is 2 sets of rules
1. What a caller ATM can do and expect to work (thats whats written there)
2. What an implementor of new functions should make sure is supported

With 2., do you mean if adding a new function into the same struct, or if
implementing the existing pix_abs[0][..] functions?

i would say both



If you mean new implementations of the existing function interface, you say
they "should be more flexible". How flexible must they be? Is it ok to
assume h<=256 for the w=16 functions?

i think thats fine

Ok, I'll go ahead and push this then.


Gradually increasing the requirements for existing function interfaces like
you suggest is really problematic.

why ?
iam really just saying
"when you add new code, dont base it on old limitations"

For this case, I just quoted what the header said, which seemed authoritative to me - but it's fine for me with a wider spec too, up to h=256. But saying arbitrarily "any height" really inhibits what you can do in asm.

Also if I shouldn't reference that limitation in the header, please update/reword it, as it's actively misleading for anyone working on this right now.

And in general, we can design for an intended use case and calculate whether it should work or not - but as long as it's not tested, those cases will often have hidden bugs - see e.g. patch 1/5 in this series.

// 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".

Reply via email to