pnmdec: Fix ASCII parsing

Parse gray16 properly in ascii mode, and fix
some bugs around reading data in chunks when
ascii values cross chunk boundaries

gst/pnm/gstpnmdec.c

@@ -44,6 +44,7 @@
 #include <stdio.h>
 
 static gboolean gst_pnmdec_start (GstVideoDecoder * decoder);
+static GstFlowReturn gst_pnmdec_finish (GstVideoDecoder * decoder);
 static gboolean gst_pnmdec_set_format (GstVideoDecoder * decoder,
     GstVideoCodecState * state);
 static gboolean gst_pnmdec_stop (GstVideoDecoder * decoder);
@@ -94,6 +95,7 @@ gst_pnmdec_class_init (GstPnmdecClass * klass)
       "Lutz Mueller <lutz@users.sourceforge.net>");
 
   vdec_class->start = gst_pnmdec_start;
+  vdec_class->finish = gst_pnmdec_finish;
   vdec_class->stop = gst_pnmdec_stop;
   vdec_class->parse = gst_pnmdec_parse;
   vdec_class->handle_frame = gst_pnmdec_handle_frame;
@@ -238,7 +240,7 @@ gst_pnmdec_parse_ascii (GstPnmdec * s, const guint8 * b, guint bs)
 {
   GScanner *scanner;
   guint i = 0;
-  guint target;
+  guint target, last_val = 0;
   GstMapInfo map;
   guint8 *outdata;
 
@@ -246,42 +248,78 @@ gst_pnmdec_parse_ascii (GstPnmdec * s, const guint8 * b, guint bs)
 
   gst_buffer_map (s->buf, &map, GST_MAP_WRITE);
 
-  /* leave the number of bytes already parsed */
+  if (bs) {
-  outdata = map.data + s->current_size;
+    GST_MEMDUMP_OBJECT (s, "Starting parse:", b, MIN (16, bs));
-  if (!bs) {
-    goto drop_ok;
   }
 
-  if (s->last_byte) {
+  /* leave the number of bytes already parsed */
-    while (*b >= '0' && *b <= '9') {
+  outdata = map.data + s->current_size;
-      s->last_byte = 10 * s->last_byte + *b - '0';
+
+  if (s->have_last_val) {
+    while (bs && *b >= '0' && *b <= '9') {
+      s->last_val = 10 * s->last_val + *b - '0';
       b++;
       if (!--bs) {
         goto drop_error;
       }
     }
-    if (s->last_byte > s->mngr.info.max) {
+    if (s->last_val > s->mngr.info.max) {
       GST_DEBUG_OBJECT (s, "Corrupt ASCII encoded PNM file.");
       goto drop_error;
     }
+
+    GST_LOG_OBJECT (s, "Collected partial value from previous parse - %u",
+        s->last_val);
+    if (s->mngr.info.max > 255) {
+      if (i + 1 >= target) {
+        GST_DEBUG_OBJECT (s, "PNM file contains too much data.");
+        goto drop_error;
+      }
+      if (s->out_format == GST_VIDEO_FORMAT_GRAY16_BE)
+        GST_WRITE_UINT16_BE (outdata + i, s->last_val);
+      else
+        GST_WRITE_UINT16_LE (outdata + i, s->last_val);
+      i += 2;
+    } else {
+      outdata[i++] = s->last_val;
+    }
+    last_val = s->last_val;
+    s->have_last_val = FALSE;
   }
 
-  if (s->last_byte) {
+  /* Might be no data if we're at EOS */
-    outdata[i++] = s->last_byte;
+  if (!bs)
-    s->last_byte = 0;
+    goto done;
-  }
 
   scanner = g_scanner_new (NULL);
   g_scanner_input_text (scanner, (gchar *) b, bs);
   while (!g_scanner_eof (scanner)) {
     switch (g_scanner_get_next_token (scanner)) {
       case G_TOKEN_INT:
-        if (i == target) {
+        if (s->mngr.info.max > 255) {
-          GST_DEBUG_OBJECT (s, "PNM file contains too much data.");
+          if (i + 1 >= target) {
-          g_scanner_destroy (scanner);
+            GST_DEBUG_OBJECT (s,
-          goto drop_error;
+                "PNM file contains too much data after line %u col %u.",
+                scanner->line, scanner->position);
+            g_scanner_destroy (scanner);
+            goto done;          // drop_error;
+          }
+          if (s->out_format == GST_VIDEO_FORMAT_GRAY16_BE)
+            GST_WRITE_UINT16_BE (outdata + i, scanner->value.v_int);
+          else
+            GST_WRITE_UINT16_LE (outdata + i, scanner->value.v_int);
+          i += 2;
+        } else {
+          if (i == target) {
+            GST_DEBUG_OBJECT (s,
+                "PNM file contains too much data after line %u col %u.",
+                scanner->line, scanner->position);
+            g_scanner_destroy (scanner);
+            goto drop_error;
+          }
+          outdata[i++] = scanner->value.v_int;
         }
-        outdata[i++] = scanner->value.v_int;
+        last_val = scanner->value.v_int;
         break;
       default:
         /* Should we care? */ ;
@@ -290,18 +328,25 @@ gst_pnmdec_parse_ascii (GstPnmdec * s, const guint8 * b, guint bs)
   g_scanner_destroy (scanner);
 
   /* If we didn't get the whole image, handle the last byte with care. */
-  if (i && i < target && b[bs - 1] > '0' && b[bs - 1] <= '9') {
+  if (i && i < target && b[bs - 1] >= '0' && b[bs - 1] <= '9') {
-    s->last_byte = outdata[--i];
+    s->last_val = last_val;
+    s->have_last_val = TRUE;
+    if (s->mngr.info.max > 255)
+      i -= 2;
+    else
+      i--;
+    GST_LOG_OBJECT (s, "Stored last value %u for next parse cycle",
+        s->last_val);
   }
 
-  /* Update the number of bytes parsed in this scan */
+done:
+  /* Update the number of output bytes parsed in this scan */
   s->current_size += i;
+  GST_LOG_OBJECT (s, "Parsed %u bytes, now have %u bytes of %u output",
+      i, s->current_size, s->size);
   gst_buffer_unmap (s->buf, &map);
 
   return GST_FLOW_OK;
-drop_ok:
-  gst_buffer_unmap (s->buf, &map);
-  return GST_FLOW_OK;
 
 drop_error:
   gst_buffer_unmap (s->buf, &map);
@@ -416,6 +461,8 @@ gst_pnmdec_handle_frame (GstVideoDecoder * decoder, GstVideoCodecFrame * frame)
   }
   gst_buffer_unmap (frame->output_buffer, &omap);
 
+  s->current_size = 0;
+
   r = gst_video_decoder_finish_frame (GST_VIDEO_DECODER (s), frame);
 
 out:
@@ -432,19 +479,23 @@ gst_pnmdec_parse (GstVideoDecoder * decoder, GstVideoCodecFrame * frame,
   GstPnmdec *s = GST_PNMDEC (decoder);
   GstFlowReturn r = GST_FLOW_OK;
   guint offset = 0;
-  const guint8 *raw_data;
+  const guint8 *raw_data = NULL;
 
   GST_VIDEO_CODEC_FRAME_SET_SYNC_POINT (frame);
 
   size = gst_adapter_available (adapter);
-  if (size < 8) {
+  if (size > 0)
-    goto need_more_data;
+    raw_data = gst_adapter_map (adapter, size);
-  }
+
-  raw_data = gst_adapter_map (adapter, size);
+  GST_LOG_OBJECT (s, "Entering parse with %" G_GSIZE_FORMAT " bytes. at_eos %d",
+      size, at_eos);
 
   if (s->mngr.info.fields != GST_PNM_INFO_FIELDS_ALL) {
     GstPnmInfoMngrResult res;
 
+    if (size < 8)
+      goto need_more_data;
+
     res = gst_pnm_info_mngr_scan (&s->mngr, raw_data, size);
 
     switch (res) {
@@ -461,10 +512,10 @@ gst_pnmdec_parse (GstVideoDecoder * decoder, GstVideoCodecFrame * frame,
           goto out;
 
         if (s->mngr.info.encoding == GST_PNM_ENCODING_ASCII) {
-          s->mngr.data_offset++;
           /* It is not possible to know the size of input ascii data to parse.
              So we have to parse and know the number of pixels parsed and
              then finally decide when we have full frame */
+          GST_DEBUG_OBJECT (s, "Allocating output frame of size %u", s->size);
           s->buf = gst_buffer_new_and_alloc (s->size);
         }
         offset = s->mngr.data_offset;
@@ -474,8 +525,9 @@ gst_pnmdec_parse (GstVideoDecoder * decoder, GstVideoCodecFrame * frame,
   }
 
   if (s->mngr.info.encoding == GST_PNM_ENCODING_ASCII) {
-    /* Parse ASCII data dn populate s->current_size with the number of
+    /* Parse ASCII data and populate s->current_size with the number of
        bytes actually parsed from the input data */
+    GST_DEBUG_OBJECT (s, "Parsing %u bytes at offset %u", (guint) size, offset);
     r = gst_pnmdec_parse_ascii (s, raw_data + offset, size);
   } else {
     /* Bitmap Contains 8 pixels in a byte */
@@ -505,5 +557,24 @@ gst_pnmdec_start (GstVideoDecoder * decoder)
 {
   GstPnmdec *pnmdec = (GstPnmdec *) decoder;
   gst_video_decoder_set_packetized (GST_VIDEO_DECODER (pnmdec), FALSE);
+  gst_pnmdec_flush (pnmdec);
   return TRUE;
 }
+
+static GstFlowReturn
+gst_pnmdec_finish (GstVideoDecoder * decoder)
+{
+  GstPnmdec *s = (GstPnmdec *) decoder;
+
+  GST_LOG_OBJECT (s, "finishing");
+
+  if (s->mngr.info.encoding == GST_PNM_ENCODING_ASCII) {
+    /* One last go at outputting any final value */
+    gst_pnmdec_parse_ascii (s, 0, 0);
+    if (s->size <= s->current_size) {
+      return gst_video_decoder_have_frame (decoder);
+    }
+  }
+
+  return GST_FLOW_OK;
+}

gst/pnm/gstpnmdec.h

@@ -42,7 +42,8 @@ struct _GstPnmdec
   GstVideoDecoder decoder;
   GstPnmInfoMngr mngr;
   GstVideoCodecState *input_state;
-  guint size, last_byte, current_size ;
+  guint size, last_val, current_size ;
+  gboolean have_last_val;
   GstBuffer *buf;
   GstVideoFormat out_format;
 };