GNOME Bugzilla – Bug 707447
The size of image is small when playing some jpeg via gst-launch
Last modified: 2013-09-20 16:35:38 UTC
Overview: The size of image is small when playing some jpeg via gst-launch ========================================================================================== Steps to Reproduce: user@user: gst-launch-1.0 playbin uri=file:///home/user/Pictures/Default.jpg Setting pipeline to PAUSED ... Pipeline is PREROLLING ... libva info: VA-API version 0.34.0 libva info: va_getDriverName() returns 0 libva info: Trying to open /opt/gst_git/lib/dri/i965_drv_video.so libva info: Found init function __vaDriverInit_0_34 libva info: va_openDriver() returns 0 Pipeline is PREROLLED ... Setting pipeline to PLAYING ... New clock: GstSystemClock Got EOS from element "playbin0". Execution ended after 0:00:00.095124597 Setting pipeline to PAUSED ... Setting pipeline to READY ... Setting pipeline to NULL ... Freeing pipeline ... ========================================================================================== Actual results: The picture is small than the one displayed by image viewer ========================================================================================== Expected results: The picture is just the same size of the one display by image viewer
Created attachment 254047 [details] [review] Fix the bug of the wrong size caculation of segment App0, Appn Codecparser: jpeg: Fix the bug of the wrong size caculation of segment App0, Appn
Created attachment 254048 [details] [review] Change the way of calculation of the size of a jpeg image jpeg: Change the way of calculation of the size of a jpeg image, originally, it regards the size between two SOI segment, but it will fail if Appn segment contains SOI. Now it regards the size between the SOI and the corresponding EOI.
Created attachment 254049 [details] The jpeg file which makes the bug appear. The jpeg file which makes the bug appear.
Created attachment 254160 [details] [review] Handle the issue of decoding the jpeg image when there's subimage contained in Appn segment The original way of calculating the size of a jpeg image is like this: first find a SOI segments which indicates the start of an image, then try to find the second one, if the second one is found, then subtract their offset to get the size, if the second one is not found, then the size is the same as the file size. So if Appn segment contains a subimage, the second SOI is the subimage's SOI, not the next jpeg image's. Here the program tries to find the corresponding EOI of the SOI to get the size of a jpeg image. It is tested successfully both for jpeg and M-JPEG.
Created attachment 254164 [details] [review] codeparser: jpeg: Fix the bug of the wrong size caculation of segment App0 and Appn In function gst_jpeg_parse in gstjpegparser.c, the length caculation of segments is confusing, the SOI and EOI is two bytes long, which is the whole size of the segment, but the lengths of other segments like App0, Appn, DQT, DHT do not include their two bytes headers. So in function decode_buffer in gstvaapidecoder_jpeg.c, when searching for the next segments header in while loop, it actually begins two bytes ahead of the actual offset where the next segment begins, normally this two bytes are not segment header, but sometimes when there is subimage in Appn segment, the last two byte of Appn is EOI, and the next two bytes are the segment header of DQH or another Appn, that causes an wrong segment detection. To fix this bug, I modify the gst_jpeg_parse function to make "size" indicate the whole size of the segment, and also modify the decode_buffer function to pass the proper size to other function there.
Created attachment 254558 [details] [review] Handle the issue of decoding the jpeg image when there's subimage contained in Appn segment The original way of calculating the size of a jpeg image is like this: first find a SOI segments which indicates the start of an image, then try to find the second one, if the second one is found, then subtract their offset to get the size, if the second one is not found, then the size is the same as the file size. So if Appn segment contains a subimage, the second SOI is the subimage's SOI, not the next jpeg image's. Here the program tries to find the corresponding EOI of the SOI to get the size of a jpeg image. It is tested successfully both for jpeg and M-JPEG.
Created attachment 254559 [details] [review] codecparser: jpeg: Fix the bug of the wrong size caculation of segment App0 and Appn In function gst_jpeg_parse in gstjpegparser.c, the length caculation of segments is confusing, the SOI and EOI is two bytes long, which is the whole size of the segment, but the lengths of other segments like App0, Appn, DQT, DHT do not include their two bytes headers. So in function decode_buffer in gstvaapidecoder_jpeg.c, when searching for the next segments header in while loop, it actually begins two bytes ahead of the actual offset where the next segment begins, normally this two bytes are not segment header, but sometimes when there is subimage in Appn segment, the last two byte of Appn is EOI, and the next two bytes are the segment header of DQH or another Appn, that causes an wrong segment detection. To fix this bug, I modify the gst_jpeg_parse function to make seg->size indicate the size without segment header, and also modify the decode_buffer function to calculate the correct offset and size.
Review of attachment 254559 [details] [review]: --- a/gst-libs/gst/vaapi/gstvaapidecoder_jpeg.c +++ b/gst-libs/gst/vaapi/gstvaapidecoder_jpeg.c @@ -537,7 +537,7 @@ decode_buffer(GstVaapiDecoderJpeg *decoder, const guchar *buf, guint buf_size) GST_DEBUG("buffer to short for parsing"); return GST_VAAPI_DECODER_STATUS_ERROR_NO_DATA; } - ofs += seg.size; + ofs = seg.offset + seg.size; consider the meaning of seg.offset here, previous ofs is treated as start point, right? so, the seg.offset in gstjpegparser.c seems incorrect: line 561: " seg->offset = offset + gst_byte_reader_get_pos (&br);" should it change to "seg->offset = gst_byte_reader_get_pos (&br);" ? line 605: " seg->size = gst_byte_reader_get_pos (&br) - seg->offset;"
All the offsets are calculated from buf. In decode_buffer, seg.offset is the start point without segment header, seg.size is the size without segment header, ofs is the start point with segment header. In gst_jpeg_parse, gst_byte_reader_gst_pos(&br) return value is always 2, because it inits from offset in gst_byte_reader_init(&br, &data[offset], size), so I think it shouldn't be modified as seg->offset = gst_byte_reader_get_pos(&br); As for line 605, in my jpeg test, line 605 is never executed, precisely, I think it should be modified to seg->size = gst_byte_reader_get_pos(&br) - seg->offset -2 if I assume the segment header is always 2 bytes long. (In reply to comment #8) > Review of attachment 254559 [details] [review]: > > --- a/gst-libs/gst/vaapi/gstvaapidecoder_jpeg.c > +++ b/gst-libs/gst/vaapi/gstvaapidecoder_jpeg.c > @@ -537,7 +537,7 @@ decode_buffer(GstVaapiDecoderJpeg *decoder, const guchar > *buf, guint buf_size) > GST_DEBUG("buffer to short for parsing"); > return GST_VAAPI_DECODER_STATUS_ERROR_NO_DATA; > } > - ofs += seg.size; > + ofs = seg.offset + seg.size; > > consider the meaning of seg.offset here, previous ofs is treated as start > point, right? > so, the seg.offset in gstjpegparser.c seems incorrect: > > line 561: " seg->offset = offset + gst_byte_reader_get_pos (&br);" > should it change to "seg->offset = gst_byte_reader_get_pos (&br);" ? > line 605: " seg->size = gst_byte_reader_get_pos (&br) - seg->offset;"
I mistake reading "+" to "+=" in "+ ofs = seg.offset + seg.size;"
The codecparser part is incomplete. The existing implementation of gst_jpeg_parse() does indeed *not* match the documentation in the corresponding .h file. The seg->size in the default: clause is offset by 2 bytes too. Ideally, jpeg_parse_to_next_marker() should also be fixed to actually move to the start of the segment, thus removing the +2 there, fixing the initial seg->offset (+2) in gst_jpeg_parse() and keep the default: clause as is.
Hi, (In reply to comment #4) > Created an attachment (id=254160) [details] [review] > Handle the issue of decoding the jpeg image when there's subimage contained in > Appn segment > > The original way of calculating the size of a jpeg image is like this: first > find a SOI segments which indicates the start of an image, then try to find the > second one, if the second one is found, then subtract their offset to get the > size, if the second one is not found, then the size is the same as the file > size. So if Appn segment contains a subimage, the second SOI is the subimage's > SOI, not the next jpeg image's. > Here the program tries to find the corresponding EOI of the SOI to get the size > of a jpeg image. It is tested successfully both for jpeg and M-JPEG. For this one, the better option is to use gst_jpeg_parse() in scan_for_start_code(). It's optimized to skip the exact number of bytes in a segment based on the Lx field. The next step is to optimize overall so that we have a parse sequence only once. Fixed locally for the first part.
Fixed in git master branch. Please re-open if this is still an issue.
commit 23c7dde4048c54c9af6ad1f2894a669cebcd1f1f Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com> Date: Wed Sep 18 15:56:52 2013 +0200 codecparsers: jpeg: fix calculation of segment size. The size of a marker segment is defined to be exclusive of any initial marker code. So, fix the size for SOI, EOI and APPn segments but also the size of any possible segment that is usually "reserved" or not explicitly defined. https://bugzilla.gnome.org/show_bug.cgi?id=707447 commit 9c29bbde58e1a9a343b8e973295c6d98bb8fed91 Author: Junfeng Xu <jun.feng.xu@intel.com> Date: Tue Sep 10 15:46:09 2013 +0800 jpeg: fix calculation of offset to next marker segment. Fix calculation of the offset to the next marker segment since the correction of the codecparser part to match the API specification. i.e. the GstJpegMarkerSegment.size field represents the size in bytes of the segment minus any marker prefix. https://bugzilla.gnome.org/show_bug.cgi?id=707447 Signed-off-by: Junfeng Xu <jun.feng.xu@intel.com> commit 77298beb15575131600867d6df2a3e85c3f1bc20 Author: Gwenole Beauchesne <gwenole.beauchesne@intel.com> Date: Wed Sep 18 17:59:44 2013 +0200 jpeg: fix determination of image bounds. Look for the exact image bounds characterised by the <SOI> and <EOI> markers. Use the gst_jpeg_parse() codec parser utility function to optimize the lookup for the next marker segment. https://bugzilla.gnome.org/show_bug.cgi?id=707447