| Message ID | 20170128125910.GE30664@tsuri |
|---|---|
| State | Accepted |
| Headers | show |
On Sat, Jan 28, 2017 at 01:59:10PM +0100, Matthieu Bouron wrote: > On Sat, Jan 28, 2017 at 12:59:39PM +0100, Matthieu Bouron wrote: > > 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). you could keep track of the unescaped bytes ad add them too > > > > I'll send a new patch soon. > > Alternative patch attached. LGTM thx [...]
On Sat, Jan 28, 2017 at 10:40:18PM +0100, Michael Niedermayer wrote: > On Sat, Jan 28, 2017 at 01:59:10PM +0100, Matthieu Bouron wrote: > > On Sat, Jan 28, 2017 at 12:59:39PM +0100, Matthieu Bouron wrote: > > > 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). > > you could keep track of the unescaped bytes ad add them too > > > > > > > > I'll send a new patch soon. > > > > Alternative patch attached. > > LGTM Pushed. Thanks. [...]
From 1bbb34b3874393507cc49a4f9685d54850ca5057 Mon Sep 17 00:00:00 2001 From: Matthieu Bouron <matthieu.bouron@gmail.com> Date: Sat, 28 Jan 2017 13:49:52 +0100 Subject: [PATCH] lavc/mjpegdec: consume SOS data even if the frame is discarded Speeds up next marker search when a SOS marker is found but the frame is discarded (which happens in avformat_find_stream_info). --- libavcodec/mjpegdec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 7d17e3dfcb..07988b68b1 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -2247,8 +2247,10 @@ eoi_parser: goto the_end; case SOS: s->cur_scan++; - if (avctx->skip_frame == AVDISCARD_ALL) + if (avctx->skip_frame == AVDISCARD_ALL) { + skip_bits(&s->gb, get_bits_left(&s->gb)); break; + } if ((ret = ff_mjpeg_decode_sos(s, NULL, 0, NULL)) < 0 && (avctx->err_recognition & AV_EF_EXPLODE)) -- 2.11.0