GNOME Bugzilla – Bug 623799
tagsetter: Configurable metadata output
Last modified: 2013-07-24 11:15:58 UTC
XMP spec aggregates multiple tag systems/groups, called schemas. Examples: exif, iptc, photoshop, dublin core... One use case that isn't covered is to allow applications to select which ones should be used when serializing a taglist to the streams. This requires working at 2 levels. 1) Implement a configuration struct/parameter for the xmp lib in -base/gst-libs/tag, allowing elements to inform which schemas should be used. 2) Implement a way for elements to expose this API to applications. I haven't started working on this (yet), but I thought it would be nice to start discussing here so I can start on the correct direction. The xmp schemas are represented by strings, here are some: * dc * xap * photoshop * tiff * iptc Check http://www.adobe.com/devnet/xmp/pdfs/XMPSpecificationPart2.pdf for more information. So I was thinking to have some API to blacklist/whitelist what schemas to use, this is simple (from the API point of view) of adding to the taglib, but how should we expose it from elements? As I don't know any other specs similar to XMP, I think an specific interface would be nice for configuring it, rather than going the generic way. Do we have any other options here? We could also create a GstTagSetter2, but I really can't think of a nice API that would be able to handle all this generically. Maybe someone out there has a better idea?
Aren't you going to only use one schema per format using xmp ? i.e. For jpeg/tiff files you'd only use one format, no ? If so, one would only need to modify the xmp helper functions to specify the xmp format one want depending on who's calling it (i.e. qtmux would use a different format than another container format).
Thiago, the api could also be used to select between different id3-tag schemas (not sure if that is a good idea, but I think people have asked for it, but we don't have an api for it). It seems we can't add vmethods to GstTagSetterIFace as it never ever had padding :/. Not sure if we can't just add it, as I am sure nobody subclassed that iface yet. Anyone against that? Edward, the schemas are kind of namesspaces of metadata owned by different authorities. It totally makes sense to use multiple of them at the same time.
(In reply to comment #1) > Aren't you going to only use one schema per format using xmp ? > > i.e. For jpeg/tiff files you'd only use one format, no ? > > If so, one would only need to modify the xmp helper functions to specify the > xmp format one want depending on who's calling it (i.e. qtmux would use a > different format than another container format). Imo qtmux-like elements are differentiating only the atoms (or boxes) written to the file so that the same metadata informations (e.g. location infos) can be stored in different ways for different formats. I like the idea to use the same approach for this feature, but it would work without too many messes in the code only if we there aren't too many overlapping tags between the different specialized "formats" (e.g. like for 3gp and mp4).
(In reply to comment #3) > (In reply to comment #1) > > Aren't you going to only use one schema per format using xmp ? > > > > i.e. For jpeg/tiff files you'd only use one format, no ? > > > > If so, one would only need to modify the xmp helper functions to specify the > > xmp format one want depending on who's calling it (i.e. qtmux would use a > > different format than another container format). > > Imo qtmux-like elements are differentiating only the atoms (or boxes) written > to the file so that the same metadata informations (e.g. location infos) can be > stored in different ways for different formats. Exactly, this happens because the specs dictates the 'native' formats for them, but there is no relation between these specs to xmp schemas. (at least, not that I know of) > > I like the idea to use the same approach for this feature, but it would work > without too many messes in the code only if we there aren't too many > overlapping tags between the different specialized "formats" (e.g. like for 3gp > and mp4). Either I don't get how exactly this would work, or this makes no sense IMO. Hardcoding schemas to muxers is not a good solution, this schema selection is application specific IMHO and should be handled as such. My idea is that applications would call something like this: gst_xmp_serializer_configure (my_element_that_writes_xmp, GST_TAG_XMP_ASSOCIATE_FIRST/ALL, "dc", "xap", "iptc", ..., NULL); GST_TAG_XMP_ASSOCIATE_FIRST/ALL would make the tag association from a GST_TAG_* to a xmp one stop at the first mapping found or to map to all different schemas that contain that mapping. The list of strings would define what schemas are to be used and their order of priority.
(In reply to comment #4) > > My idea is that applications would call something like this: > > gst_xmp_serializer_configure (my_element_that_writes_xmp, > GST_TAG_XMP_ASSOCIATE_FIRST/ALL, > "dc", "xap", "iptc", ..., NULL); > > GST_TAG_XMP_ASSOCIATE_FIRST/ALL would make the tag association from a GST_TAG_* > to a xmp one stop at the first mapping found or to map to all different schemas > that contain that mapping. > > The list of strings would define what schemas are to be used and their order of > priority. This is the best approach in case of many possible combinations. Duplications for the same XMP tag in more than one schema would be another good reason for this solution, so I guess we're actually in such a case. A confirmation will make me silent on this bug, at least for a while :) .
I just started implementing this. Starting by the library and adding configuration parameters in new functions for serialization. Then I should start the GstXmpConfigurationInterface, do we have a better option/idea to solve this for the elements?
Thiago, as I said in comment #2, I wonder if we could keep more generic. We could have an iface with methods to query what variants of the metadata standard the element supports and then select a combination of them. Then people can select which metadata formats they want at all. Like in mp4mux, people could select between native metadata, xmp and id3. That would need some recursive mechanism to handle the 'subformats'=schemas for xmp and 'subformats'=id3-versions for id3 (just one of them). This can be solved using a hierarchic structure for query and selection. Not sure if this is getting too complex ...
> It seems we can't add vmethods to GstTagSetterIFace as it never ever had > padding :/. Not sure if we can't just add it, as I am sure nobody subclassed > that iface yet. That's not a problem. Interfaces don't need padding, we can add additional vmethods without breaking ABI or API. (This is not to say that I think it's a good idea to add this to the existing GstTagSetter interface, just that it's possible.)
If we were to create a generic interface and add it to GstTagSetter, I can think of the following methods: * const gchar** _get_supported_formats () - returns the list of supported formats: e.g. native, xmp, exif, id3... * const gchar** _get_supported_sub_formats (major-format) - returns the list of subformats supported for the specific major-format * void _set_enabled_formats (hashtable of major-format to subformat list) * hashtable _get_enabled_formats () This looks like it would work, don't know how worth would be doing that and if there is a good generic way of handling subformats.
Exposing hash tables directly in the API is not very nice, and also not very bindings/introspection friendly.
Thiago, I was suggesting a GstStructure. Do you see any problem with that?
Created attachment 171868 [details] [review] tag: Adds GstTagFormatConfig Adds structure to represent a configuration of tag formats to be serialized by a tagsetter API: gst_tag_formats_config_new API: gst_tag_formats_config_free API: gst_tag_formats_config_add_format API: gst_tag_formats_config_get_nth_format API: gst_tag_formats_config_get_size
Created attachment 171869 [details] [review] tagsetter: Adds new API for configuring metadata formats Adds new functions for querying and configuring metadata formats to be serialized. This allows application to select which metadata formats (exif/xmp/native/id3) should be written to the streams. The configuration is done through a GstStructure and allow selecting 'subformats' as well. For example, xmp has lots of different schemes, each scheme is considered a subformat. id3 versions are also subformats of id3. API: gst_tag_setter_get_supported_formats API: gst_tag_setter_get_config API: gst_tag_setter_set_config Fixes #623799
Created attachment 171870 [details] [review] tagsetter: Adds a virtual function for supported formats This is not correct as it breaks ABI, can someone think of an alternative? Those 3 patches add the basis for configuring metadata formats, once we agree this is on the right track I'll implement it on qtmux and jifmux as test cases to see what we could improve or helper functions to provide.
Created attachment 172772 [details] [review] tests: Remove GstTagSetter from ABI checks Interfaces can have new members added without breaking ABI, so remove it from the check.
Created attachment 172773 [details] [review] tag: Adds GstTagFormatConfig Adds structure to represent a configuration of tag formats to be serialized by a tagsetter API: gst_tag_formats_config_new API: gst_tag_formats_config_free API: gst_tag_formats_config_add_format API: gst_tag_formats_config_get_nth_format API: gst_tag_formats_config_get_size
Created attachment 172774 [details] [review] tagsetter: Adds new API for configuring metadata formats Adds new functions for querying and configuring metadata formats to be serialized. This allows application to select which metadata formats (exif/xmp/native/id3) should be written to the streams. The configuration is done through a GstStructure and allow selecting 'subformats' as well. For example, xmp has lots of different schemes, each scheme is considered a subformat. id3 versions are also subformats of id3. API: gst_tag_setter_get_supported_formats API: gst_tag_setter_get_config API: gst_tag_setter_set_config Fixes #623799
Created attachment 172775 [details] [review] xmp: Add new function that has a configuration struct Adds a new function that accepts a GstStructure as a configuration for the xmp serialization. This GstStructure is the same used in the GstTagSetter configuration. It is now possible to specify which schemas should be used in xmp. API: gst_tag_list_to_xmp_buffer_full
Created attachment 172776 [details] [review] examples: Adds a jifmux-tagsetter example Adds an example showing the new configuration API for GstTagSetter using jifmux
Created attachment 172777 [details] [review] jifmux: Implement the new GstTagSetter configuration bits Use GstTagSetter configuration API to select which metadata formats are put into resulting jpegs
Created attachment 172778 [details] [review] qtmux: Implement new GstTagSetter bits Implement new tagsetter interface methods to allow configuration of the metadata formats to be used
Moving to core as the most important bits for this are there. 2 open points in these patches: * Should we add a get mapped subformats to our xmp lib to provide a more complete supported formats description? * As we're adding a gst_tag_list_to_xmp_buffer_full I wonder if that should also include the GstCaps for the format as I've seen some exif tags that are for data that is usually on caps on gstreamer. The same might happen for xmp as well.
Anyone got time to review this? :)
Comment on attachment 172772 [details] [review] tests: Remove GstTagSetter from ABI checks looks good to me
I can't help but feel this is a bit overdesigned. And the API doesn't look very nice to use tbh, judging by the example in the GstTagFormatsConfig docs. Also, when it comes to subformats, there seems to be two types, which are mixed up here API-wise if I'm not mistaken: mutually-exclusive subformats (pick one) like ID3 versions, and options (pick any) like XMP namespaces. I guess we could just assume apps know what to do for each format, but it still doesn't feel right to me. I'll see if I have ideas for an alternative API.
Comment on attachment 172773 [details] [review] tag: Adds GstTagFormatConfig I also think we better hide the fact that we use a structure.
Review of attachment 172775 [details] [review]: ::: gst-libs/gst/tag/gstxmptag.c @@ +1593,1 @@ what about keeping the local data var abd just assigning it here? write_data.data = data; then you have less changes below and not pointer deref.
There's now a GstTagXmpWriter interface which does exactly what it's supposed to. I don't really see any major use cases remaining, nor do I have better ideas how to approach this with generic API, so I think we should just close this bug or rename it back to 'configurable xmp output', unless someone wants to pursue the generic approach further.
(In reply to comment #28) > There's now a GstTagXmpWriter interface which does exactly what it's supposed > to. I don't really see any major use cases remaining, nor do I have better > ideas how to approach this with generic API, so I think we should just close > this bug or rename it back to 'configurable xmp output', unless someone wants > to pursue the generic approach further. I just now saw that plan b was done in bug #645167. Whats is the issue with a more generic API? Elements that implement tagsetter would need two extra methods: GList *gst_tag_setter_get_variants(GstTagSetter *ts) if return list is NULL -> no variants gst_tag_setter_set_variants(GstTagSetter *ts, GList *variants) set the variants to use, all unknown variants are ignored. one should really use _get_variants(), filter and call _set_variants() Each variant would be a group in the sense of a pick one or pick any. The group would be a list of string and description. Basically what thiago did, but hiding the structure from the api if that is desired. I really thing the XmpConfig is not something we want to have in a release.
> Whats is the issue with a more generic API? See comment #25: I think the current proposal is overdesigned, convoluted, and confused. And I think other than XMP the use cases mentioned a re rather unconvincing and can easily be handled differently in better ways. > I really thin[k] the XmpConfig is not something we want to have in a release. I think it's fine. It does exactly what it's supposed to do with straight-forward API. It solves a real problem that can't easily be solved better in another way. It's not that I wouldn't prefer a generic API, I just can't think of such an API that makes sense to me and doesn't have the issues mentioned above.
I think you're both right ;) GstXmpConfig is only solving part of the problem and a more generic GstTagSetter configuration system would be better. But OTOH it seems to be complicated to define a nice interface for this and the need for GstXmpConfig is there *now*. We can still remove GstXmpConfig from 0.11 and replace it with something more generic, 0.11 will be there soon.
Comment on attachment 172772 [details] [review] tests: Remove GstTagSetter from ABI checks commit 6da1778dad88777b19265a257c1121af4917e230 Author: Thiago Santos <thiago.sousa.santos@collabora.co.uk> Date: Fri Oct 15 13:16:59 2010 -0300 tests: Remove GstTagSetter from ABI checks Interfaces can have new members added without breaking ABI, so remove it from the check. https://bugzilla.gnome.org/show_bug.cgi?id=623799
What's the plan here now? Any interest still or just forget it?
I refer to my comment #25 and comment #28 :)
Let's close it then