[Bug 738538] Gstreamer should support reading of CAF files

[GStreamer (bugzilla.gnome.org)]

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

Olivier Crete (Tester) <olivier.crete> changed:

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

--- Comment #3 from Olivier Crete (Tester) <olivier.crete at ocrete.ca> 2014-10-14 23:07:25 UTC ---
Review of attachment 288549:
 --> (https://bugzilla.gnome.org/review?bug=738538&attachment=288549)

General stylistic comment, in GStreamer, we normally don't use CamelCase, but
it's not a big deal. Also, this not really being a parser (but a demuxer), I'm
not sure if GstBaseParse is really the right base class, but we have no demuxer
base class, so if it works, I'd leave it as-is.

If you have one a typefind function for the typefind plugin would also be
appreciated, so it can "just work".

::: gst/audioparsers/gstcafparse.c
@@ +54,3 @@
+    "channels = (int) [ 1, MAX ]," \
+    "layout = interleaved, " \
+    "channel-mask = (bitmask) 0"

Leave that channel mask out I think. 0 sounds wrong.

@@ +95,3 @@
+
+  gst_element_class_set_static_metadata (element_class, "CAF audio parser",
+      "Codec/Parser/Audio", "Parses a Core Audio (CAF) audio stream",

In GStreamer nomenclature, it's really a Demuxer, not a parser (so it
wavparse).

@@ +109,3 @@
+{
+  gst_caf_parse_reset (caf);
+  GST_PAD_SET_ACCEPT_INTERSECT (GST_BASE_PARSE_SINK_PAD (caf));

This is not needed, the sink caps have no properties.

@@ +222,3 @@
+      "rate", G_TYPE_INT, rate, "channels", G_TYPE_INT, channelsPerFrame,
+      "layout", G_TYPE_STRING, "interleaved",
+      "channel-mask", GST_TYPE_BITMASK, G_GUINT64_CONSTANT (0), NULL);

To create the audio caps, its safer to do gst_audio_info_init();
gst_audio_info_set_format(); gst_audio_info_to_caps();

@@ +223,3 @@
+      "layout", G_TYPE_STRING, "interleaved",
+      "channel-mask", GST_TYPE_BITMASK, G_GUINT64_CONSTANT (0), NULL);
+  gst_pad_set_caps (GST_BASE_PARSE_SRC_PAD (GST_BASE_PARSE (caf)), caps);

This gst_pad_set_caps() is redundant. In GStreamer 1.x, it's just a compat
macro to gst_pad_push_event(gst_event_new_caps()).

@@ +263,3 @@
+    case GST_MAKE_FOURCC ('a', 'l', 'a', 'c'):
+      GST_DEBUG_OBJECT (caf, "format '%" GST_FOURCC_FORMAT
+          "' not yet implemented", GST_FOURCC_ARGS (formatID));

If you return a GST_FLOW_ERROR, you must post an error message with
GST_ELEMENT_ERROR()

@@ +267,3 @@
+    default:
+      GST_DEBUG_OBJECT (caf, "unknown format '%" GST_FOURCC_FORMAT "'",
+          GST_FOURCC_ARGS (formatID));

This one too.

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