windowscodecs: Create a IWICMetadataReader when loading a TIFF frame.

[Dmitry Timoshkov]

Vincent Povirk <madewokherd at gmail.com> wrote:

> >> We can't use that stream because we provide separate seek and
> >> read/write methods to libtiff, and libtiff may rely on the stream's
> >> position to not change between operations.
> >
> > libtiff doesn't rely on stream positions, and my use of stream matches
> > MSDN guidelines on writing a WIC enabled codec.
> 
> We should provide the stream interface to libtiff as documented and
> not rely on implementation details.
> 
> Even without libtiff, you will end up using the same stream for
> multiple metadata handlers, each of which could end up using it later,
> unless you specify WICPersistOptionNoCacheStream.

That's not the case of an IFD metadata reader, it creates all the items
on load, so creating multiple metadata readers from the same stream won't
hurt.

> >> Your GetReaderByIndex method should check whether
> >> This->metadata_reader has been set and return an error if not.
> >
> > No, that's not an error. TIFF always provides metadata since IFD is
> > the core of TIFF. If a metadata reader can't be created it's OK to
> > return 0 metadata items.
> 
> That's fine, but your check in GetCount suggests that having no
> metadata reader is a valid state, and GetReaderByIndex(0) will crash
> in that case. If it's not valid, you shouldn't need to test for it in
> GetCount.

That's a valid point, sorry, I misread your comment first time.

> Also, I just noticed that you're calling create_metadata_reader
> unconditionally from QueryInterface, and not checking in the process
> whether the reader has already been created. That will leak if QI is
> called multiple times.
> 
> I'm not sure doing real work inside QueryInterface is a good idea.
> It's pretty surprising.
> 
> I think it would make more sense to do this when a method on
> IWICMetadataBlockReader is first called, or in Initialize if
> WICDecodeMetadataCacheOnLoad is specified (though doing it in
> Initialize would require creating the frame objects then as well, and
> probably combining their refcounts with the parent).

MSDN suggests different ways of creating a metadata reader:
1. when creating a IWICBitmapFrameDecode
2. when creating a block reader
3. inside of GetReaderByIndex

It's all up to an implementor. I decided to that at #1. Each instance
of IWICBitmapFrameDecode has its own metadata reader, I don't see how
it could be created twice or leaked.

-- 
Dmitry.