windowscodecs: Create a IWICMetadataReader when loading a TIFF frame.

[Vincent Povirk]

>> > +        hr = IWICPersistStream_LoadEx(persist, This->parent->stream, NULL, persist_options);
>> > +        if (FAILED(hr))
>> > +            ERR("IWICPersistStream_LoadEx error %#x\n", hr);
>>
>> 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.

>> 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.

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).