gst-launch-1.0 -v filesrc location=some.jp2 ! decodebin ! imagefreeze ! autovideosink also doesn't seem to work - core dump

Do you have a repro sample (or a pipeline to create one) ?

Thanks, Vincent. You can find a sample JP2 image here:

https://github.com/GrokImageCompression/grok-test-data/blob/master/input/conformance/zoo1.jp2

This repo is full of jp2 and j2k test images.

Verifying that

gst-launch-1.0 -v filesrc location=some.ppm ! decodebin ! imagefreeze ! autovideosink 

works fine on my system, but


gst-launch-1.0 -v filesrc location=some.ppm ! openjpegenc ! openjpegdec ! imagefreeze ! autovideosink 

fails with the same error as above

As far as I can tell, the reason is that openjpegdec specifically asks to only get used if the media type of the incoming data is known (gst_video_decoder_set_needs_format (decoder, TRUE);). With filesrc, it is not, since it's ANY caps. I traced into the code thinking it was a bug somewhere which was skipping negotiation, but I now think it's as intended. If a typefind element is inserted after filesink, that error is now gone, though openjpegdec fails to decode the data (I'm not sure whether it's supposed to be able to decode that particular variant).


I pushed a couple things I found while investigating too:

commit 903684aa78787f90ba95355ebc90d58bb31ce7a0
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Wed Jun 7 16:17:50 2017 +0100

    openjpeg: guard against invalid memory access on crafted files

commit e5a0c4a751383145d6a5da4e1912d21f41c5e575
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Mon Jun 5 15:31:52 2017 +0100

    jpeg2000sampling: fix critical when sampling is missing from caps
    
    This can happen with real files

Thanks, Vincent! I am still seeing some issues:

So, here is the status of this bug now:

1)  

gst-launch-1.0 -v filesrc location=some.jp2 ! decodebin ! imagefreeze ! autovideosink

coredumps


2)  gst-launch-1.0 -v filesrc location=some.jp2 ! typefind ! jpeg2000parse ! openjpegdec ! imagefreeze ! autovideosink

Errors with  "jpeg2000parse: unable to get current caps.


3) gst-launch-1.0 -v filesrc location=some.j2k ! typefind ! openjpegdec ! imagefreeze ! autovideosink

Errors with "Could not determine type of stream".


4) Take the following ppm file:

https://github.com/GrokImageCompression/grok-test-data/blob/master/input/nonregression/Bretagne1.ppm

and compress it to jp2 using the following command line:

$ opj_compress -i Bretagn1.ppm -o Bretagne1.jp2 -OutFor jp2


Now run:


 gst-launch-1.0 -v filesrc location=Bretagne1.jp2 ! typefind ! openjpegdec ! imagefreeze ! autovideosink


The image does display, but there are severe artifacts in the image: vertical black lines.


I'm really opening up a can of worms here :)  Thanks for your help on this, appreciate it.

I don't get crashes, or images with artifacts. With the sample file in the post above, I also get failure to decode. I have openjpeg2, though, and a opj2_compress tool, not opj_compress. That might by why. Fedora doesn't seen to have opj_compress, but I could go find the source if needed. Is that version 2 a newer version, or a different API which could be installed alongside ?

Hmmmmm, very strange. openjpeg2 is the latest version.

I am using Ubuntu 16.04, with latest master version of gstreamer.

For .j2k images, I guess this is not a recognized file type in gstreamer.
But, I also tested with .jpc, and typefind does not recognize this type.

Created attachment 353466 [details]
Screenshot of jp2 artifacts

On the left is the original image, while on the right is the image using the following pipeline

 gst-launch-1.0 -v filesrc location=Bretagne1.jp2 ! typefind ! openjpegdec ! imagefreeze ! autovideosink

I found some serious bugs in 

fill_frame_packed8_3 (GstVideoFrame * frame, opj_image_t * image)

in openjpegdec

Vincent - did you use the pipeline above with an RGB 8 bit jp2 image and it looked correct ?

By the way, I am running this on a VM.  Could this be the reason why display is messed up?

 gst-launch-1.0 -v filesrc location=Bretagne1.jp2 ! typefind ! openjpegdec ! imagefreeze ! autovideosink


I added some test code to store the video buffer to disk before decoding.

I am finding, on my Ubuntu VM, that the video buffer is corrupt before 
it gets to the decoder.

I am attaching before and after for a grayscale image.

Created attachment 353504 [details]
Input image decoded to tif

Created attachment 353505 [details]
Output buffer before openjpegdec has a chance to decode (decoded to ppm afterwards)

I found that problem: filesrc sends the file in chunks, and openjpegdec is expecting the whole file in a single chunk.

Is there a way of increasing the blocksize in filesrc? Looking at the code, I don't see how to do it.

(In reply to Aaron Boxer from comment #10)

> Vincent - did you use the pipeline above with an RGB 8 bit jp2 image and it
> looked correct ?


No. As posted above:

I don't get crashes, or images with artifacts. With the sample file in the post above, I also get failure to decode.

What you want is a parser between filesrc and openjpegdec, which will parse and send a frame at a time. There is a blocksize=N property in filesrc, but it would just use anohter constant size, which isn't what you want either.

Thanks! I think I have a fix for the parser, so it can at least handle
jpc files.

Created attachment 353512 [details] [review]
jpeg2000parse: parse when current caps are empty

We make a few guesses in this case: jpc media format, and color space based on number of components. Once we support jp2 format in parser, we will need to be a bit smarter about guessing the media format.

Another pipeline useful in testing these changes (j2c media format)

gst-launch-1.0 -v videotestsrc pattern=ball ! openjpegenc ! jpeg2000parse ! rtpj2kpay ! rtpj2kdepay ! openjpegdec ! videoconvert ! ximagesink

Created attachment 353572 [details] [review]
jpeg2000parse: parse when current caps are empty

In this patch, we parse the buffer to detect media type (j2c or jpc), rather than
assume jpc, as we did in previous patch.

Created attachment 353575 [details] [review]
jpeg2000parse: parse when current caps are empty

a few more little improvements

From my perspective, this patch is ready to merge.

Created attachment 354393 [details] [review]
Parse file when caps are null

*** Bug 783650 has been marked as a duplicate of this bug. ***

a polite <ping/> for this patch - complete, tested in my work flow, and working correctly.

Review of attachment 354393 [details] [review]:

::: gst/videoparsers/gstjpeg2000parse.c
@@ +516,3 @@
+    colorspace =
+        (numcomps >=
+        3) ? GST_JPEG2000_COLORSPACE_RGB : GST_JPEG2000_COLORSPACE_GRAY;

Why RGB and not YUV? :) This seems like it should print a warning at least. For all guesses that we make which are not necessarily correct.

@@ +670,3 @@
+    if (current_caps_struct) {
+      if (gst_structure_get_fraction (current_caps_struct, "framerate", &fr_num,
+              &fr_denom)) {

Merge these two ifs into one

Thanks for the review! Yes, warning is a good idea. My thinking is that if we don't have caps, then frame is probably coming from a filesrc element, in which case it is probably RGB.  Does that make sense ?

Created attachment 355041 [details] [review]
jpeg2000: allow parsing when caps are null

Assume that 4 components == RGBA, 3 components == RGB and 2 components == grayscale with alpha. User is warned about these assumptions.

commit 2cffa1579a1adeded10a9036876d729bbf6a32d1 (HEAD -> master)
Author: Aaron Boxer <boxerab@gmail.com>
Date:   Mon Jun 12 09:47:49 2017 -0400

    jpeg2000parse: allow parsing when current caps are null
    
    In this case, we assume that the format is jpc, and we infer the color
    space from the number of components. This allows the parser to process a
    jpc disk file coming from a filesrc element.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=783291