[Bug 680998] [wavenc] TOC support

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #15 from Anton Belka <dark-al at ya.ru> 2013-03-21 15:29:52 UTC ---
(In reply to comment #14)
> (From update of attachment 239400 [details])
> >+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)..
> 

I use GST_WRITE_UINT32_LE there, because it's used in other places at wavenc
(to keep the overall coding style).

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

Oh, here really better use _stroll and check if >=0, because look better. :)
CUE IDs don't have any semantic. From specification:
Each cue point has a unique identification value used to associate cue points
with information in other chunks. For example, a Label chunk contains text that
describes a point in the wave file by referencing the associated cue point.

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

Yes, CUEs stored at the end 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);
>   }

Why we need unref TOC here if it will be NULL?

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