I vote for #2 with a small addition that provides some of the functionality
of #1. In my branch I've added a static function to ImageSpec that replaces
ImageSpec::metadata_val:
/// For a given parameter (in this ImageSpec's extra_attribs),
/// format the value nicely as a string. If 'human' is true, use
/// especially human-readable explanations (units, or decoding of
/// values) for certain known metadata.
static std::string attribute_value_string (const ImageIOParameter &p,
bool human=false);
/// DEPRECATED - Now a static
std::string metadata_val (const ImageIOParameter &p,
bool human=false) const {
return attribute_value_string(p, human);
}
This would allow us to store the timecode as an array of uint64 and provide
a way to get that same value as a string. Since OIIO deals with images, and
not video, I don't think there's justification for #3.
I'd also point out that TypeDesc and ParamValue have an even older legacy
than OIIO (in a number of proprietary tools), and their similarity to plain
C-types is extremely valuable. Extending this mechanism would have a large
impact at a very deep level to some code bases that may use these types
internally, in other pieces of code beside OIIO strictly. So it doesn't
just break compatibility with OIIO, changes to these low-level types could
have a much larger impact on code that really wants nothing to do with
timecodes.
On Thu, Oct 10, 2013 at 10:18 AM, Larry Gritz <[email protected]> wrote:
> As I see it, we have basically three choices.
>
> 1. Encode as a string, establishing a convention for how it should be
> formatted (and maybe how it must be named to be recognized specifically as
> an SMPTE timecode).
>
> 2. Encode as some other type we already support (a uint64, or array of
> ints, say, if there is already an accepted convention for how SMPTE
> timecodes should be represented using ints or bitfields). Again, some
> convention in the metadata name might disambiguate which metadata are to be
> interpreted this way, versus truly being an int[2], for example.
>
> 3. Make an entirely new type, which is obviously better suited for that
> type, but which would necessitate a break in API and link compatibility,
> because it would involve changing the definitions of TypeDesc and ParamList
> and every place in the code (and application code!) that deals with those
> data structures and think that they currently cover all the possibilities.
>
> Note that #1 and #2 would be API forward and back compatible and only
> impact the couple of format readers/writers that deal with SMPTE timecodes,
> would not alter OIIO internals or other plugins. There is precedent for
> this approach for other formats/data items, and it generalizes to other
> similar issues that might come up in the future. Once we open the door on
> #3, will we have a proliferation of new types (each one requiring a change
> to multiple classes, internals, and apps, and representing a compatibility
> break) as additional purposes dribble in from time to time?
>
> I'm not saying no... I just want to get all the pros and cons on the
> table. But that's why #3 is, in my mind, a choice of last resort. I prefer
> standard types and conventions to growing a large "type zoo." The fact
> that TypeDesc and ParamList have been almost completely unchanged for 5
> years is a strength and speaks to it being a pretty good fit for its
> intended purpose.
>
> Wait, there's one other possibility to throw out there, a bit of a
> compromise between #2 and #3:
>
> 4. We could transform the 'reserved' field in TypeDesc to some kind of
> "special meaning" code -- or just make another tag for the existing
> 'vecsemantics' byte -- and let "this is an SMPTE timecode" be one of those
> values. So it could store the data as a uint32[2], but have the semantic
> code set to indicate that it should be interpreted as an SMPTE timecode.
> This (versus truly making a new core 'basetype') changes very little of
> the internals, is not a break in link compatibility, and any apps or
> plugins that don't care specifically about timecodes can ignore it (or, at
> least, just continue to treat it as the int[2] that it's stored as).
>
>
>
>
> On Oct 10, 2013, at 9:13 AM, Mark Boorer wrote:
>
> > I guess I also disagree that timecode should survive a transfer into a
> > format that doesn't support it. To me, OIIO is not a metadata
> > pipeline, but just a way to get the maximum amount of information from
> > as many image formats in a standardised way. If metadata goes
> > EXR:TimeCode -> JPEG:string -> EXR I would expect the attribute to
> > arrive as a string.
> >
> > If we are making the call that certain metadata types will be
> > serialised in a certain way, and reconstructed at the other end, that
> > opens another whole kettle of fish. What if I want my string unmangled
> > for instance? Not converting metadata types is the least surprising
> > behaviour.
> >
> > On Thu, Oct 10, 2013 at 4:55 PM, Mark Boorer <[email protected]>
> wrote:
> >> But it isn't just SMPTE timecodes. there are many different rich types
> >> exposed in EXR metadata, some of which overlap with each other in
> >> terms of POD types, but you would want their types to be maintained.
> >>
> >> I agree that TypeDesc probably shouldn't describe these rich types, so
> >> perhaps the ParamValue shouldn't take a TypeDesc and maybe needs a
> >> more general AttributeDesc?
> >>
> >> I have some exr's with metadata like this:
> >>
> >> End (type string): "21:38:14:09"
> >> Start (type string): "21:37:36:03"
> >> timeCode (type timecode):
> >> time 21:37:36:03
> >> drop frame 0, color frame 0, field/phase 0
> >> bgf0 0, bgf1 0, bgf2 0
> >> user data 0x0
> >>
> >> When writing this data back out, I would expect that the "End" and
> >> "Start" attributes remain as strings, whereas the "timeCode" attribute
> >> is written as a TimeCode instance.
> >>
> >> The readers and writers can convert from these richer types to strings
> >> or ints or whatever we decide on a case by case basis, as long as they
> >> are consistent for their filetype, but I don't feel as though richer
> >> formats should lose out on this functionality.
> >>
> >>
> >>
> >> On Thu, Oct 10, 2013 at 4:45 PM, Larry Gritz <[email protected]> wrote:
> >>> I'm leaning toward representing it (as it travels through the OIIO
> interface) as a string, where we can pick the representation/convention for
> encoding it to preserve all the original information. There's an example
> (which may or may not be correct and/or complete) in how we handle DPX, but
> I'm not at all averse to changing it if we now have a better understanding
> of what we want.
> >>>
> >>> But I'm not an expert on SMPTE timecodes and how they're used, so if
> there's a good argument for keeping it as a 64 bit int or two 32 bit ints
> -- for example, if there is an established convention for storing them this
> way that SMPTE experts expect -- we can probably deal with that, too.
> Maybe the metadata name is required to have some kind of hint (e.g., needs
> the substring 'SMPTETime' in it somewhere). Maybe we need to provide
> utility functions to split the bit fields into a more self-documenting
> struct, and back. But I do know that I would prefer not to extend TypeDesc
> (and our metadata passing in the ParamList) to take on the complexity of
> being able to describe arbitrary structs, or even adding tags for one-off
> special purpose structs. It's so nicely simple and general now.
> >>>
> >>> One of the best arguments I can think of for preferring strings is as
> follows. If you were somehow making a round trip like this
> >>>
> >>> format A -> format B -> format A
> >>>
> >>> Now, suppose A supports SMPTE timecodes, but B does not (but does
> support string metadata). Merely by adopting the string<->timecode
> convention within A's format reader/writer, the timecode would make it
> through the above pipeline intact. B will blindly pass the string along,
> and then the A writer will recognize it as a timecode and deal with that
> appropriately. The alternative is that *every* format reader would need to
> recognize SMPTE timecodes (even if that is not a concept native to that
> file format) and have to deal with it in some way. Also, the string
> approach lets a program like 'iinfo' just print the string and be on its
> merry way, without needing to be cluttered with timecode-specific logic.
> >>>
> >>> -- lg
> >>>
> >>>
> >>> On Oct 9, 2013, at 11:31 AM, Pete Black wrote:
> >>>
> >>>> Hi All,
> >>>>
> >>>> First, the below is my understanding of SMPTE timecode, it may not be
> 100% complete, or totally correct -
> >>>>
> >>>> SMPTE timecode includes both clock data - this is binary-coded
> decimal and takes 32 bits, and user data which is another 32 bits with
> unspecified content (often a 'tape name' or similar, but also containing
> flag bits to indicate drop frame or otr details).
> >>>>
> >>>> In LTC (Longitudinal timecode, as it is encoded and written to tape
> or sent on the wire), the 4-bit nibbles which make up each binary-coded
> digit in both blocks of data are interleaved, which is why a SMPTE timecode
> is often represented by 64 bits of packed data.
> >>>>
> >>>> If you wanted to read timecode off the wire, write it into a file,
> then read the data from the file and write it back on the wire, without
> parsing it at all then you might want to ensure you had a 'flat' 64-bit
> type for this.
> >>>>
> >>>> Personally I think it makes more sense to separate clock and user
> data into 2 32 bit fields, if you want 'raw' storage of this data - using a
> string representation to get and set the clock data seems ideal, but might
> be tricky for user data, as bitwise interpretation/manipulation of this
> data ( e.g. drop frame indicator bit ) may be desirable - though this usage
> tends to be less important now we are leaving the world of analog tape, I
> know we abuse it for our purposes in 48fps workflow.
> >>>>
> >>>> There would also be some merit in dumping the 'raw' format entirely
> and just reading/writing an arbitrary string for timecode, as this would
> work for most practical purposes ( image file metadata is reference only,
> and timecode is always reconstructed and sent by some application) just
> fine.
> >>>>
> >>>> Not sure if this helps too much, I guess following what the DPX
> format does would be good for consistency.
> >>>>
> >>>> -Pete
> >>>>
> >>>>
> >>>> Mark Boorer <[email protected]> wrote:
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> Looking further into this, are we sure that timecode doesn't warrant
> a
> >>>>> new type? EXR's can contain many metadata fields in Timecode format
> >>>>> (not just one) and I don't think we can just give users 2 unsigned
> >>>>> ints and be done with it. Users will probably want their timecode in
> >>>>> string format (as is presently done in the DPX header) or at least
> >>>>> have an easy way for converting to/from strings.
> >>>>>
> >>>>> Further more, I don't think it's particularly intuitive to assume
> that
> >>>>> any TypeDesc of 2 UINT32 should be written as timecode, especially
> >>>>> where I would expect a TypeDesc of 2 INT32s to be written as an
> >>>>> ImfVecAttribute "v2i".
> >>>>>
> >>>>> Thoughts?
> >>>>>
> >>>>> Cheers,
> >>>>> Mark
> >>>>>
> >>>>> On Sun, Oct 6, 2013 at 9:27 PM, Pete Black <[email protected]>
> wrote:
> >>>>>> Hi All,
> >>>>>>
> >>>>>> DPX definitely supports timecode - and inside the DPX file, this is
> stored as a uint32 for the timecode data and a uint32 for the user data, by
> the looks
> >>>>>>
> >>>>>> Retrieving timecode from the DPX files we have using OIIO involves
> reading the metadata attribute 'dpx:Timecode' which returns the 'TV
> Industry specific' timecode formatted as a string e.g. '00:00:00:05'.
> >>>>>>
> >>>>>> -Pete
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> I believe this maps to the
> >>>>>>
> >>>>>> On 7/10/2013, at 6:58 AM, Andrew Hunter <[email protected]>
> wrote:
> >>>>>>
> >>>>>>> I believe that DPX also supports SMPTE time code as well as
> CinemaDNG.
> >>>>>>>
> >>>>>>> --
> >>>>>>> +1 416 567 9514
> >>>>>>> [email protected]
> >>>>>>> http://aehunter.net
> >>>>>>>
> >>>>>>> On 3 Oct 2013 13:14, "Larry Gritz" <[email protected]> wrote:
> >>>>>>> I would welcome a pull request with this fully fleshed out and
> tested.
> >>>>>>>
> >>>>>>> Two more things...
> >>>>>>>
> >>>>>>> 1. Just to future-proof it, I might add this in there:
> >>>>>>>
> >>>>>>> ASSERT(sizeof(Imf::TimeCode) == 2*sizeof(unsigned int));
> >>>>>>>
> >>>>>>> 2. If there are other formats that have full SMPTE timecodes, we
> might want to instead change the attribute to "SMPTE:TimeCode" (making it
> not be exr-specific) and dictate that any other formats follow the same
> encoding if they have an SMPTE timecode.
> >>>>>>>
> >>>>>>>
> >>>>>>> On Oct 3, 2013, at 10:07 AM, Larry Gritz wrote:
> >>>>>>>
> >>>>>>>> I don't think this merits a new core type.
> >>>>>>>>
> >>>>>>>> I would add it to the metadata as an array of two unsigned ints,
> which is how it's encoded in Imf::TimeCode. You'd want something like this
> in exrinput.cpp:
> >>>>>>>>
> >>>>>>>> static TypeDesc TwoUints (TypeDesc::UINT, 2);
> >>>>>>>> Imf::TimeCode tc = ...; // get the timecode from the EXR file
> >>>>>>>> spec.attribute ("openexr:TimeCode", TwoUints, &tc);
> >>>>>>>>
> >>>>>>>> And then to extract it (including in exroutput.cpp):
> >>>>>>>>
> >>>>>>>> ImageIOParameter *f = spec.find_attribute ("openexr:TimeCode",
> TwoUints);
> >>>>>>>> if (f) {
> >>>>>>>> Imf::TimeCode tc = *(const ImfTimeCode *) f->data();
> >>>>>>>> ... do your thing with tc ...
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On Oct 3, 2013, at 8:56 AM, Mark Boorer wrote:
> >>>>>>>>
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> Was just looking to access timecode metadata from some EXR
> files, and noticed that timecode keys aren't extracted by the reader.
> >>>>>>>>>
> >>>>>>>>> I then found this rather old issue
> https://github.com/OpenImageIO/oiio/issues/337. Is it possible for me to
> add this in? A simple class based on ImfTimeCode.h (or even just this class
> directly) and a new enum added to TypeDesc?
> >>>>>>>>>
> >>>>>>>>> Cheers,
> >>>>>>>>> Mark
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> Larry Gritz
> >>>>>>>> [email protected]
> >>>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>> Larry Gritz
> >>>>>>> [email protected]
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>> Oiio-dev mailing list
> >>>>>>> [email protected]
> >>>>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> >>>>>>>
> >>>>>>> _______________________________________________
> >>>>>>> Oiio-dev mailing list
> >>>>>>> [email protected]
> >>>>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> Oiio-dev mailing list
> >>>>>> [email protected]
> >>>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> >>>>> _______________________________________________
> >>>>> Oiio-dev mailing list
> >>>>> [email protected]
> >>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> >>>> _______________________________________________
> >>>> Oiio-dev mailing list
> >>>> [email protected]
> >>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> >>>
> >>> --
> >>> Larry Gritz
> >>> [email protected]
> >>>
> >>>
> >>> _______________________________________________
> >>> Oiio-dev mailing list
> >>> [email protected]
> >>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
> > _______________________________________________
> > Oiio-dev mailing list
> > [email protected]
> > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>
> --
> Larry Gritz
> [email protected]
>
>
> _______________________________________________
> Oiio-dev mailing list
> [email protected]
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>
_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
