[Bug 683010] New: gst_tag_list_add_value_internal() behaviour for tags with no merge function

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=683010
  GStreamer | gstreamer (core) | git

           Summary: gst_tag_list_add_value_internal() behaviour for tags
                    with no merge function
    Classification: Platform
           Product: GStreamer
           Version: git
        OS/Version: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: gstreamer (core)
        AssignedTo: gstreamer-bugs at lists.freedesktop.org
        ReportedBy: lrn1986 at gmail.com
         QAContact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---


Looking at the source code of gst_tag_list_add_value_internal(), it seems to be
working like this:

If tag has a merge function and there already is a value for this tag:
GST_TAG_MERGE_REPLACE_ALL: replace
GST_TAG_MERGE_REPLACE    : replace
GST_TAG_MERGE_PREPEND    : concatenate new + old into a list of values
GST_TAG_MERGE_APPEND     : concatenate old + new into a list of values
GST_TAG_MERGE_KEEP       : do nothing
GST_TAG_MERGE_KEEP_ALL   : do nothing
Note that merge function is not actually used here (it will be used later, when
merge is required, to turn concatenated list into a single value).

HOWEVER. If there is no merge function for this tag, OR there is a merge
function, but no value for this tag in the list, it does this:
GST_TAG_MERGE_REPLACE_ALL: replace unconditionally
GST_TAG_MERGE_REPLACE    : replace unconditionally
GST_TAG_MERGE_PREPEND    : replace unconditionally!!!
GST_TAG_MERGE_APPEND     : if there already is a value for this tag - do
nothing, replace otherwise!!!
GST_TAG_MERGE_KEEP       : if there already is a value for this tag - do
nothing, replace otherwise
GST_TAG_MERGE_KEEP_ALL   : do nothing

Note that behaviour for PREPEND and APPEND is unexpected, if you consider the
case where merge function is NULL, and there is a value for this tag already.
It means that it's impossible for non-mergeable tags to make a list of values
for a tag, even if only one will be shown later, when merge is required.
And indeed, gst_tag_list_copy_value() fails when merge_func is NULL.

The reason for this seems to go like this:
If we can't keep both, when asked to append or prepend (because non-mergeable
tags can't be stored as lists of values), do imaginary append/prepend
operation, then take the head of the list (which means "keep existing item" for
append, and "take new item" for prepend).
This seems to be a good idea, however:
1) It makes handling of a tag dependent on the merge mode, and not on tag's
contents.
2) It is completely undocumented.

Proposed fix:
1) Give gst_tag_merge_use_first() merge function to types that have no merge
function, do actual appending/prepending in a normal way. The end result will
be the same (gst_tag_merge_use_first() will pick the head of the list, which
will depend on whether the list was made by appending or prepending), except
that user code that is list-aware will be able to fetch the whole list, and
values won't be lost.
2) Also document this (unchanged, when merge function is NULL) behaviour (for
users who register their own tags).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.