swscale/ops: don't strip SwsComps from SWS_OP_READ

The current behavior of assuming the value range implicitly on SWS_OP_READ
has a number of serious drawbacks and shortcomings:

- It ignored the effects of SWS_OP_RSHIFT, such as for p010 and related
  MSB-aligned formats. (This is actually a bug)

- It adds a needless dependency on the "purely informative" src/dst fields
  inside SwsOpList.

- It is difficult to reason about when acted upon by SWS_OP_SWAP_BYTES, and
  the existing hack of simply ignoring SWAP_BYTES on the value range is not
  a very good solution here.

Instead, we need a more principled way for the op list generating code
to communicate extra metadata about the operations read to the optimizer.

I think the simplest way of doing this is to allow the SwsComps field attached
to SWS_OP_READ to carry additional, user-provided information about the values
read.

This requires changing ff_sws_op_list_update_comps() slightly to not completely
overwrite SwsComps on SWS_OP_READ, but instead merge the implicit information
with the explictly provided one.
This commit is contained in:
Niklas Haas
2025-12-22 14:16:50 +01:00
committed by Niklas Haas
parent 61eca588dc
commit 9586b81373
2 changed files with 32 additions and 13 deletions
+18 -12
View File
@@ -232,14 +232,15 @@ void ff_sws_op_list_update_comps(SwsOpList *ops)
for (int n = 0; n < ops->num_ops; n++) {
SwsOp *op = &ops->ops[n];
/* Prefill min/max values automatically; may have to be fixed in
* special cases */
memcpy(op->comps.min, prev.min, sizeof(prev.min));
memcpy(op->comps.max, prev.max, sizeof(prev.max));
if (op->op != SWS_OP_SWAP_BYTES) {
ff_sws_apply_op_q(op, op->comps.min);
ff_sws_apply_op_q(op, op->comps.max);
if (op->op != SWS_OP_READ) {
/* Prefill min/max values automatically; may have to be fixed in
* special cases */
memcpy(op->comps.min, prev.min, sizeof(prev.min));
memcpy(op->comps.max, prev.max, sizeof(prev.max));
if (op->op != SWS_OP_SWAP_BYTES) {
ff_sws_apply_op_q(op, op->comps.min);
ff_sws_apply_op_q(op, op->comps.max);
}
}
switch (op->op) {
@@ -260,13 +261,18 @@ void ff_sws_op_list_update_comps(SwsOpList *ops)
}
}
op->comps.flags[i] = SWS_COMP_EXACT;
op->comps.min[i] = Q(0);
op->comps.max[i] = Q((1ULL << bits) - 1);
op->comps.flags[i] |= SWS_COMP_EXACT;
op->comps.min[i] = av_max_q(Q(0), op->comps.min[i]);
op->comps.max[i] = av_min_q(Q((1ULL << bits) - 1), op->comps.max[i]);
}
}
for (int i = op->rw.elems; i < 4; i++)
/* Explicitly strip flags and min/max range data for unread comps */
for (int i = op->rw.elems; i < 4; i++) {
op->comps.flags[i] = prev.flags[i];
op->comps.min[i] = prev.min[i];
op->comps.max[i] = prev.max[i];
}
break;
case SWS_OP_WRITE:
for (int i = 0; i < op->rw.elems; i++)
+14 -1
View File
@@ -195,7 +195,20 @@ typedef struct SwsOp {
SwsConst c;
};
/* For use internal use inside ff_sws_*() functions */
/**
* Metadata about the operation's input/output components.
*
* For SWS_OP_READ, this is informative; and lets the optimizer know
* additional information about the value range and/or pixel data to expect.
* The default value of {0} is safe to pass in the case that no additional
* information is known.
*
* For every other operation, this metadata is discarded and regenerated
* automatically by `ff_sws_op_list_update_comps()`.
*
* Note that backends may rely on the presence and accuracy of this
* metadata for all operations, during ff_sws_ops_compile().
*/
SwsComps comps;
} SwsOp;