gst_tag_list_add_value_internal() behaviour for tags with no merge function
Submitted by LRN
Link to original bug (#683010)
Description
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:
- It makes handling of a tag dependent on the merge mode, and not on tag's contents.
- It is completely undocumented.
Proposed fix:
- 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.
- Also document this (unchanged, when merge function is NULL) behaviour (for users who register their own tags).