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 347091 - converting vorbis comments to GstTagLists is lossy
converting vorbis comments to GstTagLists is lossy
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.10
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 350935 351426
Blocks:
 
 
Reported: 2006-07-10 13:36 UTC by James "Doc" Livingston
Modified: 2006-08-24 10:06 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
possible patch (2.91 KB, patch)
2006-07-10 13:45 UTC, James "Doc" Livingston
none Details | Review
Addendum to previous patch (2.62 KB, patch)
2006-07-12 12:42 UTC, Edward Hervey
rejected Details | Review

Description James "Doc" Livingston 2006-07-10 13:36:54 UTC
Similar to bug 334375 (which is for ID3 tags) converting vorbis comments to a GstTagList is lossy, because GStreamer doesn't know about all the things that vorbis does, and can't because they can contain arbitrary comments.

An approach similar to the "forward binary blobs" idea for ID3 would work for vorbis comments, except it would be much simplet as vorbis comments are always text of the form "key=value".
Comment 1 James "Doc" Livingston 2006-07-10 13:45:27 UTC
Created attachment 68715 [details] [review]
possible patch

This is a possible patch to do this (it works for me).

It works by registering a new tag GST_TAG_VORBIS_UNKNOWN which is used when GStreamer encounters an unknown tag, with the tag data set to be the entire "key=value" string. I haven't exported it because it's only used inside the gst_vorbis_tag_add and gst_tag_to_vorbis_comments function (and indirectly by gst_tag_list_{to,from}_vorbiscomment_buffer).

If this looks like an acceptable approach, I'll tidy it up. I'm not sure whether  the gst_tag_to_vorbis_tag and gst_tag_from_vorbis_tag should, or even could correctly, deal with the _UNKNOWN tag. If it's thought applications should be able to use this to write arbitrary vorbis comments, then the tag could be made public.
Comment 2 Edward Hervey 2006-07-12 12:42:46 UTC
Created attachment 68809 [details] [review]
Addendum to previous patch

The previous patch breaks make check.

This update to the patch makes it emit tags on the bus as messages even though the decoder isn't initialized.
Comment 3 Edward Hervey 2006-07-12 12:43:19 UTC
Sorry, wrong bug.
Comment 4 Alex Lancaster 2006-08-12 09:57:20 UTC
I guess if bug #350935 is implemented, the GST_TAG_VORBIS_UNKNOWN in this patch would be replaced by GST_TAG_EXTENDED_COMMENT?
Comment 5 Tim-Philipp Müller 2006-08-12 10:05:11 UTC
> I guess if bug #350935 is implemented, the GST_TAG_VORBIS_UNKNOWN in this patch
> would be replaced by GST_TAG_EXTENDED_COMMENT?

That was the idea, yes.


Comment 6 Tim-Philipp Müller 2006-08-17 16:59:39 UTC
 2006-08-17  Tim-Philipp Müller  <tim at centricular dot net>

        * gst-libs/gst/tag/gstvorbistag.c: (gst_vorbis_tag_add),
        (gst_tag_to_vorbis_comments):
          Serialise unknown vorbis comments into GST_TAG_EXTENDED_COMMENT
          tags and deserialise them properly as well (#351768).
          Add some more gtk-doc blurbs and also some g_return_if_fail().

        * tests/check/libs/tag.c: (GST_START_TEST),
        (back_to_vorbis_comments), (taglists_are_equal), (tag_suite):
          More tests.


Still need to check implementations of vorbisdec/vorbisenc, speexdec/speexenc, theoraenc/theoradec, etc. to make sure they handle extended comments properly if they are not using the do-everything utility functions. Hence, keeping bug open for now.
Comment 7 Alex Lancaster 2006-08-18 18:17:45 UTC
(In reply to comment #6)
>  2006-08-17  Tim-Philipp Müller  <tim at centricular dot net>
> 
>         * gst-libs/gst/tag/gstvorbistag.c: (gst_vorbis_tag_add),
>         (gst_tag_to_vorbis_comments):
>           Serialise unknown vorbis comments into GST_TAG_EXTENDED_COMMENT
>           tags and deserialise them properly as well (#351768).
                                                          ^^^^^^

The ChangeLog appears to reference the wrong bug number, shouldn't it be *this* bugs number, i.e. bug #347091, not bug #351768?
                                        
Comment 8 Tim-Philipp Müller 2006-08-18 21:22:33 UTC
> The ChangeLog appears to reference the wrong bug number, shouldn't it be *this*
> bugs number, i.e. bug #347091, not bug #351768?

It should, thanks. ChangeLog entry fixed in CVS.
Comment 9 Tim-Philipp Müller 2006-08-24 10:06:34 UTC
Things should be fine now for vorbisdec/enc, speexdec/enc, flacdec/enc (with CVS at least), so this bug can be closed I think.