[Bug 790686] matroskamux: re-activate TOC support

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=790686

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #364153|none                        |needs-work
             status|                            |

--- Comment #6 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 364153:
 --> (https://bugzilla.gnome.org/review?bug=790686&attachment=364153)

::: gst/matroska/matroska-mux.c
@@ +2658,3 @@
       gst_ebml_write_master_start (ebml, GST_MATROSKA_ID_CHAPTERATOM);
+
+  uid = gst_matroska_mux_create_uid (mux);

We're not using the UIDs from the GstTocEntry but always create our own here.
Why? We're also not even updating the entry's UID, which is what the previous
code seemed to have done

@@ +2933,3 @@
+    toc_entries = gst_toc_get_entries (toc);
+    if (toc_entries != NULL) {
+      cur = g_list_first (toc_entries);

"cur = g_list_first(toc_entries)" is the same as "cur = toc_entries" btw

@@ +2964,3 @@
+        gst_matroska_mux_write_chapter_edition (mux, cur->data, ebml,
+            &master_chapters);
+      }

The to_write list seems to be never freed

::: tests/check/pipelines/toc.c
@@ +183,3 @@
+
+  launch_str = g_strdup_printf ("filesrc location=%s ! %s name=demux ! "
+      "fakesink", file, demuxer);

Thanks for also providing a test! But instead of a full pipeline based test,
maybe just add something to the existing matroskademux / matroskamux tests that
only checks the TOC functionality? E.g. for matroskamux configure the TOC, and
check that the output is bit-equivalent with what you would expect to get for
the TOC. For matroskademux, push some minimal Matroska beginning into the
demuxer and check that you get the expected TOC out.

This here is testing too many things at once and if something fails it's harder
to debug

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.