diff mbox

[FFmpeg-devel] lavc/mjpegdec: hint next marker position in ff_mjpeg_find_marker

Message ID 20170127084718.GC30664@tsuri
State Superseded
Headers show

Commit Message

Matthieu Bouron Jan. 27, 2017, 8:47 a.m. UTC 
On Fri, Jan 27, 2017 at 02:30:40AM +0100, Michael Niedermayer wrote:
> On Thu, Jan 26, 2017 at 05:47:51PM +0100, Matthieu Bouron wrote:
> > Speeds up next marker search when a SOS marker is found.
> > 
> > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > sample_2992x4000.jpg
> > ---
> >  libavcodec/mjpegdec.c | 31 +++++++++++++++++++++++++------
> >  libavcodec/mjpegdec.h |  2 +-
> >  libavcodec/mxpegdec.c |  5 +++--
> >  3 files changed, 29 insertions(+), 9 deletions(-)
> 
> this breaks decoding multiple jpeg files
> for example tickets//856/nint.jpg

New patch attached fixing the issue.

[...]

Comments

Michael Niedermayer Jan. 28, 2017, 1:25 a.m. UTC | #1 
On Fri, Jan 27, 2017 at 09:47:18AM +0100, Matthieu Bouron wrote:
> On Fri, Jan 27, 2017 at 02:30:40AM +0100, Michael Niedermayer wrote:
> > On Thu, Jan 26, 2017 at 05:47:51PM +0100, Matthieu Bouron wrote:
> > > Speeds up next marker search when a SOS marker is found.
> > > 
> > > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > > sample_2992x4000.jpg
> > > ---
> > >  libavcodec/mjpegdec.c | 31 +++++++++++++++++++++++++------
> > >  libavcodec/mjpegdec.h |  2 +-
> > >  libavcodec/mxpegdec.c |  5 +++--
> > >  3 files changed, 29 insertions(+), 9 deletions(-)
> > 
> > this breaks decoding multiple jpeg files
> > for example tickets//856/nint.jpg
> 
> New patch attached fixing the issue.
> 
> [...]

>  mjpegdec.c |   31 +++++++++++++++++++++++++------
>  mjpegdec.h |    2 +-
>  mxpegdec.c |    5 +++--
>  3 files changed, 29 insertions(+), 9 deletions(-)
> 6ba49d73c189b197bdc1a983e26ccd49713c617c  0001-lavc-mjpegdec-hint-next-marker-position-in-ff_mjpeg_.patch
> From ebef63ebdc75872d52529d5b74b6d05254d4c0fd Mon Sep 17 00:00:00 2001
> From: Matthieu Bouron <matthieu.bouron@gmail.com>
> Date: Thu, 26 Jan 2017 15:17:36 +0100
> Subject: [PATCH] lavc/mjpegdec: hint next marker position in
>  ff_mjpeg_find_marker
> 
> Speeds up next marker search when a SOS marker is found.
> 
> ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> sample_2992x4000.jpg

the patch seems to work but
why is the "buf_ptr += (get_bits_count(&s->gb) + 7) / 8;" not
already skiping all that what the buf_hint does ?
is the difference the unescaping ?
maybe the buf_hint stuff can be simplified by adjusting buf_ptr instead
?

or maybe i misunderstand

thx

[...]
Matthieu Bouron Jan. 28, 2017, 11:59 a.m. UTC | #2 
On Sat, Jan 28, 2017 at 02:25:34AM +0100, Michael Niedermayer wrote:
> On Fri, Jan 27, 2017 at 09:47:18AM +0100, Matthieu Bouron wrote:
> > On Fri, Jan 27, 2017 at 02:30:40AM +0100, Michael Niedermayer wrote:
> > > On Thu, Jan 26, 2017 at 05:47:51PM +0100, Matthieu Bouron wrote:
> > > > Speeds up next marker search when a SOS marker is found.
> > > > 
> > > > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > > > sample_2992x4000.jpg
> > > > ---
> > > >  libavcodec/mjpegdec.c | 31 +++++++++++++++++++++++++------
> > > >  libavcodec/mjpegdec.h |  2 +-
> > > >  libavcodec/mxpegdec.c |  5 +++--
> > > >  3 files changed, 29 insertions(+), 9 deletions(-)
> > > 
> > > this breaks decoding multiple jpeg files
> > > for example tickets//856/nint.jpg
> > 
> > New patch attached fixing the issue.
> > 
> > [...]
> 
> >  mjpegdec.c |   31 +++++++++++++++++++++++++------
> >  mjpegdec.h |    2 +-
> >  mxpegdec.c |    5 +++--
> >  3 files changed, 29 insertions(+), 9 deletions(-)
> > 6ba49d73c189b197bdc1a983e26ccd49713c617c  0001-lavc-mjpegdec-hint-next-marker-position-in-ff_mjpeg_.patch
> > From ebef63ebdc75872d52529d5b74b6d05254d4c0fd Mon Sep 17 00:00:00 2001
> > From: Matthieu Bouron <matthieu.bouron@gmail.com>
> > Date: Thu, 26 Jan 2017 15:17:36 +0100
> > Subject: [PATCH] lavc/mjpegdec: hint next marker position in
> >  ff_mjpeg_find_marker
> > 
> > Speeds up next marker search when a SOS marker is found.
> > 
> > ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
> > sample_2992x4000.jpg
> 
> the patch seems to work but
> why is the "buf_ptr += (get_bits_count(&s->gb) + 7) / 8;" not
> already skiping all that what the buf_hint does ?
> is the difference the unescaping ?
> maybe the buf_hint stuff can be simplified by adjusting buf_ptr instead
> ?

I've just realized that the SOS marker handling does not consume any byte
from the GetBitContext *only* when the frame is being discard (which
happens in avformat_find_stream_info).

So I guess a better solution would be to consume all the bytes from the
GetBitContext while this marker is skipped and let the code already in
place handle that.
However, the buf_ptr won't directly be at the next marker position since
the sos data is unescaped. With the sample I use there is a difference of
13774 bytes (thus iterations in find_marker) to find the next marker. It
reduces the function (according to perf) from 5,4% to 4.8% while the frame
is being decoded (ffmpeg -i sample_2999x4000.jpeg -f null -) but it seems
the performance gain is too little versus the code that the patch
introduces and the decoder will benefits more from having aarch64 simd
code for the various idct functions (something that i plan to do in the
upcoming days).

I'll send a new patch soon.

Matthieu

[...]
diff mbox

Patch

From ebef63ebdc75872d52529d5b74b6d05254d4c0fd Mon Sep 17 00:00:00 2001
From: Matthieu Bouron <matthieu.bouron@gmail.com>
Date: Thu, 26 Jan 2017 15:17:36 +0100
Subject: [PATCH] lavc/mjpegdec: hint next marker position in
 ff_mjpeg_find_marker

Speeds up next marker search when a SOS marker is found.

ff_mjpeg_find_marker goes from 54% to 30% under perf record ffmpeg -i
sample_2992x4000.jpg
---
 libavcodec/mjpegdec.c | 31 +++++++++++++++++++++++++------
 libavcodec/mjpegdec.h |  2 +-
 libavcodec/mxpegdec.c |  5 +++--
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index 7d17e3dfcb..347c363da0 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -1921,13 +1921,23 @@  static int mjpeg_decode_com(MJpegDecodeContext *s)
 
 /* return the 8 bit start code value and update the search
    state. Return -1 if no start code found */
-static int find_marker(const uint8_t **pbuf_ptr, const uint8_t *buf_end)
+static int find_marker(const uint8_t **pbuf_ptr, const uint8_t *buf_hint, const uint8_t *buf_end)
 {
     const uint8_t *buf_ptr;
     unsigned int v, v2;
     int val;
     int skipped = 0;
 
+    buf_ptr = buf_hint;
+    if (buf_ptr && (buf_end - buf_ptr > 1)) {
+        v = *buf_ptr++;
+        v2 = *buf_ptr;
+        if ((v == 0xff) && (v2 >= 0xc0) && (v2 <= 0xfe) && buf_ptr < buf_end) {
+            val = *buf_ptr++;
+            goto found;
+        }
+    }
+
     buf_ptr = *pbuf_ptr;
     while (buf_end - buf_ptr > 1) {
         v  = *buf_ptr++;
@@ -1947,12 +1957,15 @@  found:
 }
 
 int ff_mjpeg_find_marker(MJpegDecodeContext *s,
-                         const uint8_t **buf_ptr, const uint8_t *buf_end,
+                         const uint8_t **buf_ptr,
+                         const uint8_t **buf_hint,
+                         const uint8_t *buf_end,
                          const uint8_t **unescaped_buf_ptr,
                          int *unescaped_buf_size)
 {
     int start_code;
-    start_code = find_marker(buf_ptr, buf_end);
+    start_code = find_marker(buf_ptr, *buf_hint, buf_end);
+    *buf_hint = NULL;
 
     av_fast_padded_malloc(&s->buffer, &s->buffer_size, buf_end - *buf_ptr);
     if (!s->buffer)
@@ -1963,6 +1976,7 @@  int ff_mjpeg_find_marker(MJpegDecodeContext *s,
         const uint8_t *src = *buf_ptr;
         const uint8_t *ptr = src;
         uint8_t *dst = s->buffer;
+        const uint8_t *next_marker = NULL;
 
         #define copy_data_segment(skip) do {       \
             ptrdiff_t length = (ptr - src) - (skip);  \
@@ -1999,8 +2013,10 @@  int ff_mjpeg_find_marker(MJpegDecodeContext *s,
 
                     if (x < 0xd0 || x > 0xd7) {
                         copy_data_segment(1);
-                        if (x)
+                        if (x) {
+                            next_marker = ptr - 2;
                             break;
+                        }
                     }
                 }
             }
@@ -2009,6 +2025,8 @@  int ff_mjpeg_find_marker(MJpegDecodeContext *s,
         }
         #undef copy_data_segment
 
+        if (next_marker)
+            *buf_hint = next_marker;
         *unescaped_buf_ptr  = s->buffer;
         *unescaped_buf_size = dst - s->buffer;
         memset(s->buffer + *unescaped_buf_size, 0,
@@ -2073,7 +2091,7 @@  int ff_mjpeg_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     const uint8_t *buf = avpkt->data;
     int buf_size       = avpkt->size;
     MJpegDecodeContext *s = avctx->priv_data;
-    const uint8_t *buf_end, *buf_ptr;
+    const uint8_t *buf_end, *buf_hint, *buf_ptr;
     const uint8_t *unescaped_buf_ptr;
     int hshift, vshift;
     int unescaped_buf_size;
@@ -2087,10 +2105,11 @@  int ff_mjpeg_decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
     s->adobe_transform = -1;
 
     buf_ptr = buf;
+    buf_hint = NULL;
     buf_end = buf + buf_size;
     while (buf_ptr < buf_end) {
         /* find start next marker */
-        start_code = ff_mjpeg_find_marker(s, &buf_ptr, buf_end,
+        start_code = ff_mjpeg_find_marker(s, &buf_ptr, &buf_hint, buf_end,
                                           &unescaped_buf_ptr,
                                           &unescaped_buf_size);
         /* EOF */
diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
index fb811294a1..c9d289187b 100644
--- a/libavcodec/mjpegdec.h
+++ b/libavcodec/mjpegdec.h
@@ -144,7 +144,7 @@  int ff_mjpeg_decode_sos(MJpegDecodeContext *s,
                         const uint8_t *mb_bitmask,int mb_bitmask_size,
                         const AVFrame *reference);
 int ff_mjpeg_find_marker(MJpegDecodeContext *s,
-                         const uint8_t **buf_ptr, const uint8_t *buf_end,
+                         const uint8_t **buf_ptr, const uint8_t **buf_hint, const uint8_t *buf_end,
                          const uint8_t **unescaped_buf_ptr, int *unescaped_buf_size);
 
 #endif /* AVCODEC_MJPEGDEC_H */
diff --git a/libavcodec/mxpegdec.c b/libavcodec/mxpegdec.c
index 2e3ebe6e70..28a479a3b5 100644
--- a/libavcodec/mxpegdec.c
+++ b/libavcodec/mxpegdec.c
@@ -189,18 +189,19 @@  static int mxpeg_decode_frame(AVCodecContext *avctx,
     int buf_size = avpkt->size;
     MXpegDecodeContext *s = avctx->priv_data;
     MJpegDecodeContext *jpg = &s->jpg;
-    const uint8_t *buf_end, *buf_ptr;
+    const uint8_t *buf_end, *buf_hint, *buf_ptr;
     const uint8_t *unescaped_buf_ptr;
     int unescaped_buf_size;
     int start_code;
     int ret;
 
     buf_ptr = buf;
+    buf_hint = NULL;
     buf_end = buf + buf_size;
     jpg->got_picture = 0;
     s->got_mxm_bitmask = 0;
     while (buf_ptr < buf_end) {
-        start_code = ff_mjpeg_find_marker(jpg, &buf_ptr, buf_end,
+        start_code = ff_mjpeg_find_marker(jpg, &buf_ptr, &buf_hint, buf_end,
                                           &unescaped_buf_ptr, &unescaped_buf_size);
         if (start_code < 0)
             goto the_end;
-- 
2.11.0