[Bug 731943] pnmenc: PNM Encoder needs to use gstVideoEncoder as base class

[GStreamer (bugzilla.gnome.org)]

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

Thiago Sousa Santos <thiagossantos> changed:

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

--- Comment #1 from Thiago Sousa Santos <thiagossantos at gmail.com> 2014-06-25 17:34:19 UTC ---
Review of attachment 278813:
 --> (https://bugzilla.gnome.org/review?bug=731943&attachment=278813)

Thanks for this patch, it is nearly ready. Just some minor nitpicks pointed
below.

::: gst/pnm/gstpnmenc.c
@@ +203,3 @@
+    size = size * 4;
+    size += strlen (header);
+

You can use one of the gst_video_encoder_allocate_output_*() functions here.

@@ +206,3 @@
+  } else {
+    size += strlen (header);
+    /* Per component 4 bytes are used in case of ASCII encoding */

Same here.

@@ +213,3 @@
+    goto done;
+  }
+  if (gst_buffer_map (frame->input_buffer, &imap, GST_MAP_READ) == FALSE) {

You should unmap the output_buffer here to avoid keeping it mapped.

@@ +226,3 @@
+      if (pnmenc->info.type == GST_PNM_TYPE_PIXMAP) {
+        o_rowstride = 3 * pnmenc->info.width;
+  memcpy (omap.data, header, strlen (header));

You can get the rowstride from the video format with
GST_VIDEO_FRAME_COMP_STRIDE IIRC.

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