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 795117 - jpegparse: Fix the code to handle APP1 marker segment skipping
jpegparse: Fix the code to handle APP1 marker segment skipping
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.14.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 793787
 
 
Reported: 2018-04-10 00:56 UTC by sreerenj
Modified: 2018-04-16 21:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
jpegparse: Fix APP1 marker segment parsing (951 bytes, patch)
2018-04-10 00:58 UTC, sreerenj
committed Details | Review

Description sreerenj 2018-04-10 00:56:24 UTC
The Jpegparse code is not repositioning the bytereader before skipping and that causes errors in jpeg decode pipelines.

Streams are attached here: https://bugzilla.gnome.org/show_bug.cgi?id=793787
Comment 1 sreerenj 2018-04-10 00:58:43 UTC
Created attachment 370715 [details] [review]
jpegparse: Fix APP1 marker segment parsing
Comment 2 Víctor Manuel Jáquez Leal 2018-04-10 08:10:43 UTC
Review of attachment 370715 [details] [review]:

::: gst/jpegformat/gstjpegparse.c
@@ +575,3 @@
+    /* restore the byte position and size */
+    reader->size += 2;
+    reader->byte -= 2;

rather than modify the internal state of the reader why not just skip the reported marker size, something like

    GST_DEBUG_OBJECT (parse, "unhandled marker %x skiping %u bytes", APP1,
        size);
    if (!gst_byte_reader_skip (reader, size))
      return FALSE;
Comment 3 sreerenj 2018-04-13 01:15:52 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #2)
> Review of attachment 370715 [details] [review] [review]:
> 
> ::: gst/jpegformat/gstjpegparse.c
> @@ +575,3 @@
> +    /* restore the byte position and size */
> +    reader->size += 2;
> +    reader->byte -= 2;
> 
> rather than modify the internal state of the reader why not just skip the
> reported marker size, something like
> 
>     GST_DEBUG_OBJECT (parse, "unhandled marker %x skiping %u bytes", APP1,
>         size);
>     if (!gst_byte_reader_skip (reader, size))
>       return FALSE;

No, it won't work, the immediate call to
gst_jpeg_parse_skip_marker() has to retrieve the size of the header from the first two bytes.
Comment 4 sreerenj 2018-04-16 21:40:37 UTC
Review of attachment 370715 [details] [review]:

Pushed as, commit 9a090538be874d1d5031b1a225e8ee9d767603c3