Skip to content

Commit ed01d74

Browse files
lgritzlgritz
authored
feat(oiiotool): Expression eval improvements: BOTTOM, IMG[expression], check label/var names (#4334)
Improvements to expression evaluation of oiiotool command line arguments surrounded by `{braces}`. * Just like `TOP` refers to the top image on the stack, now `BOTTOM` refers to the bottom image. * Referring to images with `IMG[id]` previously worked if `id` was a literal integer or an image label or an image filepath. But now it can be any expression that evaluates to any of those things. So, for example, you could say `{IMG[i].filename}` to get the filename of the i-th image down the stack (where `i` is a variable), or `{IMG[NIMAGES-2].width}` to get the x resolution of the 2nd-from-bottom image on the stack. * The `--label` and `--set` commands check that the name you use for an image label or user variable follow the "C identifier" lexical rules: must be alphanumeric + underscore, but not start with number. It was previously possibly to do completely chaotic things like `--set 1 2`. --------- Signed-off-by: Larry Gritz <lg@larrygritz.com>
1 parent 0d824b6 commit ed01d74

File tree

7 files changed

+111
-29
lines changed

7 files changed

+111
-29
lines changed

‎src/doc/oiiotool.rst‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -123,12 +123,18 @@ contents of an expression may be any of:
123123
If there is no metadata whose name matches, the expression will not have any
124124
substitution made and an error will be issued.
125125

126-
The *imagename* may be one of: `TOP` (the top or current image), `IMG[i]`
127-
describing the i-th image on the stack (thus `TOP` is a synonym for
128-
`IMG[0]`, the next image on the stack is `IMG[1]`, etc.), or `IMG[name]`
129-
to denote an image named by filename or by label name. Remember that the
130-
positions on the stack (including `TOP`) refer to *at that moment*, with
131-
successive commands changing the contents of the top image.
126+
The *imagename* may be one of:
127+
128+
* `TOP` : the top or current image;
129+
* `BOTTOM` : the image at the bottom of the stack;
130+
* `IMG[index]` : if `index` evaluates to an integer `i`, the i-th image on
131+
the stack (thus `TOP` is a synonym for `IMG[0]`, the next image on the
132+
stack is `IMG[1]`, ..., and `BOTTOM` is a synonmym for `IMG[NIMAGES-1]`);
133+
* `IMG[name]` : an image named by filename or by label name.
134+
135+
Remember that the positions on the stack (including `TOP`) refer to *at that
136+
moment*, with successive commands changing the contents of the top image. If
137+
the
132138

133139
The *metadata* may be any of:
134140

@@ -982,6 +988,10 @@ output each one to a different file, with names `sub0001.tif`,
982988
sign), it will be saved as `int`; if it also contains a decimal point, it
983989
will be saved as a `float`; otherwise, it will be saved as a `string`.
984990

991+
The name of the variable must be in the form of an "identifier" (a
992+
sequence of alphanumeric characters and underscores, starting with a
993+
letter or underscore).
994+
985995
This command was added in OIIO 2.4.0.
986996

987997
Examples::
@@ -2242,6 +2252,10 @@ current top image.
22422252
the usual manner that an ordinary input image would be specified by
22432253
filename.
22442254

2255+
The name of the label must be in the form of an "identifier" (a sequence
2256+
of alphanumeric characters and underscores, starting with a letter or
2257+
underscore).
2258+
22452259

22462260
:program:`oiiotool` commands that make entirely new images
22472261
==========================================================

‎src/oiiotool/expressions.cpp‎

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,34 +149,70 @@ Oiiotool::express_parse_atom(const string_view expr, string_view& s,
149149
return false;
150150

151151
} else if (Strutil::starts_with(s, "TOP")
152+
|| Strutil::starts_with(s, "BOTTOM")
152153
|| Strutil::starts_with(s, "IMG[")) {
153154
// metadata substitution
154155
ImageRecRef img;
155156
if (Strutil::parse_prefix(s, "TOP")) {
156157
img = curimg;
158+
} else if (Strutil::parse_prefix(s, "BOTTOM")) {
159+
img = (image_stack.size() <= 1) ? curimg : image_stack[0];
157160
} else if (Strutil::parse_prefix(s, "IMG[")) {
158-
int index = -1;
159-
if (Strutil::parse_int(s, index) && Strutil::parse_char(s, ']')
160-
&& index >= 0 && index <= (int)image_stack.size()) {
161-
if (index == 0)
162-
img = curimg;
163-
else
164-
img = image_stack[image_stack.size() - index];
165-
} else {
166-
string_view name = Strutil::parse_until(s, "]");
167-
auto found = image_labels.find(name);
168-
if (found != image_labels.end())
169-
img = found->second;
170-
else
171-
img = ImageRecRef(new ImageRec(name, imagecache));
172-
Strutil::parse_char(s, ']');
161+
std::string until_bracket = Strutil::parse_until(s, "]");
162+
if (until_bracket.empty() || !Strutil::parse_char(s, ']')) {
163+
express_error(expr, until_bracket,
164+
"malformed IMG[] specification");
165+
result = orig;
166+
return false;
167+
}
168+
auto labelfound = image_labels.find(until_bracket);
169+
if (labelfound != image_labels.end()) {
170+
// Found an image label
171+
img = labelfound->second;
172+
} else if (Strutil::string_is_int(until_bracket)) {
173+
// It's an integer... don't process more quite yet
174+
} else if (Filesystem::exists(until_bracket)) {
175+
// It's the name of an image file
176+
img = ImageRecRef(new ImageRec(until_bracket, imagecache));
177+
}
178+
if (!img) {
179+
// Not a label, int, or file. Evaluate it as an expression.
180+
// Evaluate it as an expression and hope it's an integer or
181+
// the name of an image?
182+
until_bracket = express_impl(until_bracket);
183+
if (Strutil::string_is_int(until_bracket)) {
184+
// Between brackets (including an expanded variable) is an
185+
// integer -- it's an index into the image stack (error if out
186+
// of range).
187+
int index = Strutil::stoi(until_bracket);
188+
if (index >= 0 && index <= (int)image_stack.size()) {
189+
img = (index == 0)
190+
? curimg
191+
: image_stack[image_stack.size() - index];
192+
} else {
193+
express_error(expr, until_bracket,
194+
"out-of-range IMG[] index");
195+
result = orig;
196+
return false;
197+
}
198+
} else if (Filesystem::exists(until_bracket)) {
199+
// It's the name of an image file
200+
img = ImageRecRef(new ImageRec(until_bracket, imagecache));
201+
}
202+
}
203+
if (!img || img->has_error()) {
204+
express_error(expr, until_bracket, "not a valid image");
205+
result = orig;
206+
return false;
173207
}
174208
}
175-
if (!img.get()) {
209+
if (!img || img->has_error()) {
176210
express_error(expr, s, "not a valid image");
177211
result = orig;
178212
return false;
179213
}
214+
OIIO_ASSERT(img);
215+
img->read();
180216
bool using_bracket = false;
181217
if (Strutil::parse_char(s, '[')) {
182218
using_bracket = true;

‎src/oiiotool/oiiotool.cpp‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -699,8 +699,13 @@ unset_autopremult(Oiiotool& ot, cspan<const char*> argv)
699699
static void
700700
action_label(Oiiotool& ot, cspan<const char*> argv)
701701
{
702-
string_view labelname = ot.express(argv[1]);
703-
ot.image_labels[labelname] = ot.curimg;
702+
string_view command = ot.express(argv[0]);
703+
string_view name = ot.express(argv[1]);
704+
if (!Strutil::string_is_identifier(name)) {
705+
ot.errorfmt(command, "Invalid label name \"{}\"", name);
706+
return;
707+
}
708+
ot.image_labels[name] = ot.curimg;
704709
}
705710

706711

@@ -1397,7 +1402,13 @@ set_user_variable(Oiiotool& ot, cspan<const char*> argv)
13971402
string_view command = ot.express(argv[0]);
13981403
string_view name = ot.express(argv[1]);
13991404
string_view value = ot.express(argv[2]);
1400-
auto options = ot.extract_options(command);
1405+
1406+
if (!Strutil::string_is_identifier(name)) {
1407+
ot.errorfmt(command, "Invalid variable name \"{}\"", name);
1408+
return 0;
1409+
}
1410+
1411+
auto options = ot.extract_options(command);
14011412
TypeDesc type(options["type"].as_string());
14021413

14031414
set_attribute_helper(ot.uservars, name, value, type);

‎testsuite/oiiotool-control/ref/out.txt‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ Testing --set of implied types:
5858
i = 42, f = 3.5, s = hello world
5959
Testing --set of various explicit types:
6060
i = 42, f = 3.5, s = hello world, tc = 01:02:03:04, rat = 1/2
61+
This should make an error:
62+
oiiotool ERROR: -set : Invalid variable name "3"
63+
Full command line was:
64+
> oiiotool -echo "This should make an error:" -set 3 5
6165
Expr getattribute(limits:channels) = 1024
6266
Testing if with true cond (expect output):
6367
inside if clause, i=42
@@ -255,7 +259,9 @@ Stats FiniteCount: 12288 12288 12288
255259
Constant: No
256260
Monochrome: No
257261

258-
Stack holds [0] = ../common/tahoe-small.tif, [1] = ../common/tahoe-tiny.tif
262+
Stack holds [0] = ../common/grid.tif, [1] = ../common/tahoe-small.tif, [2] = ../common/tahoe-tiny.tif
263+
TOP = ../common/grid.tif, BOTTOM = ../common/tahoe-tiny.tif
264+
Stack holds [1] = ../common/tahoe-small.tif
259265
filename=../common/tahoe-tiny.tif file_extension=.tif file_noextension=../common/tahoe-tiny
260266
MINCOLOR=0,0,0 MAXCOLOR=0.745098,1,1 AVGCOLOR=0.101942,0.216695,0.425293
261267
Testing NIMAGES:

‎testsuite/oiiotool-control/run.py‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@
8888
'-set:type=timecode tc 01:02:03:04 ' +
8989
'-set:type=rational rat 1/2 ' +
9090
'-echo " i = {i}, f = {f}, s = {s}, tc = {tc}, rat = {rat}"')
91+
command += oiiotool ('-echo "This should make an error:" ' +
92+
'-set 3 5')
9193

9294
# Test getattribute in an expression
9395
command += oiiotool ('-echo "Expr getattribute(\"limits:channels\") = {getattribute(\"limits:channels\")}"')
@@ -132,9 +134,13 @@
132134
+ " --echo \"\\nMeta native: {TOP.METANATIVE}\""
133135
+ " --echo \"\\nStats:\\n{TOP.STATS}\\n\"")
134136

135-
# Test IMG[]
136-
command += oiiotool ("../common/tahoe-tiny.tif ../common/tahoe-small.tif " +
137-
"--echo \"Stack holds [0] = {IMG[0].filename}, [1] = {IMG[1].filename}\"")
137+
# Test IMG[], TOP, BOTTOM
138+
command += oiiotool ("../common/tahoe-tiny.tif ../common/tahoe-small.tif ../common/grid.tif " +
139+
"--echo \"Stack holds [0] = {IMG[0].filename}, [1] = {IMG[1].filename}, [2] = {IMG[2].filename}\" " +
140+
"--echo \"TOP = {TOP.filename}, BOTTOM = {BOTTOM.filename}\" " +
141+
"--set i 1 " +
142+
"--echo \"Stack holds [{i}] = {IMG[i].filename}\" "
143+
)
138144

139145
# Test some special attribute evaluation names
140146
command += oiiotool ("../common/tahoe-tiny.tif " +

‎testsuite/oiiotool/ref/out.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44
0 < 0,0,0
55
6400 > 1,0.9,1
66
59136 within range
7+
This should make an error:
8+
oiiotool ERROR: --label : Invalid label name "2hot2handle"
9+
Full command line was:
10+
> oiiotool -echo "This should make an error:" --create 1x1 3 --label 2hot2handle -o out.tif
711
--printstats:
812
128 x 96, 3 channel, float tiff
913
Stats Min: 0 0 0 (of 255)

‎testsuite/oiiotool/run.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
# SPDX-License-Identifier: Apache-2.0
55
# https://github.com/AcademySoftwareFoundation/OpenImageIO
66

7+
redirect += " 2>&1"
8+
failureok = True
9+
710
# Create some test images we need
811
command += oiiotool ("--create 320x240 3 -d uint8 -o black.tif")
912
command += oiiotool ("--pattern constant:color=0.5,0.5,0.5 128x128 3 -d half -o grey128.exr")
@@ -180,6 +183,8 @@
180183
" --pattern constant:color=0.5,0.0,0.0 128x128 3 --label B " +
181184
" --pop --pop --pop " +
182185
" R G --add -d half -o labeladd.exr")
186+
command += oiiotool ('-echo "This should make an error:" ' +
187+
'--create 1x1 3 --label 2hot2handle -o out.tif')
183188

184189
# test subimages
185190
command += oiiotool ("--pattern constant:color=0.5,0.0,0.0 64x64 3 " +

0 commit comments

Comments
 (0)