After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 350935 - API: add GST_TAG_EXTENDED_COMMENT
API: add GST_TAG_EXTENDED_COMMENT
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: 0.10.10
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 347091 348762
 
 
Reported: 2006-08-11 18:16 UTC by Tim-Philipp Müller
Modified: 2006-08-14 19:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed addition of GST_TAG_EXTENDED_COMMENT (1.79 KB, patch)
2006-08-11 18:18 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2006-08-11 18:16:49 UTC
I'd like to add a GST_TAG_EXTENDED_COMMENT field specifically for key/value pair type comments. This would be used to store unknown vorbiscomment/APE tags (so retagging is possible without losing information), and to store certain type of ID3v2 comment frames.

We could theoretically use GST_TAG_COMMENT for those as well, but it would be possible to accidentally munge information that way when re-tagging, hence the new tag.
Comment 1 Tim-Philipp Müller 2006-08-11 18:18:34 UTC
Created attachment 70735 [details] [review]
proposed addition of GST_TAG_EXTENDED_COMMENT

(sneakily changes GST_TAG_COMMENT's merge function to something more sensible too)
Comment 2 Alex Lancaster 2006-08-12 09:51:59 UTC
(In reply to comment #1)

> (sneakily changes GST_TAG_COMMENT's merge function to something more sensible
> too)

What's the semantic difference between those two functions?  Does it mean that only the first comment tag is used rather than using all of them with commas separating them? 

Comment 3 Tim-Philipp Müller 2006-08-12 10:01:31 UTC
> > (sneakily changes GST_TAG_COMMENT's merge function to something more sensible
> > too)
> 
> What's the semantic difference between those two functions?  Does it mean that
> only the first comment tag is used rather than using all of them with commas
> separating them? 

The merge function determines what happens when you use gst_tag_list_get_xyz(), as opposed to gst_tag_List_get_xyz_index (..., 0), on a taglist that contains multiple tags. If you have a list with

 GST_TAG_COMMENT="This song sucks"
 GST_TAG_COMMENT="This song rocks"

then gst_tag_list_get_string (list, GST_TAG_COMMENT) will currently automatically concatenate the two comments into a combined string separated by a comma, ie. you get "This song sucks, this song rocks" returned; changing the merge function to merge_use_first will make it return only the first string and leave it to the application to look for other comments.

I think this makes more sense than merge_with_comma (since concatenated comment strings rarely look as nice as the one above), but I'm not married to it. Just came across it as I added this tag.

What do you think?
Comment 4 Alex Lancaster 2006-08-12 15:45:15 UTC
What would happen with multiple comment tags in the case of retagging a file that has multiple comments?  We would want to preserve them all separately (as we do now, I think).

I'm not sure what you mean by "leaving it to the application"?  Can you give an example of how this would in (say) rhythmbox?
Comment 5 Alex Lancaster 2006-08-12 15:49:43 UTC
Note that retagging via rhythmbox (as opposed to a gst pipeline), already seems to have the problem that only the first comment tag is kept (bug #348761) and I was wondering if it was related to this problem.
Comment 6 Tim-Philipp Müller 2006-08-13 19:18:03 UTC
> What would happen with multiple comment tags in the case of retagging a file
> that has multiple comments?  We would want to preserve them all separately (as
> we do now, I think).

That change wouldn't really have any effect on retagging (unless the tag writing element is buggy).


> I'm not sure what you mean by "leaving it to the application"?  Can you give an
> example of how this would in (say) rhythmbox?

Applications who want to get _all_ comments would need to do

  for (i = 0; i < gst_tag_list_get_tag_size (taglist, GST_TAG_COMMENT); ++i) {
    gchar *nth_comment;

    nth_comment = gst_tag_list_get_string_index (taglist, GST_TAG_COMMENT, i, &nth_comment);
  }

since

  gst_tag_list_get_string (taglist, GST_TAG_COMMENT, &str);

would now be equivalent to

  gst_tag_list_get_string (taglist, GST_TAG_COMMENT, 0, &str);


I don't know where the problem is with that rhythmbox retagging bug, but feel free to point me or others to some code on IRC so we can take a look. It's either an extraction problem (not doing the above), or a tag merging problem. The id3v2mux log should tell you. Feel free to CC me on the bug if you want me to comment on anything related there.



Comment 7 Tim-Philipp Müller 2006-08-14 19:05:53 UTC
Committed (OK-ed by Wim on IRC):

 2006-08-14  Tim-Philipp Müller  <tim at centricular dot net>

       * docs/gst/gstreamer-sections.txt:
       * gst/gsttaglist.c: (_gst_tag_initialize):
       * gst/gsttaglist.h:
         API: add GST_TAG_EXTENDED_COMMENT (#350935).