The problem was the implementation of handling the case where you pass chend < chbegin, which is supposed to mean "all channels."
Proposed fix here: https://github.com/OpenImageIO/oiio/pull/2742 <https://github.com/OpenImageIO/oiio/pull/2742> As a workaround, rather than call add_tile() and get_tile() with chbegin=1, chend=0, I would recommend passing chbegin=0 and chend=the correct number of channels. In addition to working around this bug, I think it's better practice. In code like this, passing the special signal for "all channels" but just assuming what that is, seems dangerous. For example, you are taking it for granted that your files have the same resolution and number of channels in each of the subimages, but that doesn't need to be the case, and code that relies on that assumption will sooner or later exhibit tricky bugs when you encounter a file for which (by design or accident) the assumptions are false. Now then, TOTALLY UNRELATED TO THIS BUG, I have a few comments regarding things I noticed about your code snippet that as a user of OIIO you may find helpful to keep in mind: > auto test_file = [](const ustring& filename){ There is never a reason to have a `const ustring&`. A ustring is 64 bits wide (same as the pointer that underlies a reference), and passing it by value does not do a deep copy (nor does it change ownership or even diddle reference counts). It just adds a needless doubly indirect access. > auto tile = ic->get_tile(filename, ss, 0, 0, 0, 0, 1, 0); Your uses of get_tile (which increases a reference count on the tile so that it's not freed while still potentially in use) don't have corresponding calls to release_tile(), so the cache will think those tiles are never safe to reclaim, and eventually the cache will fill up. > int number_of_subimages = 0; > while (ic->get_imagespec (filename, spec, number_of_subimages)){ > number_of_subimages++; > } Repeatedly calling get_imagespec until it fails is a fairly inefficient way to determine the number of subimages (you end up needlessly copying the spec of each subimage). You could instead simply query it directly: int nsubimages = 0; ic->get_image_info(filename, 0, 0, ustring("subimages"), OIIO::TypeInt, &nsubimages); Also, it is worth noting that the `while` loop that you wrote will end up with `spec` containing the ImageSpec of the *last* subimage in the file. It may be fine for this example file, but in general I don't think it's safe to assume that all subimages will have matching resolutions, numbers of channels, or anything else where in this example you assume that whatever spec.* holds is valid for all the subimages. In fact, what I find is that for many real-world multi-image files, quite often the useful metadata like DateTime is diligently set for the FIRST subimage only, and omitted from the subsequent ones. So if you're going to assume that one subimage is representative of the whole stack, it's probably safer to use subimage 0 (rather than nsubimages-1) as the template. Hope this helps, -- lg > On Oct 11, 2020, at 9:12 PM, Arman Garakani <arman.garak...@darisa.net> wrote: > > I am adding unit test as part of adding OpenImageIO to our app ( Machine > Learning / Computer Vision / Biological Assays ). Before going further, oiio > solves a fundamental problem in developing media accessing functionality. I > have a very directed functionality for videos. Oiio has more functionality, > simpler API and supports much wider image format APIs. > > Here is the problem I am having with small tif stack files I use for testing. > <zser16.tif><zser8.tif> > > > Here is what iinfo says about it: > > $ iinfo zser8.tif > zser8.tif : 160 x 128, 1 channel, uint8 tiff (11 subimages) > > OpenImageIO ImageCache statistics (shared) ver 2.1.18 > Options: max_memory_MB=2048.0 max_open_files=100 autotile=0 > autoscanline=0 automip=0 forcefloat=0 accept_untiled=1 > accept_unmipped=1 deduplicate=1 unassociatedalpha=0 > failure_retries=0 > > In the function below, the output before the loop calling add_tile, outputs: > 11 (160,128) 255::-1::-1::NA::8 > > > call to add_tile generates a memory access fault. The interesting thing here > is that similar file but containing 16bit data works fine. > > > > > This is a simple test I am developing to assess hit and miss performance > with ImageCache. The test function is a lambda here for simplicity. > > > > > > TEST(oiio, basic){ > > auto test_file = [](const ustring& filename){ > // Create a private ImageCache so we can customize its cache size > // and instruct it store everything internally as floats. > ImageCache* ic = ImageCache::create(true); > ic->attribute("autotile", 0); > ic->attribute("max_memory_MB", 2048.0); > > > ImageSpec spec; > int number_of_subimages = 0; > while (ic->get_imagespec (filename, spec, number_of_subimages)){ > number_of_subimages++; > } > unsigned int maxval = (unsigned int)get_intsample_maxval(spec); > int xres = spec.width; > int yres = spec.height; > int channels = spec.nchannels; > int i = spec.get_int_attribute ("oiio:subimages", -1.0); > int bp = spec.get_int_attribute ("oiio:BitsPerSample", -1); > float fps = spec.get_float_attribute ("Fps", -1.0f); > std::string s = spec.get_string_attribute ("DateTime", "NA"); > > std::vector<unsigned char> pixels (xres*yres*channels); > std::cout << number_of_subimages << " (" << xres << "," << yres << ") " << > maxval << "::" << i << "::" << fps << "::" << s << "::" << bp << > std::endl; > > for (int ss = 0; ss < number_of_subimages; ss++){ > if (ss & 1) > ic->add_tile(filename, ss, 0, 0, 0, 0, 1, 0, TypeDesc::UINT8, > pixels.data()); > } > > { > std::cout << " Half Cached " << std::endl; > // average hit time and miss time > float hits(0.0), miss(0.0); > int hitn(0), missn(0); > for (int ss = 0; ss < number_of_subimages; ss++){ > if (ss & 1){ > OIIO::Timer hitimer; > auto tile = ic->get_tile(filename, ss, 0, 0, 0, 0, 1, 0); > assert(tile != nullptr); > hits += hitimer(); > hitn++; > } > else{ > OIIO::Timer misstimer; > ic->add_tile(filename, ss, 0, 0, 0, 0, 1, 0, TypeDesc::UINT8, > pixels.data()); > miss += misstimer(); > missn++; > } > } > hits /= hitn; > miss /= missn; > std::cout << hitn << "," << missn << std::endl; > > std::cout << hits * 1000 << " , " << miss * 1000 << std::endl; > } > > std::cout << " All Cached " << std::endl; > > float hits(0.0), miss(0.0); > int hitn(0), missn(0); > for (int ss = 0; ss < number_of_subimages; ss++){ > OIIO::Timer hitimer; > auto tile = ic->get_tile(filename, ss, 0, 0, 0, 0, 1, 0); > if (tile != nullptr){ > hits += hitimer(); > hitn++; > } > else{ > miss += hitimer(); > missn++; > } > } > hits /= hitn; > if (missn) miss /= missn; > std::cout << hitn << "," << missn << std::endl; > std::cout << hits * 1000 << " , " << miss * 1000 << std::endl; > std::cout << ic->getstats() << std::endl; > > }; > > > auto res = dgenv_ptr->asset_path("zser8.tif"); > EXPECT_TRUE(res.second); > EXPECT_TRUE(boost::filesystem::exists(res.first)); > ustring filename (res.first.c_str()); > test_file(filename); > > } > > > _______________________________________________ > Oiio-dev mailing list > Oiio-dev@lists.openimageio.org > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org -- Larry Gritz l...@larrygritz.com
_______________________________________________ Oiio-dev mailing list Oiio-dev@lists.openimageio.org http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
