[Bug 729764] pnmdec: Not respecting downstream rowstride

[GStreamer (bugzilla.gnome.org)]

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

Thiago Sousa Santos <thiagossantos> changed:

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

--- Comment #5 from Thiago Sousa Santos <thiagossantos at gmail.com> 2014-05-21 14:28:12 UTC ---
Review of attachment 276915:
 --> (https://bugzilla.gnome.org/review?bug=729764&attachment=276915)

Please don't run gst-indent for .h files, only .c needs to be properly
indented.

See inline comments below. Please look at the code I pointed in the comments
below so you understand why this caps negotiation is important and how it
works. This can also help understanding:
http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/design/part-negotiation.txt

::: gst/pnm/gstpnmdec.c
@@ +49,3 @@
+{
+  GST_PNMDEC_PROP_0,
+  GST_PNMDEC_PROP_STRIDE

You shouldn't need a property for this, it should be done automatically: If
downstream requests a particular rowstride than this element will provide it.
Otherwise it should fallback to either rowstride=width or using a multiple of 4
to maintain backwards compatibility.

@@ +105,3 @@
 {
   /* Need to convert from PNM rowstride to GStreamer rowstride */
+  if ((s->mngr.info.width % 4 != 0) && (s->rowstride == FALSE)) {

This should not be controlled by a property, but from a variable read at the
beginning of the chain function (that updates every time a reconfigure event is
sent).

You can look at the "ensure_negotiated" function from gstgoom.c in
gst-plugins-good for an example. It will check if needs to reconfigure and then
queries downstream caps and checks for the properties it needs to know. For
pnmdec it would need to check that width and height match and also look if
there is any rowstride field to use that instead of always using a multiple of
4.

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