Fix example plugin problems found by fuzzing: Test, ChoiceParams, ColourSpace, DepthConverter

Examples/ChoiceParams/choiceparams.cpp

@@ -195,6 +195,10 @@ getTemporalDomain( OfxImageEffectHandle  effect,  OfxPropertySetHandle /*inArgs*
 static OfxStatus 
 getClipPreferences( OfxImageEffectHandle  effect,  OfxPropertySetHandle /*inArgs*/,  OfxPropertySetHandle outArgs)
 {
+  // our only preference is the output depth, which needs a host that supports multiple depths
+  if(!gHostSupportsMultipleBitDepths)
+    return kOfxStatReplyDefault;
+
   // retrieve any instance data associated with this effect
   MyInstanceData *myData = getMyInstanceData(effect);
 
@@ -207,8 +211,7 @@ getClipPreferences( OfxImageEffectHandle  effect,  OfxPropertySetHandle /*inArgs
   const char *bitDepthStr = bitDepth == 8 ? kOfxBitDepthByte : (bitDepth == 16 ? kOfxBitDepthShort : kOfxBitDepthFloat);
 
   // set out output to be the same same as the input bitdepth
-  if(gHostSupportsMultipleBitDepths)
-    gPropHost->propSetString(outArgs, "OfxImageClipPropDepth_Output", 0, bitDepthStr);
+  gPropHost->propSetString(outArgs, "OfxImageClipPropDepth_Output", 0, bitDepthStr);
 
   return kOfxStatOK;
 }

Examples/ColourSpace/colourspace.cpp

@@ -61,7 +61,8 @@ ColourManagementStyle ofxstring_to_style(const std::string & style)
 
 static space_info input_space_choices[]
 {
-  { "[Unspecified, accepts anything]", NULL, ColourManagementStyle::Basic },
+  // offered in every style, so the parameter always has a choice
+  { "[Unspecified, accepts anything]", "", ColourManagementStyle::None },
 
   // For this example plugin we'll offer every possible space defined in the header file
   // Real plugins do not need this flexibility!
@@ -137,7 +138,8 @@ static space_info input_space_choices[]
 
 static space_info output_space_choices[]
 {
-  { "[Same as input]", "OfxColourspace_Source", ColourManagementStyle::Basic },
+  // offered in every style, so the parameter always has a choice
+  { "[Same as input]", "OfxColourspace_Source", ColourManagementStyle::None },
 
   // For this example plugin we'll offer every possible space defined in the header file
   // Real plugins do not need this flexibility!
@@ -271,7 +273,8 @@ static constexpr T lerp(T a, T b, float amount) {
 } while (0)
 
 /**
- * Draw a string of text at the given position in the image
+ * Draw a string of text at the given position in the image, clipped to
+ * its xdim x ydim pixels
  */
 static void drawText(const std::string &message, int x, int y,
                      unsigned int font_height,
@@ -292,10 +295,10 @@ static void drawText(const std::string &message, int x, int y,
     float white = 255.0f;
     float color_scale = 1.0f/256.0f;
     float bg_opacity = 0.2f;
-    for (int iy = y, ty = 0; iy < ydim && ty < txt_height; iy++, ty++) {
+    for (int iy = std::max(y, 0), ty = iy - y; iy < ydim && ty < txt_height; iy++, ty++) {
       T *row = (T *)((unsigned char *)image + iy * rowbytes);
       float *txt_row = txt_img.data(0, txt_height - 1 - ty);
-      for (int ix = x, tx = 0; ix < xdim && tx < txt_width; ix++, tx++) {
+      for (int ix = std::max(x, 0), tx = ix - x; ix < xdim && tx < txt_width; ix++, tx++) {
         switch (nchannels) {
         case 1:                 // Alpha only
           row[ix*4] = std::max(row[ix*4], (T)(txt_row[tx] * color_scale));
@@ -318,10 +321,10 @@ static void drawText(const std::string &message, int x, int y,
     float white = 65535.0f;
     float color_scale = 1.0f/65536.0f;
     float bg_opacity = 0.2f;
-    for (int iy = y, ty = 0; iy < ydim && ty < txt_height; iy++, ty++) {
+    for (int iy = std::max(y, 0), ty = iy - y; iy < ydim && ty < txt_height; iy++, ty++) {
       T *row = (T *)((unsigned char *)image + iy * rowbytes);
       float *txt_row = txt_img.data(0, txt_height - 1 - ty);
-      for (int ix = x, tx = 0; ix < xdim && tx < txt_width; ix++, tx++) {
+      for (int ix = std::max(x, 0), tx = ix - x; ix < xdim && tx < txt_width; ix++, tx++) {
         switch (nchannels) {
         case 1:                 // Alpha only
           row[ix*4] = std::max(row[ix*4], (T)(txt_row[tx] * color_scale));
@@ -342,10 +345,10 @@ static void drawText(const std::string &message, int x, int y,
   case 32: {
     float white = 1.0f;
     float bg_opacity = 0.2f;
-    for (int iy = y, ty = 0; iy < ydim && ty < txt_height; iy++, ty++) {
+    for (int iy = std::max(y, 0), ty = iy - y; iy < ydim && ty < txt_height; iy++, ty++) {
       float *row = (float *)((unsigned char *)image + iy * rowbytes);
       float *txt_row = txt_img.data(0, txt_height - 1 - ty);
-      for (int ix = x, tx = 0; ix < xdim && tx < txt_width; ix++, tx++) {
+      for (int ix = std::max(x, 0), tx = ix - x; ix < xdim && tx < txt_width; ix++, tx++) {
         switch (nchannels) {
         case 1:                 // Alpha only
           row[ix*4] = std::max(row[ix*4], txt_row[tx]);
@@ -685,6 +688,12 @@ static std::string getClipColourspace(const OfxImageClipHandle clip) {
 
 }
 
+// the address of pixel (x, y) in an image whose data starts at its bounds' bottom left
+static char *pixelAddress(void *data, const OfxRectI &bounds, int rowBytes, int bytesPerPixel, int x, int y)
+{
+  return (char *)data + (y - bounds.y1) * rowBytes + (x - bounds.x1) * bytesPerPixel;
+}
+
 static OfxStatus render( OfxImageEffectHandle  instance,
                          OfxPropertySetHandle inArgs,
                          OfxPropertySetHandle /*outArgs*/)
@@ -743,33 +752,51 @@ static OfxStatus render( OfxImageEffectHandle  instance,
     outputImg = ofxuGetImage(myData->outputClip, time, dstRowBytes, dstBitDepth, dstIsAlpha, dstRect, dst);
     if(outputImg == NULL) throw OfxuNoImageException();
 
-    // see if they have the same depths and bytes and all
-    if(srcBitDepth != dstBitDepth || srcIsAlpha != dstIsAlpha || srcRowBytes != dstRowBytes) {
+    // see if they have the same depths and all
+    if(srcBitDepth != dstBitDepth || srcIsAlpha != dstIsAlpha) {
       throw OfxuStatusException(kOfxStatErrImageFormat);
     }
 
-    int xdim = srcRect.x2 - srcRect.x1;
-    int ydim = srcRect.y2 - srcRect.y1;
+    // the whole frame, of which the render window may be one tile
+    OfxRectI frame;
+    gPropHost->propGetIntN(outputImg, kOfxImagePropRegionOfDefinition, 4, &frame.x1);
+    int xdim = frame.x2 - frame.x1;
+    int ydim = frame.y2 - frame.y1;
     // do the rendering
 
     int nchannels = 4;
+    int bytesPerPixel = nchannels * dstBitDepth / 8;
     int font_height = 50 * renderScale[0];
-    spdlog::info(OFX_FMT_STRING("Rendering {}x{} image @{},{}, depth={}"), xdim, ydim, srcRect.x1, srcRect.y1, dstBitDepth);
-
-    // Just copy from source to dest, and draw some text
-    if (srcRowBytes < 0 && dstRowBytes < 0)
-      memcpy((char *)dst + dstRowBytes * (ydim-1), (char*)src + srcRowBytes * (ydim-1), -srcRowBytes * ydim);
-    else
-      memcpy(dst, src, srcRowBytes * ydim);
-    int ystart = ydim - 100;
+    spdlog::info(OFX_FMT_STRING("Rendering {}x{} image @{},{}, depth={}"), xdim, ydim, frame.x1, frame.y1, dstBitDepth);
+
+    int windowWidth = renderWindow.x2 - renderWindow.x1;
+    int windowHeight = renderWindow.y2 - renderWindow.y1;
+
+    // Copy the render window from source to dest a row at a time, as each
+    // image has its own bounds and row bytes; black where there is no source
+    for (int y = renderWindow.y1; y < renderWindow.y2; y++) {
+      char *dstRow = pixelAddress(dst, dstRect, dstRowBytes, bytesPerPixel, renderWindow.x1, y);
+      memset(dstRow, 0, windowWidth * bytesPerPixel);
+      int x1 = std::max(renderWindow.x1, srcRect.x1);
+      int x2 = std::min(renderWindow.x2, srcRect.x2);
+      if (y >= srcRect.y1 && y < srcRect.y2 && x1 < x2)
+        memcpy(dstRow + (x1 - renderWindow.x1) * bytesPerPixel,
+               pixelAddress(src, srcRect, srcRowBytes, bytesPerPixel, x1, y),
+               (x2 - x1) * bytesPerPixel);
+    }
+
+    // Draw some text, placed in the frame and clipped to the render window
+    char *window = pixelAddress(dst, dstRect, dstRowBytes, bytesPerPixel, renderWindow.x1, renderWindow.y1);
+    int xstart = frame.x1 + 100 - renderWindow.x1;
+    int ystart = frame.y2 - 100 - renderWindow.y1;
     drawText(fmt::format(OFX_FMT_STRING("Image: {}x{}, depth={}, scale={:.2f}x{:.2f}"), xdim, ydim, dstBitDepth, renderScale[0], renderScale[1]),
-             100, ystart, font_height, dst, xdim, ydim, dstBitDepth, nchannels, dstRowBytes);
+             xstart, ystart, font_height, window, windowWidth, windowHeight, dstBitDepth, nchannels, dstRowBytes);
     ystart -= font_height;
     drawText(fmt::format(OFX_FMT_STRING("input colourspace: {}"), inputColourspace),
-	     100, ystart, font_height, dst, xdim, ydim, dstBitDepth, nchannels, dstRowBytes);
+	     xstart, ystart, font_height, window, windowWidth, windowHeight, dstBitDepth, nchannels, dstRowBytes);
     ystart -= font_height;
     drawText(fmt::format(OFX_FMT_STRING("output colourspace: {}"), outputColourspace),
-             100, ystart, font_height, dst, xdim, ydim, dstBitDepth, nchannels, dstRowBytes);
+             xstart, ystart, font_height, window, windowWidth, windowHeight, dstBitDepth, nchannels, dstRowBytes);
   }
   catch(OfxuNoImageException &ex) {
     // if we were interrupted, the failed fetch is fine, just return kOfxStatOK
@@ -980,7 +1007,7 @@ template<ColourManagementStyle STYLE>
 static OfxStatus
 pluginMain(const char *action, const void *handle, OfxPropertySetHandle inArgs,  OfxPropertySetHandle outArgs)
 {
-  OfxStatus stat = kOfxStatOK;
+  OfxStatus stat = kOfxStatReplyDefault;
 
   if (silentActions.find(action) == silentActions.end())
     spdlog::info(OFX_FMT_STRING(">>> pluginMain({})"), action);

Examples/DepthConverter/depthConverter.cpp

@@ -31,6 +31,7 @@ static bool gSupportsBytes  = false;
 static bool gSupportsShorts = false;
 static bool gSupportsFloats = false;
 static int gDepthParamToBytes[3]; // maps the value of the bit depth param to a host supported bit depth
+static int gHostSupportsMultipleDepths = 0; // whether the host lets the output depth differ from the input's
 
 // pointers64 to various bits of the host
 OfxHost               *gHost;
@@ -374,6 +375,10 @@ getClipPreferences(OfxImageEffectHandle effect,
 		   OfxPropertySetHandle /*inArgs*/,
 		   OfxPropertySetHandle outArgs)
 {
+  // only a host that supports multiple clip depths lets us change the output's
+  if(!gHostSupportsMultipleDepths)
+    return kOfxStatReplyDefault;
+
   // retrieve any instance data associated with this effect
   MyInstanceData *myData = getMyInstanceData(effect);
 
@@ -389,6 +394,8 @@ getClipPreferences(OfxImageEffectHandle effect,
   case 16 : gPropHost->propSetString(outArgs, "OfxImageClipPropDepth_Output", 0, kOfxBitDepthShort); break;
   // float
   case 32 : gPropHost->propSetString(outArgs, "OfxImageClipPropDepth_Output", 0, kOfxBitDepthFloat); break;
+  // no depth the host supports, so nothing to set
+  default : return kOfxStatReplyDefault;
   }
 
   return kOfxStatOK;
@@ -461,17 +468,16 @@ describe(OfxImageEffectHandle  effect)
   if((stat = ofxuFetchHostSuites()) != kOfxStatOK)
     return stat;
 
-  int hostSupportsMultipleDepths;
   // record a few host features
-  gPropHost->propGetInt(gHost->host, kOfxImageEffectPropSupportsMultipleClipDepths, 0, &hostSupportsMultipleDepths);
+  gPropHost->propGetInt(gHost->host, kOfxImageEffectPropSupportsMultipleClipDepths, 0, &gHostSupportsMultipleDepths);
 
   // see how many bit depths the host supports, this affects our parameter values
   int nHostDepths;
   gPropHost->propGetDimension(gHost->host, kOfxImageEffectPropSupportedPixelDepths, &nHostDepths);
 
   // If the host cannot support multiple bit depths on in and out clips or it only supports 1 bit depth
   // we can't do any work, so refuse to load and explain why.
-  if(!hostSupportsMultipleDepths || nHostDepths == 1) {
+  if(!gHostSupportsMultipleDepths || nHostDepths == 1) {
     // post a message
     // - disabled, because posting a message within describe() crashes Nuke 6
     //gMessageSuite->message(effect, kOfxMessageError, kMessageNotEnoughBits,

Examples/Test/testProperties.cpp

@@ -966,7 +966,8 @@ getTemporalDomain( OfxImageEffectHandle  /*effect*/,  OfxPropertySetHandle /*inA
 static OfxStatus 
 getClipPreferences( OfxImageEffectHandle  /*effect*/,  OfxPropertySetHandle /*inArgs*/,  OfxPropertySetHandle /*outArgs*/)
 {
-  return kOfxStatOK;
+  // we set no preferences, so the host uses its defaults
+  return kOfxStatReplyDefault;
 }
 
 // are the settings of the effect performing an identity operation
@@ -1168,7 +1169,7 @@ pluginMain(const char *action,  const void *handle, OfxPropertySetHandle inArgsH
       checkMainHandles(action, handle, inArgsHandle, outArgsHandle, false, false, false);
     }
     else if(OFX::strEquals(action, kOfxImageEffectActionGetClipPreferences)) {
-      checkMainHandles(action, handle, inArgsHandle, outArgsHandle, false, false, false);
+      checkMainHandles(action, handle, inArgsHandle, outArgsHandle, false, true, false);
       stat = getClipPreferences(effectHandle, inArgsHandle, outArgsHandle);
     }
     else if(OFX::strEquals(action, kOfxImageEffectActionIsIdentity)) {

release-notes-next.md

@@ -27,6 +27,13 @@ This is version NEXT of the OpenFX API.
 - Fixed the ColourSpace example's `OfxSetHost` to have the proper signature so it actually gets called.
 - Fixed the `@propdef` metadata of `kOfxParamPropChoiceEnum` (a string array, not a bool) and `kOfxParamPropDimensionLabel` (one label per dimension, not one).
 - Fixed the Invert example never releasing its output image (a shadowed handle variable).
+- Fixed the Test example failing `kOfxImageEffectActionGetClipPreferences` with `kOfxStatErrBadHandle` because it required `inArgs`, which the spec passes as NULL for that action; it now answers `kOfxStatReplyDefault`, since it sets no preferences.
+- Fixed the ChoiceParams example answering `kOfxImageEffectActionGetClipPreferences` with `kOfxStatOK` when it set nothing; it now answers `kOfxStatReplyDefault` unless the host supports multiple clip depths.
+- Fixed the ColourSpace example answering `kOfxStatOK` to the actions it does not handle, such as `kOfxImageEffectActionGetFramesNeeded`; it now answers `kOfxStatReplyDefault`, as the spec requires of an action a plugin does not trap.
+- Fixed the ColourSpace example declaring its string-choice parameters with no enum values on a host without colour management, and passing a NULL string as the "unspecified" input colourspace on one with it.
+- Fixed the ColourSpace example writing its text overlay past the bottom and left edges of a frame too small to hold it.
+- Fixed the ColourSpace example under tiled rendering: it copied the whole source image to the start of each tile, overrunning the output buffer, and drew its text relative to each tile. It now copies only the render window and places the text in the frame.
+- Fixed the DepthConverter example asking for an output depth different from its input's on a host that does not support multiple clip depths, and claiming clip preferences it had not set.
 
 ## Deprecations