After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 164870 - PNM [en,de]coder
PNM [en,de]coder
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal enhancement
: 0.10.18
Assigned To: Sebastian Dröge (slomo)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-01-21 23:40 UTC by Lutz Mueller
Modified: 2010-02-09 09:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
PNM [en,de]coder (13.96 KB, application/x-compressed-tar)
2005-01-21 23:51 UTC, Lutz Mueller
  Details
PNM plugin (16.33 KB, application/x-compressed-tar)
2005-01-24 23:30 UTC, Lutz Mueller
  Details
Makefile.am (276 bytes, text/plain)
2009-09-08 20:56 UTC, Lutz Mueller
  Details
gstpnm.c (2.94 KB, text/plain)
2009-09-08 20:56 UTC, Lutz Mueller
  Details
gstpnmdec.c (6.18 KB, text/plain)
2009-09-08 20:58 UTC, Lutz Mueller
  Details
gstpnmdec.h (1.41 KB, text/plain)
2009-09-08 20:58 UTC, Lutz Mueller
  Details
gstpnmenc.c (5.68 KB, text/plain)
2009-09-08 20:58 UTC, Lutz Mueller
  Details
gstpnmenc.h (1.41 KB, text/plain)
2009-09-08 20:59 UTC, Lutz Mueller
  Details
gstpnmutils.c (5.78 KB, text/plain)
2009-09-08 20:59 UTC, Lutz Mueller
  Details
gstpnmutils.h (1.60 KB, text/plain)
2009-09-08 20:59 UTC, Lutz Mueller
  Details
gstpnm.c (2.95 KB, text/plain)
2009-09-09 19:18 UTC, Lutz Mueller
  Details
gstpnmutils.h (2.39 KB, text/plain)
2009-09-09 19:19 UTC, Lutz Mueller
  Details
gstpnmutils.c (6.52 KB, text/plain)
2009-09-09 19:20 UTC, Lutz Mueller
  Details
gstpnmenc.h (1.54 KB, text/plain)
2009-09-09 19:21 UTC, Lutz Mueller
  Details
gstpnmdec.h (1.58 KB, text/plain)
2009-09-09 19:22 UTC, Lutz Mueller
  Details
gstpnmdec.c (5.67 KB, text/x-csrc)
2009-09-09 19:24 UTC, Lutz Mueller
  Details
gstpnmenc.c (5.08 KB, text/plain)
2009-09-09 19:26 UTC, Lutz Mueller
  Details
Partially fix decoder: (1.62 KB, patch)
2009-09-12 18:56 UTC, Lutz Mueller
none Details | Review
Partial fix (1.62 KB, patch)
2009-09-12 19:01 UTC, Lutz Mueller
none Details | Review
More fixes (8.83 KB, patch)
2009-09-12 20:01 UTC, Lutz Mueller
committed Details | Review

Description Lutz Mueller 2005-01-21 23:40:53 UTC
There is no native PNM [en,de]coder yet.
Comment 1 Lutz Mueller 2005-01-21 23:51:07 UTC
Created attachment 36358 [details]
PNM [en,de]coder

This is a skeleton of a PNM [en,de]coder for GStreamer. The decoder should
basically work. 

I need help. Can someone tell me how to pass images that are split onto several
buffers to a pipeline so that they can be processed by the png, jpeg or
ximagesink plugins? Is there something like a collector plugin? How am I
supposed to pass a 100-MB-file around?
Comment 2 Lutz Mueller 2005-01-24 23:30:19 UTC
Created attachment 36488 [details]
PNM plugin

The PNM [en,de]coder now works with image/x-image-pixmap (PPM) files. Some work
is still needed for PGM and PBM. 

One question remains: How am I supposd to move large images around? The JPEG
and PNG encoders for example only work if I supply the whole image in one
buffer. Is this behaviour intended?
Comment 3 Ronald Bultje 2005-01-25 09:57:55 UTC
Yes, that is expected.

The jpeg, png codecs get/put information in containers, e.g. AVI or Quicktime.
One buffer is always one buffer in those cases. You can also use multifilesrc to
make a new stream from a series of images. So yes, whole images are buffers in
those cases. Now, as for your plugin, it depends on what you do. It is logical
to assume that one buffer is one image, because that's how all other formats
(except MPEG) work. Buffer size should not be an issue on modern computers. Is
there a specific reason you'd want something otherwise?
Comment 4 Lutz Mueller 2005-01-25 19:25:06 UTC
Converting a PostScript file into video/x-raw-rgb usually yields huge files that
I don't want/need to have in memory.

I am exploring the possibility to use gstreamer as a generic file conversion
util. My target: Drop any file onto the gnome-print preview and get it added there. 
Comment 5 Ronald Bultje 2005-01-25 20:06:01 UTC
ah, ok. How about parsing small bits in buffers of, say, one page? That'll
require a postscript parser, which can't be too hard to write.
Comment 6 Lutz Mueller 2005-01-25 20:45:33 UTC
One page can be as large as 1200 dpi ^ 2 * 21 * 29 / 2.54^2 * 3 = 400 MB. I
don't want to have that in RAM.

I already wrote the postscript plugin (#162137).

Comment 7 Ronald Bultje 2005-01-27 10:08:52 UTC
that's a good point... So how else would you want to do it? What's your purpose?
for example, if you're going to use this for display (e.g. print preview), you
will need one buffer for a page. Scaled, of course, but the scaler needs it too.
Unless you adapt the scaler.

You can use smaller buffers, but you'll need to be prepared for some elements
not handling that correctly, so you won't be able to use all existing GStreamer
elements on the raw data.
Comment 8 Christian Fredrik Kalager Schaller 2005-11-10 10:31:32 UTC
Lutz are you still working on these plugins? Also at this point porting them to
0.9 would be natural as 0.10 is meant to go out next month and at that point 0.8
will quickly starting fading into obscurity.
Comment 9 Lutz Mueller 2005-11-10 22:04:02 UTC
I'd love to continue work on these plugins, but I was under the impression that
those new plugins were not welcome (tar #303975, bz2 #303167, pnm #164870,
postscript #162137, pdf...). It seemed like nobody was interested to add
non-audio/video functionalities to gstreamer. Andy Wingo proposed to add a
gstreamer-plugins-unmaintained module for my stuff (#303975), but that was never
done (as far as I know).

I'd like to continue to work on those modules. But I'd like to do that in CVS.
Comment 10 Andy Wingo 2006-01-27 17:18:15 UTC
Hi Lutz,

See the documentation about the plugin split: http://gstreamer.freedesktop.org/documentation/splitup.html

-bad is not a pejorative, it's just in with the Eastwood theme :)
Comment 11 Sebastian Dröge (slomo) 2009-07-18 06:55:54 UTC
Lutz, are you going to port this plugin to 0.10?
Comment 12 Lutz Mueller 2009-09-08 20:56:28 UTC
Created attachment 142728 [details]
Makefile.am
Comment 13 Lutz Mueller 2009-09-08 20:56:57 UTC
Created attachment 142729 [details]
gstpnm.c
Comment 14 Lutz Mueller 2009-09-08 20:58:12 UTC
Created attachment 142730 [details]
gstpnmdec.c
Comment 15 Lutz Mueller 2009-09-08 20:58:27 UTC
Created attachment 142731 [details]
gstpnmdec.h
Comment 16 Lutz Mueller 2009-09-08 20:58:45 UTC
Created attachment 142732 [details]
gstpnmenc.c
Comment 17 Lutz Mueller 2009-09-08 20:59:00 UTC
Created attachment 142733 [details]
gstpnmenc.h
Comment 18 Lutz Mueller 2009-09-08 20:59:15 UTC
Created attachment 142734 [details]
gstpnmutils.c
Comment 19 Lutz Mueller 2009-09-08 20:59:29 UTC
Created attachment 142735 [details]
gstpnmutils.h
Comment 20 Sebastian Dröge (slomo) 2009-09-09 14:21:46 UTC
General:
- Add your copyright to the header of all files

gstpnm.c:
- shared-mime-info uses image/x-portable-anymap as mimetype for PNM images, maybe use that in your plugin too and differentiate between grayscale, etc by cap fields
- The extension of PNM files seems to be pnm, not ps
- You could as well make a patch against gst-plugins-base/gst/typefindfunctions for the PNM typefinder :)


gstpnmdec.c:
- Use GST_BOILERPLATE
- Why don't you just store pointers to the src/sinkpads in the instance struct?
- Move the instance/class struct and all the GST_FOO_BAR_GET_STUFF() macros to the header. Otherwise gtk-doc is unhappy :)
- You might want to use a GstAdapter instead of the buffer-copy-merging stuff


gstpnmenc.c:
- Why do you only write the header if the buffer has no offset?
- BOILERPLATE, src/sinkpads pointers, instance/class structs stuff from gstpnmdec.c applies here too
- You could as well use gst_structure_get_int() instead of getting the value and then the int from the value

gstpnmutils.[ch]:
- Add license/copyright
Comment 21 Lutz Mueller 2009-09-09 19:18:22 UTC
Created attachment 142818 [details]
gstpnm.c

Fixed extension. I don't know how I could move the typefinding stuff elsewhere without duplicating code. And I don't like that.
Comment 22 Lutz Mueller 2009-09-09 19:19:51 UTC
Created attachment 142819 [details]
gstpnmutils.h

Added copyright header. Added image/x-image-anymap.
Comment 23 Lutz Mueller 2009-09-09 19:20:52 UTC
Created attachment 142820 [details]
gstpnmutils.c

Added copyright header.
Comment 24 Lutz Mueller 2009-09-09 19:21:37 UTC
Created attachment 142821 [details]
gstpnmenc.h

Move the struct definition here.
Comment 25 Lutz Mueller 2009-09-09 19:22:00 UTC
Created attachment 142822 [details]
gstpnmdec.h

Move the struct definition here.
Comment 26 Lutz Mueller 2009-09-09 19:24:30 UTC
Created attachment 142823 [details]
gstpnmdec.c

Use GST_BOILERPLATE. I don't want to use GstAdapter because this would not increase readability. (It doesn't really fit and would need more code.)
Comment 27 Lutz Mueller 2009-09-09 19:26:46 UTC
Created attachment 142824 [details]
gstpnmenc.c

Use GST_BOILERPLATE. Write header for each received frame. Use gst_structure_get_int. Add support for image/x-image-anymap.
Comment 28 Lutz Mueller 2009-09-09 19:32:01 UTC
I didn't store the pointers to the src/sinkpads in the instance struct because I wanted to keep the code easy to read by using a minimum of variables. But I should probably prefer performance instead...

That being said, I couldn't get the plugins to work after these modifications (-> not-negociated). I don't really understand why.
Comment 29 Sebastian Dröge (slomo) 2009-09-10 06:55:47 UTC
commit 1678e89301d6e72df3be7668a758d10ed8f6cfd5
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Sep 10 08:54:23 2009 +0200

    pnm: Mark PNM plugin as experimental because it doesn't work well yet

commit 58a5e422b6da15431121763c6530a2ec6599eda5
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Sep 10 08:53:46 2009 +0200

    pnmdec: Partially fix negotiation issues and refcount leaks

commit cfb04acde586ef5c242ddb4ff26545c2784b8d2e
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Sep 10 08:48:12 2009 +0200

    pnmenc: Fix negotiation issues and refcount leaks

commit 77e989f4985173c36c6a9ad600c64564af83fb24
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Thu Sep 10 08:29:06 2009 +0200

    pnm: Fix typefinder caps

commit 59b6d933da390d7954f1f0a18d5708fc65a3689d
Author: Lutz Mueller <lutz@topfrose.de>
Date:   Thu Sep 10 08:23:22 2009 +0200

    pnm: Add PNM encoder and decoder elements
    
    Fixes bug #164870.
Comment 30 Sebastian Dröge (slomo) 2009-09-10 06:59:08 UTC
I've committed all your stuff now and cleaned it a bit. It's still broken though, thus I've marked it as experimental for now ;)

First thing is the encoder, it creates files that eog for example can't display properly. I guess that's related to the fact, that the caps are always BGRx, different PNM formats are chosen depending on the srcpad peer caps, and that PNM simply doesn't like BGRx but something else. Please fix that :) I guess you want the srcpad template caps to contain some RGB format(s) and video/x-raw-gray for the grayscale PNM format. And then, depending on srcpad caps choose the output format and all that.

Second thing is the decoder, it also has this caps issue (at least). Depending on input caps and width/height from the file it should set correct output caps. And apart from that it never ever pushes data downstream for me ;)
Comment 31 Lutz Mueller 2009-09-12 18:56:11 UTC
Created attachment 143071 [details] [review]
Partially fix decoder:
Comment 32 Lutz Mueller 2009-09-12 19:01:22 UTC
Created attachment 143072 [details] [review]
Partial fix

Changed output format for decoder: PNM is RGB. As ximagesink doesn't accept RGB, I added ffmpegcolorspace in the example pipeline. However, things still don't work: For an image of width=313 x height=110 (x 3 = size=103290), the following warning is displayed: WARNING **: ffmpegcsp0: size 103290 is not a multiple of unit size 103400. I have really no idea where the number 103400 (= 103290 + 110) stems from. Any guesses?
Comment 33 Lutz Mueller 2009-09-12 20:01:47 UTC
Created attachment 143080 [details] [review]
More fixes

(1) Fix examples.
(2) Add support for gray images.
(3) Remove "use_fixed_caps" which doesn't seem to be useful.
(4) Do proper negotiation in the encoder.
(5) Fix memleak in the setcaps function in the encoder.
(6) Keep a link to the src pad in the encoder now that we need it more often.

Encoder works (at least the example pipeline). Decoder doesn't work (see my last comment).
Comment 34 Sebastian Dröge (slomo) 2009-09-13 09:10:54 UTC
Thanks, I'll go through all this patch later :)

(In reply to comment #32)
> Created an attachment (id=143072) [details]
> Partial fix
> 
> Changed output format for decoder: PNM is RGB. As ximagesink doesn't accept
> RGB, I added ffmpegcolorspace in the example pipeline. However, things still
> don't work: For an image of width=313 x height=110 (x 3 = size=103290), the
> following warning is displayed: WARNING **: ffmpegcsp0: size 103290 is not a
> multiple of unit size 103400. I have really no idea where the number 103400 (=
> 103290 + 110) stems from. Any guesses?

Yes, our RGB (not xRGB right? i.e. 3 bytes per pixel, first byte is red) has a rowstride the is rounded up to the next integer multiply of 4.

So the correct size for GStreamer RGB would be:
GST_ROUND_UP_4(width * 3) * height. Because your width was 313 it would be (313 * 3 + 1) * 110 == 103400. GStreamer doesn't support random rowstrides (yet) so you have to convert that internally.

And you should do this for the encoder and decoder of course, otherwise you're created PNM files are broken for non-GStreamer use.
Comment 35 Sebastian Dröge (slomo) 2009-09-13 17:18:19 UTC
Ok, committed your patch and fix to the caps handling (to allow RGB again in the encoder and video/x-raw-gray with 8bpp has no endianness field).

commit 751843ff86a2b3caa97a4c7cb175e44f70735ca3
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Sun Sep 13 19:17:15 2009 +0200

    pnm: Fix caps
    
    8 bit grayscale caps have no endianness field and the caps name
    of GST_VIDEO_CAPS_RGB is still "video/x-raw-rgb" and not GST_VIDEO_CAPS_RGB.

commit 7f3e6f4965093f9a17e5c4e29f1bd9027ae953b8
Author: Lutz Mueller <lutz@topfrose.de>
Date:   Sun Sep 13 19:13:24 2009 +0200

    pnm: Lots of bugfixes
    
    (1) Fix examples.
    (2) Add support for gray images.
    (3) Remove "use_fixed_caps" which doesn't seem to be useful.
    (4) Do proper negotiation in the encoder.
    (5) Fix memleak in the setcaps function in the encoder.
    (6) Keep a link to the src pad in the encoder now that we need it more often
    
    Partially fixes bug #164870.
Comment 36 Sebastian Dröge (slomo) 2009-09-13 17:35:35 UTC
Ok, I've added the rowstride conversion now.

commit f09b1adf707747e96e95b5cc0ff69178a3dab738
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Sun Sep 13 19:33:57 2009 +0200

    pnm: Correctly convert from/to GStreamer rowstride




Some additional questions though. What's the difference between the _RAW and _ASCII types? And what exactly is the bitmap type? Is it really just grayscale?

And please open new bugs for any new patches, and better attach many small patches instead of one large :)
Comment 37 Lutz Mueller 2009-09-14 22:05:32 UTC
As you requested, work continues elsewhere (#595215).