Tested it by below pipeline

gst-launch-1.0 filesrc location=PNMTest.pnm ! pnmdec samedimension=TRUE ! filesink location=out.bin

Review of attachment 276118 [details] [review]:

While this patch doesn't address an issue, there is room for improvement here in pnmdec. What needs to be done is check if downstream has a 'rowstride' field in its caps and use that. Otherwise just use rowstride = width.

Another thing I noticed is that it is using buffer appends until it gets a full frame, it would be more resilient if it used a GstAdapter and only retrieved a buffer of an expected size.

::: gst/pnm/gstpnmdec.c
@@ +106,3 @@
 {
   /* Need to convert from PNM rowstride to GStreamer rowstride */
+  if ((s->mngr.info.width % 4 != 0) && (s->dimension == FALSE)) {

This is actually only increasing the image to have a rowstride that is a multiple of 4. The final buffer caps is still of the input width and should work as expected.

I don't think we have a restriction of having a multiple of 4 rowstride, it should actually be respecting whatever downstream can do if it provides a rowstride value in the caps.

Created attachment 276915 [details] [review]
pngdec: Patch to get pnm decoder output without stride alignment

At present pnmdec always aligns rowstride to 4 and then returns output buffer. This patch is to get the output without any rowstride alignment

Thanks for your review. At present pnmdec always aligns rowstride to 4 and then returns this buffer. If width is say 1919, considering bits per pixel of 3 rowstride becomes 1919 * 3 = 5757. This at present is getting adjusted to 5760 (multiple of 4) and then buffer gets returned. Sorry for confusing by saying width becomes 1920 (5760/4 = 1920). I have updated the patch to take an input parameter rowstrideorig. If this is set to TRUE then pnmdec will not align rowstride to 4 but will return the original (width * bpp) number of bytes.
If no parameter is passed then the behavior is same as before. That is it will make the rowstride alignment to 4 and return. This is more of an enhancement.

I have tested the decoder by using following commands and it is working well.

gst-launch-1.0 filesrc location=odd.pnm ! pnmdec rowstrideorig=TRUE ! filesink location=out.raw

gst-launch-1.0 filesrc location=odd.pnm ! pnmdec ! filesink location=out.raw

I will create separate patches for your suggestions to use GstAdapter.

Review of attachment 276915 [details] [review]:

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.

This is the way data is handled in gstreamer. I did not have the understanding then. So closing it.