[qcms] Add matrix transform flag and matrix extraction api

[qcms] Add matrix transform flag and matrix extraction api

When a color transform is matrix-based (transforms using ICC V2 profiles
are commonly matrix-based), flag the color transform struct as matrix at
creation-time. Add an api to query if a color transform is matrix and an
api to extract the color transform matrix in row major order.

BUG=491826
Committed: https://crrev.com/aac0a90fac9ff1deef5a5a49ec16a5f509a9a1fa
Cr-Commit-Position: refs/heads/master@{#331615}

[Noel Gordon, 2015-05-25 07:09:20]

Patchset #1 (id:1) has been deleted

[Noel Gordon, 2015-05-25 08:25:47]

noel@chromium.org changed reviewers:
+ robert.bradford@intel.com

[Noel Gordon, 2015-05-25 12:40:13]

noel@chromium.org changed reviewers:
+ junov@chromium.org
- robert.bradford@intel.com

[Noel Gordon, 2015-05-27 11:22:02]

Not possible to read out the matrix in one go, due to it internal structure [ 3
x 4 ] used to speed-up SSE correction code. About the only trick is the matrix
in stored column-major order internally, so permute i and j on access to return
the more conventional row-major order.

Patch ready to review / land, will take a review from the willing.

[Noel Gordon, 2015-05-27 11:22:28]

noel@chromium.org changed reviewers:
+ robert.bradford@intel.com

[robert.bradford, 2015-05-27 16:03:27]

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
File third_party/qcms/src/transform.c (right):

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1453: float
qcms_transform_get_matrix(qcms_transform *t, int i, int j)
unsigned int or size_t?

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1453: float
qcms_transform_get_matrix(qcms_transform *t, int i, int j)
Perhaps call it _matrix_value - in case in the future there is a matrix type to
expose it as?

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1459: return t->matrix[j][i];
Should we be doing bounds checks on array dereference?

[Stephen White, 2015-05-27 17:17:48]

senorblanco@chromium.org changed reviewers:
+ senorblanco@chromium.org

[Stephen White, 2015-05-27 17:17:49]

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
File third_party/qcms/src/transform.c (right):

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1369: transform->transform_flags |=
TRANSFORM_FLAG_MATRIX;
Does transform_flags need to be initialized to zero somewhere?

In the non-HAVE_POSIX_MEMALIGN case transform_alloc() use calloc(), which means
the above is fine. It's the posix_memalign() case I'm concerned about.

[Noel Gordon, 2015-05-27 17:21:56]

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
File third_party/qcms/src/transform.c (right):

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1453: float
qcms_transform_get_matrix(qcms_transform *t, int i, int j)
On 2015/05/27 16:03:27, robert.bradford wrote:
> unsigned int or size_t?

Yes, made it unsigned.

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1453: float
qcms_transform_get_matrix(qcms_transform *t, int i, int j)
On 2015/05/27 16:03:27, robert.bradford wrote:
> Perhaps call it _matrix_value - in case in the future there is a matrix type
to
> expose it as?

The i, j kinda does that for me too.  Let's leave it as is for now.

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1459: return t->matrix[j][i];
On 2015/05/27 16:03:27, robert.bradford wrote:
> Should we be doing bounds checks on array dereference?

Done.

[Noel Gordon, 2015-05-27 17:42:56]

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
File third_party/qcms/src/transform.c (right):

https://codereview.chromium.org/1155033003/diff/40001/third_party/qcms/src/tr...
third_party/qcms/src/transform.c:1369: transform->transform_flags |=
TRANSFORM_FLAG_MATRIX;
On 2015/05/27 17:17:49, Stephen White wrote:
> Does transform_flags need to be initialized to zero somewhere?
>
> In the non-HAVE_POSIX_MEMALIGN case transform_alloc() use calloc(), which
means
> the above is fine. It's the posix_memalign() case I'm concerned about.

calloc-ed as you noted, that's the normal case for us.  Dunno if we use
HAVE_POSIX_MEMALIGN, but other chromium users might .. so yes, let's zero it.

[Noel Gordon, 2015-05-27 17:44:32]

On 2015/05/27 17:42:56, noel gordon wrote:

> calloc-ed as you noted, that's the normal case for us.  Dunno if we use
> HAVE_POSIX_MEMALIGN, but other chromium users might .. so yes, let's zero it.

Done.

[Stephen White, 2015-05-27 17:49:46]

On 2015/05/27 17:44:32, noel gordon wrote:
> On 2015/05/27 17:42:56, noel gordon wrote:
> 
> > calloc-ed as you noted, that's the normal case for us.  Dunno if we use
> > HAVE_POSIX_MEMALIGN, but other chromium users might .. so yes, let's zero
it.
> 
> Done.

non-OWNER LGTM (although Noel seems to be the sole OWNER..)

[Noel Gordon, 2015-05-27 17:56:28]

The CQ bit was checked by noel@chromium.org

[commit-bot: I haz the power, 2015-05-27 17:58:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1155033003/80001

[commit-bot: I haz the power, 2015-05-27 18:45:25]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2015-05-27 18:46:22]

Patchset 4 (id:??) landed as
https://crrev.com/aac0a90fac9ff1deef5a5a49ec16a5f509a9a1fa
Cr-Commit-Position: refs/heads/master@{#331615}