[Bug 681891] wavenc: support LIST INFO chunk

[GStreamer (bugzilla.gnome.org)]

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

Sebastian Dröge <slomo> changed:

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

--- Comment #2 from Sebastian Dröge <slomo at circular-chaos.org> 2012-08-16 08:21:34 UTC ---
Review of attachment 221215:
 --> (https://bugzilla.gnome.org/review?bug=681891&attachment=221215)

::: gst/wavenc/gstwavenc.c
@@ +631,3 @@
+        if (gst_tag_list_get_date (tags, tag, &date)) {
+          str =
+              g_strdup_printf ("%04d:%02d:%02d", g_date_get_year (date),

With : as separator? Are you sure?

@@ +640,3 @@
+      if (str) {
+        gst_byte_writer_put_uint32_le (bw, rifftags[n].fcc);
+        gst_byte_writer_put_uint32_le (bw, GST_ROUND_UP_2 (strlen (str)));

You should store the strlen() in a variable and use that here, GST_ROUND_UP_2()
and gst_byte_writer_put_uint32_le() are macros that evaluate their parameters
multiple times, i.e. you will have multiple strlen() calls

Also add a comment that rounding up to a multiple of two is safe here because
either we write until the last character of the string or the \0 terminator at
the end.

@@ +745,3 @@
+      gst_event_parse_tag (event, &tags);
+      if (tags) {
+        if (wavenc->tags != tags) {

This is not ideal, you need to differentiate between stream-scope and
global-scope taglists here.

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