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 690639 - pngdec: decoded frames are not pushed out
pngdec: decoded frames are not pushed out
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Mac OS
: Normal normal
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-12-22 11:16 UTC by Thijs Vermeir
Modified: 2013-02-12 22:08 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
videoparsers: Add png file parser (9.78 KB, patch)
2013-01-29 03:43 UTC, Olivier Crête 		none Details | Review
videoparsers: Add png file parser (9.75 KB, patch)
2013-01-29 04:14 UTC, Olivier Crête 		committed Details | Review

Description Thijs Vermeir 2012-12-22 11:16:41 UTC 
pngdec is not producing any output frames if read from file.

This pipeline works:
gst-launch-1.0 videotestsrc num-buffers=1 ! pngenc ! pngdec ! filesink location=frame.raw

This does not work:
gst-launch-1.0 videotestsrc num-buffers=1 ! pngenc ! filesink location=img.png
gst-launch-1.0 filesrc location=img.png ! pngdec ! filesink location=frame.raw
Comment 1 Sebastian Dröge (slomo) 2012-12-26 09:59:55 UTC 
Does this also fail if you use a large enough blocksize to include the complete file in a single buffer? pngdec requires one png image per buffer.
Comment 2 Thijs Vermeir 2012-12-26 10:41:12 UTC 
It does work if the block size is increased to the file size. pngdec is indeed overwriting the current video frame on every incoming "frame".
Comment 3 Sebastian Dröge (slomo) 2012-12-26 10:54:56 UTC 
Ok, that's expected and not a bug then. You must pass exactly one image per buffer to pngdec. For your use case a png parser would be useful.
Comment 4 Thijs Vermeir 2012-12-30 22:32:52 UTC 
I don't know if this is expected behaviour, I believe this belongs to the element itself. With the GstVideoDecoder base class it could be in the parse vfunction and/or libpng already does this internally too...
Comment 5 Tim-Philipp Müller 2012-12-31 00:41:24 UTC 
I think a patch for pngdec would be desirable.
Comment 6 Olivier Crête 2013-01-29 01:01:11 UTC 
Yea, we need a parser... filesrc ! typefind ! pngdec fails.. this is pretty bad
Comment 7 Olivier Crête 2013-01-29 03:43:23 UTC 
Created attachment 234695 [details] [review]
videoparsers: Add png file parser
Comment 8 Olivier Crête 2013-01-29 04:14:43 UTC 
Created attachment 234696 [details] [review]
videoparsers: Add png file parser

Little oops in last iteration
Comment 9 Sebastian Dröge (slomo) 2013-02-11 11:34:52 UTC 
Review of attachment 234696 [details] [review]:

Looks good after a short look

::: gst/videoparsers/gstpngparse.c
@@ +108,3 @@
+    goto beach;
+
+  if (startcode != G_GUINT64_CONSTANT (0x89504E470D0A1A0A)) {

Maybe use constants for this?

@@ +125,3 @@
+        goto beach;
+
+      if (startcode == G_GUINT64_CONSTANT (0x89504E470D0A1A0A)) {

...and this
Comment 10 Nicolas Dufresne (ndufresne) 2013-02-12 21:46:51 UTC 
Review of attachment 234696 [details] [review]:

::: gst/videoparsers/gstpngparse.c
@@ +33,3 @@
+GST_STATIC_PAD_TEMPLATE ("src", GST_PAD_SRC,
+    GST_PAD_ALWAYS,
+    GST_STATIC_CAPS ("image/png, width = (int)[1, MAX], height = (int)[1, MAX]")

You should add parsed = TRUE
Comment 11 Olivier Crête 2013-02-12 22:08:14 UTC 
Comment on attachment 234696 [details] [review]
videoparsers: Add png file parser

Modified per comments and comitted
Comment 12 Olivier Crête 2013-02-12 22:08:41 UTC 
commit 41afff88e01bf54d6c6386deb3d173a22a1a87e2
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Mon Jan 28 22:42:44 2013 -0500

    videoparsers: Add png file parser
    
    https://bugzilla.gnome.org/show_bug.cgi?id=690639