Option to apply watermaking across complete (CVT/IIIF) images #219
Conversation
|
Thanks very much @benr-cogapp for the patch. You're right that certain CVT requests don't apply the watermark when they should. For TIFF images for which the CVT output is generated from tiles, the watermark is correctly applied as in this example: https://merovingio.c2rmf.cnrs.fr/fcgi-bin/iipsrv-watermark.fcgi?FIF=PIA03883.pyr.tif&WID=750&CVT=jpeg However, watermarks are not applied to JPEG2000 (from either OpenJPEG or Kakadu) images as the CVT exports are generated in a single block directly from the codecs. The watermarks are applied within TileManager.cc, so as you've added a call to perform watermarking in CVT.cc, TIFF images will end up getting watermarked twice (unless I've mis-read your code). So, I suggest we keep this all encapsulated in TileManager.cc. Also your repeat variable (which is a kind of virtual tile size, right?) will end up making watermarked TIFF and JPEG2000 behave differently, so maybe to keep things simple this variable should just be the native tile size of the underlying image as with TIFF (Yes, in reality this tile size is hard-wired to 256 for JPEG2000, but maybe this should be modifiable via an environment parameter). |
|
Hi @ruven, thanks for looking at this. Aha, I'm sorry I didn't realise that CVTs from TIFF get the watermark anyway. (I happen to be working with JP2s on this project, which is the first time I've needed the watermark functionality.) I wasn't very happy with the repeat variable, intended as you suggest to imitate the tile size, so totally agree should use the native tile size - where would I access that? (My aim in this instance is for a CVT to look the same as the image seen in a tiled viewer, i.e. an approximately similar number of watermarks.)
Would this be in TileManager::getRegion ? |
Yes, you can see here that it can bypass the usual tile-based compositing, so we should do the watermarking just after: Line 309 in 882925b You can also see how to get the native tile dimensions just below this: Line 315 in 882925b Having said all that, it may well be better and more flexible to move watermarking completely out of TileManager and up into CVT.cc and JTL.cc, so that it happens after rather than before any image processing. If you ask, for example, for rotation, the watermark will also be rotated, which is arguably not what you would expect. Any thoughts? |
|
Yes I originally put it in CVT.cc after rotation; and when I read your note yesterday and took another look at the code, I realised that an issue with watermarking in the TileManager was that the watermark would be applied before rotation. I think that's probably not what would normally be desirable; but I also tested with tiles and realised that it's what happens now in that instance! On the basis that my minimal intervention is to get the same result, in relation to watermarks, from a CVT request as seen in a tiled viewer, I suppose having the watermark rotate with the image would at least be consistent. But if prepared to make more changes, I would think it would be better to apply watermark consistently after rotation, in all cases. By the way while, checking rotated tiles to see what happened with watermarks, I noticed a more serious issue. It seems that requesting the same region, as 256x256 tile starting at 256 pixel boundaries, rotated 0, 90, or 270 degrees returns the same image, rotated appropriately (including a rotated watermark if that's happening); but rotated 180 degrees returns a different selection. See e.g. |
This is in fact due to a hack in JTL.cc, which I made to allow IIP tile viewers to be able to pan and zoom an image using JTL requests without having to re-calculate the rotated tile indices. You just add the rotation parameter to the URL and the viewer will work perfectly without having to know that the image is rotated. However, if you add 1 pixel to the IIIF region size, you get the correct view as it no longer uses JTL: https://merovingio.c2rmf.cnrs.fr/fcgi-bin/iipsrv-watermark.fcgi?IIIF=PIA03883.pyr.tif/5376,5121,256,256/full/180/default.jpg I'll look into disactivating this hack for IIIF requests.
Yes, having thought about it more, I agree that watermarking would be better applied at the end of the normal image processing chain. |
|
Hi Ruven. Thanks for your comments. Given the rotation issue, I propose to leave the code where it is; but I've taken your advice and removed the "WATERMARK_REPEAT" parameter, using |
ruven
force-pushed
the
master
branch
4 times, most recently
from
75d1f95
to
655cedb
Compare
ruven
force-pushed
the
master
branch
4 times, most recently
from
36062f2
to
750d081
Compare
| Watermark* watermark = session->watermark; | ||
| if( watermark && watermark->isSet() && (watermark->getMinCVT() > 0)){ | ||
|
|
||
| if( (complete_image.width >= watermark->getMinCVT()) && (complete_image.height >= watermark->getMinCVT())){ |
There was a problem hiding this comment.
What if complete_image.height will not be provided? So the client wants to fetch image only be specifying width of the image but still wants to see watermark.
ruven
force-pushed
the
master
branch
3 times, most recently
from
cd07a85
to
da5b2de
Compare
(See issue #218) This PR adds two new environment variables WATERMARK_MIN_CVT and WATERMARK_REPEAT. If either are missing or zero, behavious is unchanged. However, if both are non-zero, and if the other Watermarking variables are set and valid, then watermarks will be applied to CVT/IIIF images in approximately the same manner as they are applied to tiles.
WATERMARK_MIN_CVT sets a threshold below which no watermarking will take place - this allows smaller images, such as thumbnails or small previews, not to be watermarked. If the threshold is met, then the complete image is treated as if it was a series of tiles, and the watermark is applied to each tile in the same way that it would be to an individual tile, i.e. using the WATERMARK_PROBABILITY to decide whether to apply a watermark.
WATERMARK_REPEAT sets the size of the conceptual tiles. This could be a constant, but I couldn't see that there was an obvious constant to use - I may have missed it. Arguably it gives the implementer more control to have this as a separate value however.
(If you have any questions or comments please contact me at [email protected] - many thanks,)