jpegformat: code cleanups

Move the jpeg marker defines to a header (to be used for jifmux). Rewrite some
comments. Use gst_element_class_set_details_simple(). Add more logging.

4 files changed, 155 insertions, 82 deletions

diff --git a/gst/jpegformat/Makefile.am b/gst/jpegformat/Makefile.am
index c6c5ed368..5b85f3a0b 100644
--- a/gst/jpegformat/Makefile.am
+++ b/gst/jpegformat/Makefile.am
@@ -6,4 +6,4 @@ libgstjpegformat_la_LIBADD = $(GST_LIBS) $(GST_BASE_LIBS)
 libgstjpegformat_la_LDFLAGS = $(GST_PLUGIN_LDFLAGS)
 libgstjpegformat_la_LIBTOOLFLAGS = --tag=disable-static
 
-noinst_HEADERS = gstjpegparse.h
+noinst_HEADERS = gstjpegformat.h gstjpegparse.h
diff --git a/gst/jpegformat/gstjpegformat.h b/gst/jpegformat/gstjpegformat.h
new file mode 100644
index 000000000..aac4d72c3
--- /dev/null
+++ b/gst/jpegformat/gstjpegformat.h
@@ -0,0 +1,89 @@
+/* GStreamer
+ *
+ * jpegformat: a plugin for JPEG Interchange Format
+ *
+ * Copyright (C) <2010> Stefan Kost <ensonic@users.sf.net>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+
+#ifndef __GST_JPEG_FORMAT_H__
+#define __GST_JPEG_FORMAT_H__
+
+G_BEGIN_DECLS
+
+/*
+ * JPEG Markers
+ */
+ 
+/* Start Of Frame markers, non-differential, Huffman coding */
+#define SOF0      0xc0  /* Baseline DCT */
+#define SOF1      0xc1  /* Extended sequential DCT */
+#define SOF2      0xc2  /* Progressive DCT */
+#define SOF3      0xc3  /* Lossless */
+
+/* Start Of Frame markers, differential, Huffman coding */
+#define SOF5      0xc5
+#define SOF6      0xc6
+#define SOF7      0xc7
+
+/* Start Of Frame markers, non-differential, arithmetic coding */
+#define JPG       0xc8  /* Reserved */
+#define SOF9      0xc9
+#define SOF10     0xca
+#define SOF11     0xcb
+
+/* Start Of Frame markers, differential, arithmetic coding */
+#define SOF13     0xcd
+#define SOF14     0xce
+#define SOF15     0xcf
+
+/* Restart interval termination */
+#define RST0      0xd0  /* Restart ... */
+#define RST1      0xd1
+#define RST2      0xd2
+#define RST3      0xd3
+#define RST4      0xd4
+#define RST5      0xd5
+#define RST6      0xd6
+#define RST7      0xd7
+
+#define SOI       0xd8  /* Start of image */
+#define EOI       0xd9  /* End Of Image */
+#define SOS       0xda  /* Start Of Scan */
+
+#define DHT       0xc4  /* Huffman Table(s) */
+#define DAC       0xcc
+#define DQT       0xdb  /* Huffman Table(s) */
+#define DNL       0xdc  /* Number of lines */
+#define DRI       0xdd  /* Restart Interval */
+#define DHP       0xde  /* Hierarchical progression */
+#define EXP       0xdf
+
+#define APP0      0xe0  /* Application marker */
+#define APP1      0xe1
+#define APP15     0xef
+
+#define JPG0      0xf0  /* Reserved ... */
+#define JPG13     0xfd
+#define COM       0xfe  /* Comment */
+
+#define TEM       0x01
+
+G_END_DECLS
+
+#endif /* __GST_JPEG_FORMAT_H__ */
diff --git a/gst/jpegformat/gstjpegparse.c b/gst/jpegformat/gstjpegparse.c
index f59c12e81..2c9894fef 100644
--- a/gst/jpegformat/gstjpegparse.c
+++ b/gst/jpegformat/gstjpegparse.c
@@ -47,64 +47,6 @@
 
 #include "gstjpegparse.h"
 
-/*
- * JPEG Markers
- */
-#define SOF0      0xc0
-#define SOF1      0xc1
-#define SOF2      0xc2
-#define SOF3      0xc3
-
-#define SOF5      0xc5
-#define SOF6      0xc6
-#define SOF7      0xc7
-
-#define JPG       0xc8
-#define SOF9      0xc9
-#define SOF10     0xca
-#define SOF11     0xcb
-#define SOF13     0xcd
-#define SOF14     0xce
-#define SOF15     0xcf
-
-#define DHT       0xc4
-
-#define DAC       0xcc
-
-#define RST0      0xd0
-#define RST1      0xd1
-#define RST2      0xd2
-#define RST3      0xd3
-#define RST4      0xd4
-#define RST5      0xd5
-#define RST6      0xd6
-#define RST7      0xd7
-
-#define SOI       0xd8
-#define EOI       0xd9
-#define SOS       0xda
-#define DQT       0xdb
-#define DNL       0xdc
-#define DRI       0xdd
-#define DHP       0xde
-#define EXP       0xdf
-
-#define APP0      0xe0
-#define APP1      0xe1
-#define APP15     0xef
-
-#define JPG0      0xf0
-#define JPG13     0xfd
-#define COM       0xfe
-
-#define TEM       0x01
-
-static const GstElementDetails gst_jpeg_parse_details =
-GST_ELEMENT_DETAILS ("JPEG stream parser",
-    "Codec/Parser/Video",
-    "Parse JPEG images into single-frame buffers",
-    "Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>");
-
 static GstStaticPadTemplate gst_jpeg_parse_src_pad_template =
 GST_STATIC_PAD_TEMPLATE ("src",
     GST_PAD_SRC,
@@ -127,9 +69,6 @@ GST_STATIC_PAD_TEMPLATE ("sink",
 GST_DEBUG_CATEGORY_STATIC (jpeg_parse_debug);
 #define GST_CAT_DEFAULT jpeg_parse_debug
 
-#define GST_JPEG_PARSE_GET_PRIVATE(obj)  \
-    (G_TYPE_INSTANCE_GET_PRIVATE ((obj), GST_TYPE_JPEG_PARSE, GstJpegParsePrivate))
-
 struct _GstJpegParsePrivate
 {
   GstPad *srcpad;
@@ -192,7 +131,11 @@ gst_jpeg_parse_base_init (gpointer g_class)
       gst_static_pad_template_get (&gst_jpeg_parse_src_pad_template));
   gst_element_class_add_pad_template (element_class,
       gst_static_pad_template_get (&gst_jpeg_parse_sink_pad_template));
-  gst_element_class_set_details (element_class, &gst_jpeg_parse_details);
+  gst_element_class_set_details_simple (element_class,
+      "JPEG stream parser",
+      "Codec/Parser/Video",
+      "Parse JPEG images into single-frame buffers",
+      "Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>");
 }
 
 static void
@@ -216,7 +159,8 @@ gst_jpeg_parse_init (GstJpegParse * parse, GstJpegParseClass * g_class)
 {
   GstPad *sinkpad;
 
-  parse->priv = GST_JPEG_PARSE_GET_PRIVATE (parse);
+  parse->priv = G_TYPE_INSTANCE_GET_PRIVATE (parse, GST_TYPE_JPEG_PARSE,
+      GstJpegParsePrivate);
 
   /* create the sink and src pads */
   sinkpad = gst_pad_new_from_static_template (&gst_jpeg_parse_sink_pad_template,
@@ -275,9 +219,16 @@ gst_jpeg_parse_sink_setcaps (GstPad * pad, GstCaps * caps)
   return TRUE;
 }
 
-/* Flush everything until the next JPEG header.  The header is considered
- * to be the a start marker (0xff 0xd8) followed by any other marker (0xff ...).
- * Returns TRUE if the header was found, FALSE if more data is needed. */
+/*
+ * gst_jpeg_parse_skip_to_jpeg_header:
+ * @parse: the parser
+ *
+ * Flush everything until the next JPEG header.  The header is considered
+ * to be the a start marker SOI (0xff 0xd8) followed by any other marker
+ * (0xff ...).
+ *
+ * Returns: TRUE if the header was found, FALSE if more data is needed.
+ */
 static gboolean
 gst_jpeg_parse_skip_to_jpeg_header (GstJpegParse * parse)
 {
@@ -308,9 +259,16 @@ gst_jpeg_parse_parse_tag_has_entropy_segment (guint8 tag)
   return FALSE;
 }
 
-/* Find the next marker, based on the marker at data.  data[0] must be 0xff.
- * Returns the offset of the next valid marker.  Returns -1 if adapter doesn't
- * have enough data. */
+/*
+ * gst_jpeg_parse_match_next_marker:
+ * @data: data to scan (must start with 0xff)
+ * @size: amount of bytes in @data (must be >=2)
+ *
+ * Find the next marker, based on the marker at @data.
+ * 
+ * Returns: the offset of the next valid marker or -1 if buffer doesn't have
+ * enough data.
+ */
 static guint
 gst_jpeg_parse_match_next_marker (const guint8 * data, guint size)
 {
@@ -327,9 +285,13 @@ gst_jpeg_parse_match_next_marker (const guint8 * data, guint size)
     return -1;
   else
     marker_len = GST_READ_UINT16_BE (data + 2) + 2;
+
+  GST_LOG ("Have marker %x with length %u", data[1], marker_len);
+
   /* Need marker_len for this marker, plus two for the next marker. */
   if (G_UNLIKELY (marker_len + 2 >= size))
     return -1;
+
   if (G_UNLIKELY (gst_jpeg_parse_parse_tag_has_entropy_segment (tag))) {
     while (!(data[marker_len] == 0xff && data[marker_len + 1] != 0x00)) {
       if (G_UNLIKELY (marker_len + 2 >= size))
@@ -340,8 +302,16 @@ gst_jpeg_parse_match_next_marker (const guint8 * data, guint size)
   return marker_len;
 }
 
-/* Returns the position beyond the end marker, -1 if insufficient data and -2
-   if marker lengths are inconsistent. data must start with 0xff. */
+/*
+ * gst_jpeg_parse_find_end_marker:
+ * @data: data to scan (must start with 0xff)
+ * @size: amount of bytes in @data
+ *
+ * Find next position beyond end maker.
+ *
+ * Returns: the position, -1 if insufficient data and -2 if marker lengths are
+ * inconsistent. 
+ */
 static guint
 gst_jpeg_parse_find_end_marker (GstJpegParse * parse, const guint8 * data,
     guint size)
@@ -405,6 +375,7 @@ gst_jpeg_parse_get_image_length (GstJpegParse * parse)
 
   if (offset == -1) {
     GST_DEBUG_OBJECT (parse, "Insufficient data.");
+    /* FIXME: remember size, so that we don't rescan from begin */
     return 0;
   } else if (G_UNLIKELY (offset == -2)) {
     GST_DEBUG_OBJECT (parse, "Lost sync, resyncing.");
@@ -442,6 +413,7 @@ gst_jpeg_parse_sof (GstJpegParse * parse, GstByteReader * reader)
   guint8 i, value;
   gint temp;
 
+  /* flush length field */
   if (!gst_byte_reader_skip (reader, 2))
     return FALSE;
 
@@ -501,6 +473,8 @@ gst_jpeg_parse_sof (GstJpegParse * parse, GstByteReader * reader)
     return FALSE;
   }
 
+  GST_DEBUG_OBJECT (parse, "Header parsed");
+
   return TRUE;
 }
 
@@ -509,7 +483,7 @@ gst_jpeg_parse_read_header (GstJpegParse * parse, GstBuffer * buffer)
 {
   GstByteReader reader = GST_BYTE_READER_INIT_FROM_BUFFER (buffer);
   guint8 marker;
-  guint16 comsize;
+  guint16 size;
   gboolean foundSOF = FALSE;
 
   if (!gst_byte_reader_peek_uint8 (&reader, &marker))
@@ -522,7 +496,7 @@ gst_jpeg_parse_read_header (GstJpegParse * parse, GstBuffer * buffer)
     if (!gst_byte_reader_get_uint8 (&reader, &marker))
       goto error;
 
-    GST_INFO_OBJECT (parse, "marker = %x", marker);
+    GST_DEBUG_OBJECT (parse, "marker = %x", marker);
 
     switch (marker) {
       case SOS:                /* start of scan (begins compressed data) */
@@ -543,25 +517,27 @@ gst_jpeg_parse_read_header (GstJpegParse * parse, GstBuffer * buffer)
       case DHT:
       case DQT:
         /* Ignore these codes */
-        if (!gst_byte_reader_get_uint16_be (&reader, &comsize))
+        if (!gst_byte_reader_get_uint16_be (&reader, &size))
           goto error;
-        if (!gst_byte_reader_skip (&reader, comsize - 2))
+        if (!gst_byte_reader_skip (&reader, size - 2))
           goto error;
-        GST_LOG_OBJECT (parse, "comment skiping %u bytes", comsize - 2);
+        GST_LOG_OBJECT (parse, "unhandled marker %x skiping %u bytes", marker,
+            size - 2);
         break;
 
       case SOF2:
         parse->priv->interlaced = TRUE;
-
+        /* fall through */
       case SOF0:
-        /* Flush length field */
         foundSOF = TRUE;
+        /* parse Start Of Frame */
         if (!gst_jpeg_parse_sof (parse, &reader))
           goto error;
 
         return TRUE;
 
       default:
+        GST_WARNING_OBJECT (parse, "unhandled marker %x, leaving", marker);
         /* Not SOF or SOI.  Must not be a JPEG file (or file pointer
          * is placed wrong).  In either case, it's an error. */
         return FALSE;
@@ -584,6 +560,9 @@ gst_jpeg_parse_set_new_caps (GstJpegParse * parse, gboolean header_ok)
   GstCaps *caps;
   gboolean res;
 
+  GST_DEBUG_OBJECT (parse, "setting caps on srcpad (hdr_ok=%d, have_fps=%d)",
+      header_ok, parse->priv->has_fps);
+
   caps = gst_caps_new_simple ("image/jpeg",
       "parsed", G_TYPE_BOOLEAN, TRUE, NULL);
 
@@ -611,9 +590,12 @@ gst_jpeg_parse_set_new_caps (GstJpegParse * parse, gboolean header_ok)
     parse->priv->duration = GST_CLOCK_TIME_NONE;
   }
 
-  GST_DEBUG_OBJECT (parse, "setting downstream caps to %" GST_PTR_FORMAT, caps);
+  GST_DEBUG_OBJECT (parse,
+      "setting downstream caps on %s:%s to %" GST_PTR_FORMAT,
+      GST_DEBUG_PAD_NAME (parse->priv->srcpad), caps);
   res = gst_pad_set_caps (parse->priv->srcpad, caps);
   gst_caps_unref (caps);
+  gst_pad_use_fixed_caps (parse->priv->srcpad);
 
   return res;
 
@@ -727,14 +709,14 @@ gst_jpeg_parse_sink_event (GstPad * pad, GstEvent * event)
       guint available = gst_adapter_available (parse->priv->adapter);
       if (available > 0)
         gst_jpeg_parse_push_buffer (parse, available);
-      gst_pad_push_event (parse->priv->srcpad, event);
+      res = gst_pad_push_event (parse->priv->srcpad, event);
       break;
     }
     case GST_EVENT_NEWSEGMENT:
       /* Discard any data in the adapter.  There should have been an EOS before
        * to flush it. */
       gst_adapter_clear (parse->priv->adapter);
-      gst_pad_push_event (parse->priv->srcpad, event);
+      res = gst_pad_push_event (parse->priv->srcpad, event);
       parse->priv->new_segment = TRUE;
       break;
     default:
diff --git a/gst/jpegformat/gstjpegparse.h b/gst/jpegformat/gstjpegparse.h
index 3d8c45e19..dc9abda73 100644
--- a/gst/jpegformat/gstjpegparse.h
+++ b/gst/jpegformat/gstjpegparse.h
@@ -26,6 +26,8 @@
 #include <gst/gst.h>
 #include <gst/base/gstadapter.h>
 
+#include "gstjpegformat.h"
+
 G_BEGIN_DECLS
 
 #define GST_TYPE_JPEG_PARSE \