[FFmpeg-devel] [PATCH 2] add tag/comment support to the raw flac demuxer

[Jim Radford]

On Thu, 4 Dec 2008 at 21:44:51, Reimar D?ffinger wrote:
> On Mon, Dec 01, 2008 at 02:18:02PM -0800, Jim Radford wrote:
> > This patch adds support for parsing vorbis comments in plain flac
> > streams.  Only metadata packets are parsed leaving the frame data to
> > be parsed in raw 1024 byte chunks like before.

> > +        init_get_bits(&gb, header, ret*8);
> > +        flac->metadata_done = get_bits(&gb, 1);
> > +        type = get_bits(&gb, 7);
> > +        length = get_bits(&gb, 24);

> Seems very much like overkill to use get_bits here. Actually
> get_be32 to me seems much more appropriate than get_buffer to get
> the header anyway.

I thought so too, but given that I need to copy the data back into the
stream where it needs to be in its original endian, so the code
doesn't get any simpler.

Thanks for the review.  I Incorporated your other suggestions.  New
patch attached.

-Jim

PS Please CC me on follow ups as I'm not subscribed.
-------------- next part --------------
Index: libavformat/raw.c
===================================================================
--- libavformat/raw.c	(revision 16041)
+++ libavformat/raw.c	(working copy)
@@ -27,7 +27,62 @@
 #include "avformat.h"
 #include "raw.h"
 
+#define RAW_PACKET_SIZE 1024
+
 /* simple formats */
+
+#ifdef CONFIG_FLAC_DEMUXER
+#include "oggdec.h" // vorbis_comment()
+
+struct flac
+{
+    int flac_done, metadata_done;
+};
+
+static int flac_read_packet(AVFormatContext *s, AVPacket *pkt)
+{
+    struct flac *flac = s->priv_data;
+
+    if (!flac->flac_done) {
+        if (av_get_packet(s->pb, pkt, 4) <= 0)
+            return AVERROR(EIO);
+        flac->flac_done = 1;
+    } else if (!flac->metadata_done) {
+        GetBitContext gb;
+        uint8_t header[4];
+        int type, length, ret;
+
+        if (get_buffer(s->pb, header, sizeof(header)) < sizeof(header))
+            return AVERROR(EIO);
+
+        init_get_bits(&gb, header, sizeof(header)*8);
+        flac->metadata_done = get_bits(&gb, 1);
+        type                = get_bits(&gb, 7);
+        length              = get_bits(&gb, 24);
+
+        if ((ret = av_new_packet(pkt, sizeof(header) + length)) < 0)
+            return ret;
+
+        memcpy(pkt->data, header, sizeof(header));
+        pkt->pos= url_ftell(s->pb) - sizeof(header);
+
+        if (get_buffer(s->pb, pkt->data + sizeof(header), length) < length)
+            return AVERROR(EIO);
+
+        pkt->size = length + sizeof(header);
+
+        if (type == 4)
+            vorbis_comment(s, pkt->data+sizeof(header), length);
+    } else {
+        if (av_get_packet(s->pb, pkt, RAW_PACKET_SIZE) <= 0)
+            return AVERROR(EIO);
+    }
+
+    pkt->stream_index = 0;
+    return pkt->size;
+}
+#endif
+
 #ifdef CONFIG_FLAC_MUXER
 static int flac_write_header(struct AVFormatContext *s)
 {
@@ -134,8 +189,6 @@
     return 0;
 }
 
-#define RAW_PACKET_SIZE 1024
-
 static int raw_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     int ret, size, bps;
@@ -773,10 +826,10 @@
 AVInputFormat flac_demuxer = {
     "flac",
     NULL_IF_CONFIG_SMALL("raw FLAC"),
-    0,
+    sizeof(struct flac),
     flac_probe,
     audio_read_header,
-    raw_read_partial_packet,
+    flac_read_packet,
     .flags= AVFMT_GENERIC_INDEX,
     .extensions = "flac",
     .value = CODEC_ID_FLAC,