I believe the problem lies in qtdemux_handle_xmp_taglist where having qtdemux->tag_list NULL appears to cause this problem. Presumably all this means is that the first tags we've encountered are xmp tags?

Anyway, I'm using the following patch locally and it seems to be working fine:

diff --git a/gst/isomp4/qtdemux.c b/gst/isomp4/qtdemux.c
index e9c7a07..236f853 100644
--- a/gst/isomp4/qtdemux.c
+++ b/gst/isomp4/qtdemux.c
@@ -2373,7 +2373,7 @@ qtdemux_handle_xmp_taglist (GstQTDemux * qtdemux, GstTagList * taglist,
 {
   /* Strip out bogus fields */
   if (xmptaglist) {
-    if (gst_tag_list_get_scope (taglist) == GST_TAG_SCOPE_GLOBAL) {
+    if (taglist && gst_tag_list_get_scope (taglist) == GST_TAG_SCOPE_GLOBAL) {
       gst_tag_list_remove_tag (xmptaglist, GST_TAG_VIDEO_CODEC);
       gst_tag_list_remove_tag (xmptaglist, GST_TAG_AUDIO_CODEC);
     } else {
@@ -2383,8 +2383,12 @@ qtdemux_handle_xmp_taglist (GstQTDemux * qtdemux, GstTagList * taglist,
     GST_DEBUG_OBJECT (qtdemux, "Found XMP tags %" GST_PTR_FORMAT, xmptaglist);
 
     /* prioritize native tags using _KEEP mode */
-    gst_tag_list_insert (taglist, xmptaglist, GST_TAG_MERGE_KEEP);
-    gst_tag_list_unref (xmptaglist);
+    if (taglist) {
+      gst_tag_list_insert (taglist, xmptaglist, GST_TAG_MERGE_KEEP);
+      gst_tag_list_unref (xmptaglist);
+    } else {
+      qtdemux->tag_list = xmptaglist;
+    }
   }
 }

Does that seem reasonable? Of course, I'm not totally sure what I can infer about the scope of a tag list that is empty...

Perhaps you could get the first few MBs of the file with something like:

head --bytes=2M <file> and check if it still happens? You can try different sizes. But it also might be that the file has metadata at the end.

Then you can attach that file somewhere and send us a link or if it is very small, attach it here.

Can you attach your patch in git format-patch mode? Then we can use the proper tools for merging and reviewing. Thanks!

Created attachment 327525 [details] [review]
Possible patch

Unfortunately the metadata is near the end of the file which makes things a little difficult. Nonetheless, I've attached the patch that I'm using. Thanks for the quick response!!

I think the issue is that the taglist is not being created. If you look at the code, at many places it is created, somehow we managed to fail creating it for this file. Perhaps a GST_DEBUG=qtdemux:6 would help us understand why it fails.

Anything else particularly special about this file?

Created attachment 327562 [details]
Log file with some debug

Here's a log file with some debug in it. It seems to be calling qtdemux_parse_uuid before it calls qtdemux_parse_tree, which looks to be where it normally expects to make the tag list (the final line of the log where it enters qtdemux_parse_tree was added by me).

We should write a little tool that takes an mp4/qt file, and basically zeroes out the mdat atom (or everything but the first N MB of the mdat atom) and zips it up, plus the reverse unpack :)

Created attachment 328832 [details] [review]
qtdemux: rework taglist handling

Can you check if this patch fixes the issue for you? This way it won't ignore the xmp metadata just because it hasn't created a taglist yet.

David? Did you get a chance to test Thiago's patch against your file?

I must have an old version of the file because I had to merge the patch by hand :(
But yes, the file then played fine :)

Thanks Thiago

Thiago's patch is a much appreciated simplification of the taglist handling, but I have found one problem that it does not yet solve.

This is that gst_demux_push_tags() takes a second reference while pushing the new tag event to send global tags (qtdemux.c:958), so tag_list becomes non-writable.  A subsequent call to qtdemux_handle_xmp_taglist() then fails with
"gst_tag_list_insert: assertion 'gst_tag_list_is_writable (into)' failed"

It seems inefficient to make qtdemux->tag_list writable immediately after the 2nd reference is taken (although perhaps easier to think about), so I have settled for doing so in qtdemux_parse_uuid(), just before the call to qtdemux_handle_xmp_taglist().  This fixes my immediate problem, but am I still wondering if it would be better to keep qtdemux->tag_list always writable?

I'll attach a qtdemux debug trace for the failing condition and a further patch file.

Created attachment 343429 [details] [review]
Proprosed addition to Thiago's patch

Created attachment 343430 [details]
Debug trace from non-writable taglist case


I haven't found an easy way for anyone else to reproduce this, but it fails readily for me, so I can test alternative solutions.

commit 642331fd7f5b6236049fbd3b56f2f7f8a71ba174
Author: David Warman <dwarman@manglebit.org>
Date:   Fri Jan 13 12:27:40 2017 +0000

    qtdemux: avoid XMP tag parsing fatal error.
    
    qtdemux_handle_xmp_taglist() requires a writable taglist,
    but qtdemux->tag_list can become non-writable, specifically
    after sending global tags (qtdemux.c:958), which adds a
    second reference.  Ensure the list is made writable before
    calling (make_writable will copy the list if necessary).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766177

commit 5bb7ca8a623426cde67a6cf54526bcc6a3fd3395
Author: Thiago Santos <thiagossantos@gmail.com>
Date:   Tue May 31 13:17:45 2016 -0300

    qtdemux: rework taglist handling
    
    Keep taglist around during element existance to avoid having to
    create it at different places before usage. Makes code simpler to handle.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=766177

Should we consider this for 1.10 too?

*** Bug 779245 has been marked as a duplicate of this bug. ***