[Bug 738538] Add demuxer/parser for Apple's Core Audio Format (CAF)

GStreamer (bugzilla.gnome.org) bugzilla at gnome.org
Mon Nov 24 10:31:15 PST 2014


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

Reynaldo H. Verdejo Pinochet <reynaldo> changed:

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

--- Comment #12 from Reynaldo H. Verdejo Pinochet <reynaldo at opendot.cl> 2014-11-24 18:31:11 UTC ---
(From update of attachment 289335)
Hi

> [..]
>+static gboolean
>+checkCAFFileHeader (GstCafDemux * caf, GstMapInfo * map)
>+{
>+  guint32 fileType = GST_READ_UINT32_LE (map->data);
>+  guint16 version = GST_READ_UINT16_BE (map->data + 4);
>+  guint16 flags = GST_READ_UINT16_BE (map->data + 6);
>+
>+  if (fileType != GST_MAKE_FOURCC ('c', 'a', 'f', 'f')) {
>+    GST_ELEMENT_ERROR (caf, STREAM, TYPE_NOT_FOUND, (NULL),
>+        ("Invalid CAF header: %" GST_FOURCC_FORMAT,
>+            GST_FOURCC_ARGS (fileType)));
>+    return FALSE;
>+  }
>+  GST_DEBUG_OBJECT (caf, "CAF version %d flags 0x%x", (gint) version,
>+      (guint) flags);
>+
>+  return TRUE;
>+}

Might make sense to at least issue a warning if version != 1.0?
I understand this is what we are absolutely* sure this code supports.

> [..]
>+static gboolean
>+initLPCM (GstCafDemux * caf, gdouble sampleRate, guint32 formatFlags,
>+    guint32 bytesPerPacket, guint32 framesPerPacket, guint32 channelsPerFrame,
>+    guint32 bitsPerChannel)
>+{
>+  const gboolean isFloat = (formatFlags & 0x1) != 0;
>+  const gboolean isLittleEndian = (formatFlags & 0x2) != 0;
>+  const guint rate = (gint) sampleRate;
>+  GstAudioFormat format;
>+  GstCaps *caps;
>+
>+  if (sampleRate <= .0 || rate < 0) {
>+    GST_DEBUG_OBJECT (caf, "Sample rate must be > 0 (not %lf -> %d)",
>+        sampleRate, rate);
>+    return FALSE;
>+  }

Please see bellow

>+
>+  if (framesPerPacket != 1) {
>+    GST_DEBUG_OBJECT (caf, "framesPerPacket must be 1 (not %d)",
>+        framesPerPacket);
>+    return FALSE;
>+  }
>+

Ditto

>+  if (channelsPerFrame <= 0) {
>+    GST_DEBUG_OBJECT (caf, "channelsPerFrame must be > 1 (not %d)",
>+        channelsPerFrame);
>+    return FALSE;
>+  }
>+

Ditto

>+  if (isFloat) {
>+    switch (bitsPerChannel) {
>+      case 64:
>+        format = isLittleEndian ? GST_AUDIO_FORMAT_F64LE :
>+            GST_AUDIO_FORMAT_F64BE;
>+        break;
>+      case 32:
>+        format = isLittleEndian ? GST_AUDIO_FORMAT_F32LE :
>+            GST_AUDIO_FORMAT_F32BE;
>+        break;
>+      default:
>+        GST_DEBUG_OBJECT (caf,
>+            "Only float samples with 32 or 64 bits are supported (not %d)",
>+            bitsPerChannel);
>+        return FALSE;

Ditto

>+    }
>+  } else {
>+    switch (bitsPerChannel) {
>+      case 16:
>+        format = isLittleEndian ? GST_AUDIO_FORMAT_S16LE :
>+            GST_AUDIO_FORMAT_S16BE;
>+        break;
>+      case 24:
>+        format = isLittleEndian ? GST_AUDIO_FORMAT_S24LE :
>+            GST_AUDIO_FORMAT_S24BE;
>+        break;
>+      case 32:
>+        format = isLittleEndian ? GST_AUDIO_FORMAT_S32LE :
>+            GST_AUDIO_FORMAT_S32BE;
>+        break;
>+      default:
>+        GST_DEBUG_OBJECT (caf,
>+            "Only int samples with 16, 24 or 32 bits are supported (not %d)",
>+            bitsPerChannel);
>+        return FALSE;

Ditto

> [..]
>+  /* TODO check bytesPerPacket */
>+  caf->frameSize = bytesPerPacket;
>+
>+  if (caf->channelMask == -1) {
>+    GST_DEBUG_OBJECT (caf, "Setting default channel-mask");
>+    if (channelsPerFrame == 2) {
>+      caf->channelMask = (1 << GST_AUDIO_CHANNEL_POSITION_FRONT_LEFT) |
>+          (1 << GST_AUDIO_CHANNEL_POSITION_FRONT_RIGHT);
>+    } else {
>+      caf->channelMask = 0;
>+    }
>+  }
>+
>+  gst_audio_info_set_format (&caf->audioInfo, format, rate, channelsPerFrame,
>+      NULL);
>+  if (caf->channelMask == 0
>+      || !gst_audio_channel_positions_from_mask (channelsPerFrame,
>+          caf->channelMask, caf->audioInfo.position)) {
>+    int k;
>+    for (k = 0; k < 64; ++k) {
>+      GST_AUDIO_INFO_POSITION (&caf->audioInfo, k) =
>+          GST_AUDIO_CHANNEL_POSITION_NONE;
>+    }
>+    GST_INFO_OBJECT (caf, "Setting channel-mask to 0 (from 0x%"
>+        G_GINT64_MODIFIER "x)", caf->channelMask);
>+  }
>+
>+  caps = gst_audio_info_to_caps (&caf->audioInfo);
>+  GST_INFO_OBJECT (caf, "caps changed to %" GST_PTR_FORMAT, caps);
>+  gst_pad_push_event (GST_BASE_PARSE_SRC_PAD (caf), gst_event_new_caps (caps));
>+
>+  gst_caps_unref (caps);
>+
>+  return TRUE;
>+}

Looking at the error out path seems like pretty all the above
GST_DEBUG_OBJECT() messages should rather be WARNINGs. Unless
they catch known unfinished details on this new element. In
which case you should go with GST_FIXME_OBJECT() instead.

>+
>+static void
>+handleChannelBitmap (GstCafDemux * caf, guint32 channelBitmap)
>+{
>+  /* up to POSITION_REAR_CENTER the positions are in CAF the same  as in gst */

Drop the extra space before "as"

>+  if (channelBitmap < (1 << (GST_AUDIO_CHANNEL_POSITION_REAR_CENTER + 1)))
>+    caf->channelMask = channelBitmap;
>+  else {
>+    GST_WARNING_OBJECT (caf, "Channel bitmask %x not yet implemented",
>+        channelBitmap);

GST_FIXME_OBJECT()

> [..]
>+static gboolean
>+handleDescChunk (GstCafDemux * caf, const guint8 * buf)
>+{
>+  gdouble sampleRate = GST_READ_DOUBLE_BE (buf);
>+  guint32 formatID = GST_READ_UINT32_LE (buf + 8);
>+  guint32 formatFlags = GST_READ_UINT32_BE (buf + 12);
>+  guint32 bytesPerPacket = GST_READ_UINT32_BE (buf + 16);
>+  guint32 framesPerPacket = GST_READ_UINT32_BE (buf + 20);
>+  guint32 channelsPerFrame = GST_READ_UINT32_BE (buf + 24);
>+  guint32 bitsPerChannel = GST_READ_UINT32_BE (buf + 28);
>+
>+  GST_DEBUG_OBJECT (caf, "'desc': rate %lf, flags 0x%x, bytesPerPacket %d, "
>+      "framesPerPacket %d, channels %d, bitsPerChannel %d", sampleRate,
>+      formatFlags, bytesPerPacket, framesPerPacket, channelsPerFrame,
>+      bitsPerChannel);
>+  switch (formatID) {
>+    case GST_MAKE_FOURCC ('l', 'p', 'c', 'm'):
>+      return initLPCM (caf, sampleRate, formatFlags, bytesPerPacket,
>+          framesPerPacket, channelsPerFrame, bitsPerChannel);
>+    case GST_MAKE_FOURCC ('i', 'm', 'a', '4'):
>+    case GST_MAKE_FOURCC ('a', 'a', 'c', ' '):
>+    case GST_MAKE_FOURCC ('M', 'A', 'C', '3'):
>+    case GST_MAKE_FOURCC ('M', 'A', 'C', '6'):
>+    case GST_MAKE_FOURCC ('u', 'l', 'a', 'w'):
>+    case GST_MAKE_FOURCC ('a', 'l', 'a', 'w'):
>+    case GST_MAKE_FOURCC ('.', 'm', 'p', '1'):
>+    case GST_MAKE_FOURCC ('.', 'm', 'p', '2'):
>+    case GST_MAKE_FOURCC ('.', 'm', 'p', '3'):
>+    case GST_MAKE_FOURCC ('a', 'l', 'a', 'c'):
>+      GST_ELEMENT_ERROR (caf, STREAM, NOT_IMPLEMENTED, (NULL),
>+          ("format '%" GST_FOURCC_FORMAT "' not yet implemented",
>+              GST_FOURCC_ARGS (formatID)));
>+      return FALSE;

GST_FIXME_OBJECT() too.

> [..]
>+static GstFlowReturn
>+gst_caf_demux_handle_frame (GstBaseParse * demux, GstBaseParseFrame * frame,
>+    gint * skipsize)
>+{
>+  GstMapInfo map;
>+  GstFlowReturn ret = GST_FLOW_ERROR;
>+  GstCafDemux *caf = GST_CAF_DEMUX (demux);
>+
>+  if (G_LIKELY (caf->state == GST_CAF_STATE_GOT_DATA)) {
>+    gsize bufSize = gst_buffer_get_size (frame->buffer);
>+    gsize pushSize = MIN (caf->dataBytesLeft,
>+        (bufSize / caf->frameSize) * caf->frameSize);
>+
>+    GST_DEBUG_OBJECT (caf, "pushing data, bytes left=%" G_GUINT64_FORMAT
>+        " avail=%" G_GUINT64_FORMAT " frameSize:%d size:%" G_GUINT64_FORMAT,
>+        caf->dataBytesLeft, bufSize, caf->frameSize, pushSize);
>+    if (caf->dataBytesLeft == 0) {
>+      GST_DEBUG_OBJECT (caf, "Data chunk done");
>+      return GST_FLOW_OK;
>+    }
>+    caf->dataBytesLeft -= pushSize;
>+    return gst_base_parse_finish_frame (demux, frame, pushSize);
>+  }
>+
>+  gst_buffer_map (frame->buffer, &map, GST_MAP_READ);
>+
>+  g_assert (map.size >= 12);
>+
>+  if (G_UNLIKELY (caf->state == GST_CAF_STATE_NONE)) {
>+    if (!checkCAFFileHeader (caf, &map))
>+      goto fail;
>+    caf->state = GST_CAF_STATE_INITIALIZED;
>+    *skipsize = 8;
>+  } else {
>+    guint32 chunkType = GST_READ_UINT32_LE (map.data);
>+    gint64 chunkSize = GST_READ_UINT64_BE (map.data + 4);
>+
>+    g_assert (caf->state == GST_CAF_STATE_INITIALIZED);
>+
>+    switch (chunkType) {
>+      case GST_MAKE_FOURCC ('f', 'r', 'e', 'e'):
>+        *skipsize = chunkSize + 12;
>+        GST_DEBUG_OBJECT (caf, "Skipping 'free' chunk of size %"
>+            G_GINT64_FORMAT, chunkSize);
>+        break;
>+      case GST_MAKE_FOURCC ('c', 'h', 'a', 'n'):
>+        if (!fullChunkMapped (demux, chunkSize, &map, skipsize))
>+          break;
>+        handleChanChunk (caf, map.data + 12);
>+        *skipsize = chunkSize + 12;
>+        break;
>+      case GST_MAKE_FOURCC ('d', 'e', 's', 'c'):
>+        if (!fullChunkMapped (demux, chunkSize, &map, skipsize))
>+          break;
>+        if (!handleDescChunk (caf, map.data + 12))
>+          goto fail;
>+        *skipsize = chunkSize + 12;
>+        break;
>+      case GST_MAKE_FOURCC ('d', 'a', 't', 'a'):
>+        if (map.size < 16) {
>+          /* try until we have enough data for header */
>+          gst_base_parse_set_min_frame_size (demux, 16);
>+          *skipsize = 0;
>+          break;
>+        }
>+        {
>+          const guint32 editCount = GST_READ_UINT32_BE (map.data + 12);
>+          GST_DEBUG_OBJECT (caf, "editCount=%d", editCount);
>+          *skipsize = 16;
>+          caf->state = GST_CAF_STATE_GOT_DATA;
>+          caf->dataBytesLeft = chunkSize;
>+        }
>+        break;
>+      case GST_MAKE_FOURCC ('k', 'u', 'k', 'i'):
>+      case GST_MAKE_FOURCC ('i', 'n', 's', 't'):
>+      case GST_MAKE_FOURCC ('u', 'm', 'i', 'd'):
>+      case GST_MAKE_FOURCC ('u', 'u', 'i', 'd'):
>+        *skipsize = chunkSize + 12;
>+        GST_DEBUG_OBJECT (caf, "Chunk '%" GST_FOURCC_FORMAT
>+            "' not yet impelemented, skipping size %" G_GINT64_FORMAT,
>+            GST_FOURCC_ARGS (chunkType), chunkSize);

As above. Should be a GST_FIXME_OBJECT() instead.

>+        break;
>+      default:
>+        *skipsize = chunkSize + 12;
>+        GST_DEBUG_OBJECT (caf, "Skipping unknown chunk '%" GST_FOURCC_FORMAT
>+            "' size %" G_GINT64_FORMAT, GST_FOURCC_ARGS (chunkType), chunkSize);

_WARNING_OBJECT()

Also, please add the bug number in the commit message.

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


More information about the gstreamer-bugs mailing list