[Bug 680998] [wavenc] TOC support

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=680998
  GStreamer | gst-plugins-good | git

Tim-Philipp Müller <t.i.m> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #239400|none                        |reviewed
             status|                            |

--- Comment #14 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2013-03-21 09:32:38 UTC ---
(From update of attachment 239400)
>+static void
>+gst_wavenc_write_cues (guint8 ** data, struct cue_point *cues, guint32 ncues)
>+{
>+  guint32 i;
>+
>+  for (i = 0; i < ncues; i++) {
>+    GST_WRITE_UINT32_LE (*data, cues[i].id);
>+    GST_WRITE_UINT32_LE (*data + 4, cues[i].position);
>+    memcpy (*data + 8, (char *) cues[i].data_chunk_id, 4);
>+    GST_WRITE_UINT32_LE (*data + 12, cues[i].chunk_start);
>+    GST_WRITE_UINT32_LE (*data + 16, cues[i].block_start);
>+    GST_WRITE_UINT32_LE (*data + 20, cues[i].sample_offset);
>+    *data += 24;
>+  }
>+}

I wonder if GstByteWriter wouldn't be more convenient here (and in the other
functions)..

>+    uid = gst_toc_entry_get_uid (entry);
>+    id = g_ascii_strtoll (uid, NULL, 0);
>+    /* check if id unique compatible with guint32 else generate random */
>+    if (id <= 4294967295 && gst_wavenc_check_cue_id (cues, i, id)) {
>+      cues[i].id = (guint32) id;
>+    } else {
>+      cues[i].id = g_random_int ();
>+    }

id is a signed int64 here, so might want to check for >=0 as well or use
_stroull instead. Are these CUE IDs here supposed to have any semantics (are
they supposed to be like track/chapter numbers or just random identifiers?)
Even if you make a random one, you probably still want to make sure it doesn't
exist yet..


>     case GST_EVENT_EOS:{
>       GST_DEBUG_OBJECT (wavenc, "got EOS");
>+      if (wavenc->toc) {
>+        gst_wavenc_write_toc (wavenc);
>+      }

Is the TOC supposed to be at the and of the WAV file?


>+    case GST_EVENT_TOC:
>+      gst_event_parse_toc (event, &toc, NULL);
>+      if (toc) {
>+        if (wavenc->toc != toc) {
>+          if (wavenc->toc)
>+            gst_toc_unref (wavenc->toc);
>+          wavenc->toc = toc;
>+        }

  else {
    gst_toc_unref (toc);
  }

-- 
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.