diff mbox series

[FFmpeg-devel,1/8] libavcodec/jpeg2000_parser: Speed up long skips

Message ID 152f94f0779c645542f5a678d9392ee53584d45a.camel@acc.umu.se
State New
Headers show
Series [FFmpeg-devel,1/8] libavcodec/jpeg2000_parser: Speed up long skips | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Tomas Härdin May 31, 2022, 9:58 a.m. UTC

Comments

Anton Khirnov June 1, 2022, 9:59 a.m. UTC | #1 
Quoting Tomas Härdin (2022-05-31 11:58:39)
> 
> 
> From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> Date: Fri, 20 May 2022 11:38:25 +0200
> Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long skips
> 
> ---
>  libavcodec/jpeg2000_parser.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/libavcodec/jpeg2000_parser.c b/libavcodec/jpeg2000_parser.c
> index 2975e71482..9fac958dfa 100644
> --- a/libavcodec/jpeg2000_parser.c
> +++ b/libavcodec/jpeg2000_parser.c
> @@ -95,6 +95,17 @@ static int find_frame_end(JPEG2000ParserContext *m, const uint8_t *buf, int buf_
>          state64 = state64 << 8 | buf[i];
>          m->bytes_read++;
>          if (m->skip_bytes) {
> +            // handle long skips
> +            if (m->skip_bytes > 8) {
> +                // need -9 else buf_size - i == 8 ==> i == buf_size after this,
> +                // and thus i == buf_size + 1 after the loop
> +                int64_t skip = FFMIN(m->skip_bytes - 8, buf_size - i - 9);
> +                if (skip > 0) {
> +                    m->skip_bytes -= skip;
> +                    i += skip;
> +                    m->bytes_read += skip;

Shouldn't you also update state(64)?
Tomas Härdin June 1, 2022, 12:34 p.m. UTC | #2 
ons 2022-06-01 klockan 11:59 +0200 skrev Anton Khirnov:
> Quoting Tomas Härdin (2022-05-31 11:58:39)
> > 
> > 
> > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00
> > 2001
> > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > Date: Fri, 20 May 2022 11:38:25 +0200
> > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long
> > skips
> > 
> > ---
> >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/libavcodec/jpeg2000_parser.c
> > b/libavcodec/jpeg2000_parser.c
> > index 2975e71482..9fac958dfa 100644
> > --- a/libavcodec/jpeg2000_parser.c
> > +++ b/libavcodec/jpeg2000_parser.c
> > @@ -95,6 +95,17 @@ static int find_frame_end(JPEG2000ParserContext
> > *m, const uint8_t *buf, int buf_
> >          state64 = state64 << 8 | buf[i];
> >          m->bytes_read++;
> >          if (m->skip_bytes) {
> > +            // handle long skips
> > +            if (m->skip_bytes > 8) {
> > +                // need -9 else buf_size - i == 8 ==> i ==
> > buf_size after this,
> > +                // and thus i == buf_size + 1 after the loop
> > +                int64_t skip = FFMIN(m->skip_bytes - 8, buf_size -
> > i - 9);
> > +                if (skip > 0) {
> > +                    m->skip_bytes -= skip;
> > +                    i += skip;
> > +                    m->bytes_read += skip;
> 
> Shouldn't you also update state(64)?
> 

It gets updated after 8 more skips. Keep in mind the buffer may end
before the skip is fully done, so I can't just AV_RB64().

/Tomas
Anton Khirnov June 1, 2022, 1:29 p.m. UTC | #3 
Quoting Tomas Härdin (2022-06-01 14:34:57)
> ons 2022-06-01 klockan 11:59 +0200 skrev Anton Khirnov:
> > Quoting Tomas Härdin (2022-05-31 11:58:39)
> > > 
> > > 
> > > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00
> > > 2001
> > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > > Date: Fri, 20 May 2022 11:38:25 +0200
> > > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long
> > > skips
> > > 
> > > ---
> > >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/libavcodec/jpeg2000_parser.c
> > > b/libavcodec/jpeg2000_parser.c
> > > index 2975e71482..9fac958dfa 100644
> > > --- a/libavcodec/jpeg2000_parser.c
> > > +++ b/libavcodec/jpeg2000_parser.c
> > > @@ -95,6 +95,17 @@ static int find_frame_end(JPEG2000ParserContext
> > > *m, const uint8_t *buf, int buf_
> > >          state64 = state64 << 8 | buf[i];
> > >          m->bytes_read++;
> > >          if (m->skip_bytes) {
> > > +            // handle long skips
> > > +            if (m->skip_bytes > 8) {
> > > +                // need -9 else buf_size - i == 8 ==> i ==
> > > buf_size after this,
> > > +                // and thus i == buf_size + 1 after the loop
> > > +                int64_t skip = FFMIN(m->skip_bytes - 8, buf_size -
> > > i - 9);
> > > +                if (skip > 0) {
> > > +                    m->skip_bytes -= skip;
> > > +                    i += skip;
> > > +                    m->bytes_read += skip;
> > 
> > Shouldn't you also update state(64)?
> > 
> 
> It gets updated after 8 more skips. Keep in mind the buffer may end
> before the skip is fully done, so I can't just AV_RB64().

Ah right, nevermind then. Set looks good to me.
Michael Niedermayer June 1, 2022, 4:21 p.m. UTC | #4 
On Tue, May 31, 2022 at 11:58:39AM +0200, Tomas Härdin wrote:
> 

>  jpeg2000_parser.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 634546fb5a0eb281eea87ad7471c503f5bc9e8ab  0001-libavcodec-jpeg2000_parser-Speed-up-long-skips.patch
> From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> Date: Fri, 20 May 2022 11:38:25 +0200
> Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long skips
> 
> ---
>  libavcodec/jpeg2000_parser.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)

breaks
j2kref/codestreams_profile1/p1_04.j2k

[...]
Michael Niedermayer June 1, 2022, 4:23 p.m. UTC | #5 
On Wed, Jun 01, 2022 at 06:21:19PM +0200, Michael Niedermayer wrote:
> On Tue, May 31, 2022 at 11:58:39AM +0200, Tomas Härdin wrote:
> > 
> 
> >  jpeg2000_parser.c |   11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 634546fb5a0eb281eea87ad7471c503f5bc9e8ab  0001-libavcodec-jpeg2000_parser-Speed-up-long-skips.patch
> > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > Date: Fri, 20 May 2022 11:38:25 +0200
> > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long skips
> > 
> > ---
> >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> 
> breaks
> j2kref/codestreams_profile1/p1_04.j2k

[jpeg2000 @ 0x7fb0b8002600] Psot 66195 too big
[jpeg2000 @ 0x7fb0b8002600] error during processing marker segment ff90
Input #0, j2k_pipe, from 'j2kref/codestreams_profile1/p1_04.j2k':
  Duration: N/A, bitrate: N/A
  Stream #0:0: Video: jpeg2000 (JPEG 2000 codestream restriction 1), gray16le(12 bpc), 1024x1024, 25 fps, 25 tbr, 25 tbn
[jpeg2000 @ 0x7fb0b8003240] unsupported marker 0x97C8 at pos 0x6F0
[jpeg2000 @ 0x7fb0b8003240] Missing EOC Marker.
[jpeg2000 @ 0x7fb0b8032280] unsupported marker 0x97C8 at pos 0x6F0
[jpeg2000 @ 0x7fb0b8032280] Missing EOC Marker.
[jpeg2000 @ 0x7fb0b8006280] Psot 66195 too big
[jpeg2000 @ 0x7fb0b8006280] error during processing marker segment ff90


[...]
Tomas Härdin June 2, 2022, 9:54 a.m. UTC | #6 
ons 2022-06-01 klockan 18:23 +0200 skrev Michael Niedermayer:
> On Wed, Jun 01, 2022 at 06:21:19PM +0200, Michael Niedermayer wrote:
> > On Tue, May 31, 2022 at 11:58:39AM +0200, Tomas Härdin wrote:
> > > 
> > 
> > >  jpeg2000_parser.c |   11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 634546fb5a0eb281eea87ad7471c503f5bc9e8ab  0001-libavcodec-
> > > jpeg2000_parser-Speed-up-long-skips.patch
> > > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00
> > > 2001
> > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > > Date: Fri, 20 May 2022 11:38:25 +0200
> > > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long
> > > skips
> > > 
> > > ---
> > >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > 
> > breaks
> > j2kref/codestreams_profile1/p1_04.j2k
> 
> [jpeg2000 @ 0x7fb0b8002600] Psot 66195 too big
> [jpeg2000 @ 0x7fb0b8002600] error during processing marker segment
> ff90
> Input #0, j2k_pipe, from 'j2kref/codestreams_profile1/p1_04.j2k':
>   Duration: N/A, bitrate: N/A
>   Stream #0:0: Video: jpeg2000 (JPEG 2000 codestream restriction 1),
> gray16le(12 bpc), 1024x1024, 25 fps, 25 tbr, 25 tbn
> [jpeg2000 @ 0x7fb0b8003240] unsupported marker 0x97C8 at pos 0x6F0
> [jpeg2000 @ 0x7fb0b8003240] Missing EOC Marker.
> [jpeg2000 @ 0x7fb0b8032280] unsupported marker 0x97C8 at pos 0x6F0
> [jpeg2000 @ 0x7fb0b8032280] Missing EOC Marker.
> [jpeg2000 @ 0x7fb0b8006280] Psot 66195 too big
> [jpeg2000 @ 0x7fb0b8006280] error during processing marker segment
> ff90

Took a while to figure out, but this is due to buf_size - i - 9 being
changed to unsigned because of the uint32_t. Try attached patch. The
rest of the set should work with it.

Can we roll these tests into FATE?

/Tomas
Michael Niedermayer June 2, 2022, 7:19 p.m. UTC | #7 
On Thu, Jun 02, 2022 at 11:54:39AM +0200, Tomas Härdin wrote:
> ons 2022-06-01 klockan 18:23 +0200 skrev Michael Niedermayer:
> > On Wed, Jun 01, 2022 at 06:21:19PM +0200, Michael Niedermayer wrote:
> > > On Tue, May 31, 2022 at 11:58:39AM +0200, Tomas Härdin wrote:
> > > > 
> > > 
> > > >  jpeg2000_parser.c |   11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > > 634546fb5a0eb281eea87ad7471c503f5bc9e8ab  0001-libavcodec-
> > > > jpeg2000_parser-Speed-up-long-skips.patch
> > > > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00
> > > > 2001
> > > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > > > Date: Fri, 20 May 2022 11:38:25 +0200
> > > > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long
> > > > skips
> > > > 
> > > > ---
> > > >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> > > >  1 file changed, 11 insertions(+)
> > > 
> > > breaks
> > > j2kref/codestreams_profile1/p1_04.j2k
> > 
> > [jpeg2000 @ 0x7fb0b8002600] Psot 66195 too big
> > [jpeg2000 @ 0x7fb0b8002600] error during processing marker segment
> > ff90
> > Input #0, j2k_pipe, from 'j2kref/codestreams_profile1/p1_04.j2k':
> >   Duration: N/A, bitrate: N/A
> >   Stream #0:0: Video: jpeg2000 (JPEG 2000 codestream restriction 1),
> > gray16le(12 bpc), 1024x1024, 25 fps, 25 tbr, 25 tbn
> > [jpeg2000 @ 0x7fb0b8003240] unsupported marker 0x97C8 at pos 0x6F0
> > [jpeg2000 @ 0x7fb0b8003240] Missing EOC Marker.
> > [jpeg2000 @ 0x7fb0b8032280] unsupported marker 0x97C8 at pos 0x6F0
> > [jpeg2000 @ 0x7fb0b8032280] Missing EOC Marker.
> > [jpeg2000 @ 0x7fb0b8006280] Psot 66195 too big
> > [jpeg2000 @ 0x7fb0b8006280] error during processing marker segment
> > ff90
> 
> Took a while to figure out, but this is due to buf_size - i - 9 being
> changed to unsigned because of the uint32_t. Try attached patch. The
> rest of the set should work with it.

works fine here


> 
> Can we roll these tests into FATE?

feel free to do so. 
ill test if someone posts a patch

thx

[...]
Tomas Härdin June 3, 2022, 9:17 a.m. UTC | #8 
tor 2022-06-02 klockan 21:19 +0200 skrev Michael Niedermayer:
> On Thu, Jun 02, 2022 at 11:54:39AM +0200, Tomas Härdin wrote:
> > ons 2022-06-01 klockan 18:23 +0200 skrev Michael Niedermayer:
> > > On Wed, Jun 01, 2022 at 06:21:19PM +0200, Michael Niedermayer
> > > wrote:
> > > > On Tue, May 31, 2022 at 11:58:39AM +0200, Tomas Härdin wrote:
> > > > > 
> > > > 
> > > > >  jpeg2000_parser.c |   11 +++++++++++
> > > > >  1 file changed, 11 insertions(+)
> > > > > 634546fb5a0eb281eea87ad7471c503f5bc9e8ab  0001-libavcodec-
> > > > > jpeg2000_parser-Speed-up-long-skips.patch
> > > > > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17
> > > > > 00:00:00
> > > > > 2001
> > > > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > > > > Date: Fri, 20 May 2022 11:38:25 +0200
> > > > > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up
> > > > > long
> > > > > skips
> > > > > 
> > > > > ---
> > > > >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> > > > >  1 file changed, 11 insertions(+)
> > > > 
> > > > breaks
> > > > j2kref/codestreams_profile1/p1_04.j2k
> > > 
> > > [jpeg2000 @ 0x7fb0b8002600] Psot 66195 too big
> > > [jpeg2000 @ 0x7fb0b8002600] error during processing marker
> > > segment
> > > ff90
> > > Input #0, j2k_pipe, from 'j2kref/codestreams_profile1/p1_04.j2k':
> > >   Duration: N/A, bitrate: N/A
> > >   Stream #0:0: Video: jpeg2000 (JPEG 2000 codestream restriction
> > > 1),
> > > gray16le(12 bpc), 1024x1024, 25 fps, 25 tbr, 25 tbn
> > > [jpeg2000 @ 0x7fb0b8003240] unsupported marker 0x97C8 at pos
> > > 0x6F0
> > > [jpeg2000 @ 0x7fb0b8003240] Missing EOC Marker.
> > > [jpeg2000 @ 0x7fb0b8032280] unsupported marker 0x97C8 at pos
> > > 0x6F0
> > > [jpeg2000 @ 0x7fb0b8032280] Missing EOC Marker.
> > > [jpeg2000 @ 0x7fb0b8006280] Psot 66195 too big
> > > [jpeg2000 @ 0x7fb0b8006280] error during processing marker
> > > segment
> > > ff90
> > 
> > Took a while to figure out, but this is due to buf_size - i - 9
> > being
> > changed to unsigned because of the uint32_t. Try attached patch.
> > The
> > rest of the set should work with it.
> 
> works fine here

Great

> > Can we roll these tests into FATE?
> 
> feel free to do so. 
> ill test if someone posts a patch

How are you testing them? Just decoding and checking that the output is
the same as some reference output? Just looking for non-zero exit code?

/Tomas
Michael Niedermayer June 3, 2022, 2:58 p.m. UTC | #9 
On Fri, Jun 03, 2022 at 11:17:54AM +0200, Tomas Härdin wrote:
> tor 2022-06-02 klockan 21:19 +0200 skrev Michael Niedermayer:
> > On Thu, Jun 02, 2022 at 11:54:39AM +0200, Tomas Härdin wrote:
> > > ons 2022-06-01 klockan 18:23 +0200 skrev Michael Niedermayer:
> > > > On Wed, Jun 01, 2022 at 06:21:19PM +0200, Michael Niedermayer
> > > > wrote:
> > > > > On Tue, May 31, 2022 at 11:58:39AM +0200, Tomas Härdin wrote:
> > > > > > 
> > > > > 
> > > > > >  jpeg2000_parser.c |   11 +++++++++++
> > > > > >  1 file changed, 11 insertions(+)
> > > > > > 634546fb5a0eb281eea87ad7471c503f5bc9e8ab  0001-libavcodec-
> > > > > > jpeg2000_parser-Speed-up-long-skips.patch
> > > > > > From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17
> > > > > > 00:00:00
> > > > > > 2001
> > > > > > From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> > > > > > Date: Fri, 20 May 2022 11:38:25 +0200
> > > > > > Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up
> > > > > > long
> > > > > > skips
> > > > > > 
> > > > > > ---
> > > > > >  libavcodec/jpeg2000_parser.c | 11 +++++++++++
> > > > > >  1 file changed, 11 insertions(+)
> > > > > 
> > > > > breaks
> > > > > j2kref/codestreams_profile1/p1_04.j2k
> > > > 
> > > > [jpeg2000 @ 0x7fb0b8002600] Psot 66195 too big
> > > > [jpeg2000 @ 0x7fb0b8002600] error during processing marker
> > > > segment
> > > > ff90
> > > > Input #0, j2k_pipe, from 'j2kref/codestreams_profile1/p1_04.j2k':
> > > >   Duration: N/A, bitrate: N/A
> > > >   Stream #0:0: Video: jpeg2000 (JPEG 2000 codestream restriction
> > > > 1),
> > > > gray16le(12 bpc), 1024x1024, 25 fps, 25 tbr, 25 tbn
> > > > [jpeg2000 @ 0x7fb0b8003240] unsupported marker 0x97C8 at pos
> > > > 0x6F0
> > > > [jpeg2000 @ 0x7fb0b8003240] Missing EOC Marker.
> > > > [jpeg2000 @ 0x7fb0b8032280] unsupported marker 0x97C8 at pos
> > > > 0x6F0
> > > > [jpeg2000 @ 0x7fb0b8032280] Missing EOC Marker.
> > > > [jpeg2000 @ 0x7fb0b8006280] Psot 66195 too big
> > > > [jpeg2000 @ 0x7fb0b8006280] error during processing marker
> > > > segment
> > > > ff90
> > > 
> > > Took a while to figure out, but this is due to buf_size - i - 9
> > > being
> > > changed to unsigned because of the uint32_t. Try attached patch.
> > > The
> > > rest of the set should work with it.
> > 
> > works fine here
> 
> Great
> 
> > > Can we roll these tests into FATE?
> > 
> > feel free to do so. 
> > ill test if someone posts a patch
> 
> How are you testing them? Just decoding and checking that the output is
> the same as some reference output? 

yes


> Just looking for non-zero exit code?

[...]
Tomas Härdin June 10, 2022, 9:12 a.m. UTC | #10 
Pushed all except patch 8/8 because I'm working on something better

/Tomas
diff mbox series

Patch

From fedd7f9ae2c691a25c37be935d7547be61d46017 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Fri, 20 May 2022 11:38:25 +0200
Subject: [PATCH 1/8] libavcodec/jpeg2000_parser: Speed up long skips

---
 libavcodec/jpeg2000_parser.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/libavcodec/jpeg2000_parser.c b/libavcodec/jpeg2000_parser.c
index 2975e71482..9fac958dfa 100644
--- a/libavcodec/jpeg2000_parser.c
+++ b/libavcodec/jpeg2000_parser.c
@@ -95,6 +95,17 @@  static int find_frame_end(JPEG2000ParserContext *m, const uint8_t *buf, int buf_
         state64 = state64 << 8 | buf[i];
         m->bytes_read++;
         if (m->skip_bytes) {
+            // handle long skips
+            if (m->skip_bytes > 8) {
+                // need -9 else buf_size - i == 8 ==> i == buf_size after this,
+                // and thus i == buf_size + 1 after the loop
+                int64_t skip = FFMIN(m->skip_bytes - 8, buf_size - i - 9);
+                if (skip > 0) {
+                    m->skip_bytes -= skip;
+                    i += skip;
+                    m->bytes_read += skip;
+                }
+            }
             m->skip_bytes--;
             continue;
         }
-- 
2.30.2