Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:50:34PM +0200, Michael Niedermayer wrote:
> On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> > This removes big CPU overhead for demuxing chained ogg streams.
> > 
> > Signed-off-by: Paul B Mahol 
> > ---
> >  libavformat/aviobuf.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> I think we need some fate test for ffio_ensure_seekback()
> as our tests are ATM we generally have fully seekable inputs so
> failure of seekback would not be noticed.
> 
> This is a problem because the buffer IO stuff is not easy to
> understand and so it is time consuming and error prone to review these patches
> just by eye. And as we see developers disagree on what is correct
> 
> Such fate test should at least test various sequentially occuring
> ffio_ensure_seekback(), a possibly larger initial buffer from probing
> as well as its later reduction. And also reading requests failing, early EOF
> seeking, and IO errors

Well, if its documented that ffio_ensure_seekback() can not flush buffer
than there should be added code/function to do allow flushing.

Because oggdec works fine with assumed flushing and my patch.

> 
> thanks
> 
> [...]
> 
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> The day soldiers stop bringing you their problems is the day you have stopped 
> leading them. They have either lost confidence that you can help or concluded 
> you do not care. Either case is a failure of leadership. - Colin Powell



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-26 Thread Michael Niedermayer
On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> This removes big CPU overhead for demuxing chained ogg streams.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/aviobuf.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

I think we need some fate test for ffio_ensure_seekback()
as our tests are ATM we generally have fully seekable inputs so
failure of seekback would not be noticed.

This is a problem because the buffer IO stuff is not easy to
understand and so it is time consuming and error prone to review these patches
just by eye. And as we see developers disagree on what is correct

Such fate test should at least test various sequentially occuring
ffio_ensure_seekback(), a possibly larger initial buffer from probing
as well as its later reduction. And also reading requests failing, early EOF
seeking, and IO errors

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-26 Thread Michael Niedermayer
On Sun, Sep 20, 2020 at 10:49:12AM +0200, Marton Balint wrote:
[...]

> fundamental problem is that ffio_ensure_seekback simply cannot be
> implemented efficiently with the current design. The guarantees should be
> reduced (e.g. ffio_ensure_seekback can flush the buffer and invalidate
> previous ffio_ensure_seekback request)

this probably makes sense, would this cause a problem to any code using it ?

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-20 Thread Marton Balint



On Sun, 20 Sep 2020, Paul B Mahol wrote:


On Sat, Sep 19, 2020 at 11:46:50PM +0200, Marton Balint wrote:



On Sat, 19 Sep 2020, Paul B Mahol wrote:

> On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> > This removes big CPU overhead for demuxing chained ogg streams.
> > 
> > Signed-off-by: Paul B Mahol 

> > ---
> >  libavformat/aviobuf.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> 
> will apply soon.


Don't apply it, as I explained, it is not correct.


I carefully examined your explanation and I already reported
here on mailing list for every one to see that it does not work.

So please reconsider your blocking statement and provide
correct solution to problem.


Your solution will break the fundamentals of ffio_ensure_seekback, so it 
is not OK. Checking if (buf_size <= s->buffer_size) will not guarantee 
that the buffer will not be flushed when reading the next buf_size bytes 
because buf_ptr can be anywhere in the buffer. See how fill_buffer() 
works. It not only makes sure the buffer has space, it makes sure that the 
buffer has max_buffer_size space available. If not, then it gets flushed.


There are some issues with current code, I will send a patch to fix them. 
Unfortunately it will NOT fix your problem entirely. As I suggested, if 
you use 2*max_buffer_size buffers by default, that might help you, but the 
fundamental problem is that ffio_ensure_seekback simply cannot be 
implemented efficiently with the current design. The guarantees should be 
reduced (e.g. ffio_ensure_seekback can flush the buffer and invalidate 
previous ffio_ensure_seekback request)


Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-19 Thread Paul B Mahol
On Sat, Sep 19, 2020 at 11:46:50PM +0200, Marton Balint wrote:
> 
> 
> On Sat, 19 Sep 2020, Paul B Mahol wrote:
> 
> > On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> > > This removes big CPU overhead for demuxing chained ogg streams.
> > > 
> > > Signed-off-by: Paul B Mahol 
> > > ---
> > >  libavformat/aviobuf.c | 10 +-
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > 
> > 
> > will apply soon.
> 
> Don't apply it, as I explained, it is not correct.

I carefully examined your explanation and I already reported
here on mailing list for every one to see that it does not work.

So please reconsider your blocking statement and provide
correct solution to problem.

I can even upload relevant sample somewhere privately
for you so you can freely produce correct solution
and compare it with my solution for everyones benefit.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-19 Thread Marton Balint



On Sat, 19 Sep 2020, Paul B Mahol wrote:


On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:

This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
 libavformat/aviobuf.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)



will apply soon.


Don't apply it, as I explained, it is not correct.

Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-19 Thread Paul B Mahol
On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> This removes big CPU overhead for demuxing chained ogg streams.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/aviobuf.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

will apply soon.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-17 Thread Paul B Mahol
On Thu, Sep 17, 2020 at 09:44:52PM +0200, Marton Balint wrote:
> 
> 
> On Thu, 17 Sep 2020, Paul B Mahol wrote:
> 
> > This removes big CPU overhead for demuxing chained ogg streams.
> > 
> > Signed-off-by: Paul B Mahol 
> > ---
> > libavformat/aviobuf.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> > index a77517d712..ce9b7d59c9 100644
> > --- a/libavformat/aviobuf.c
> > +++ b/libavformat/aviobuf.c
> > @@ -996,20 +996,20 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t 
> > buf_size)
> > uint8_t *buffer;
> > int max_buffer_size = s->max_packet_size ?
> >   s->max_packet_size : IO_BUFFER_SIZE;
> > -int filled = s->buf_end - s->buffer;
> > ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
> > s->buffer : -1;
> > 
> > -buf_size += s->buf_ptr - s->buffer + max_buffer_size;
> > -
> > -if (buf_size < filled || s->seekable || !s->read_packet)
> > +if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
> > return 0;
> 
> This is still not correct. You can only return if
> buf_size <= s->buf_end - s->buf_ptr

That one gives you original issue.

> OR
> buf_size <= s->buffer + s->buffer_size - s->buf_ptr - max_buffer_size + 1
> 

That one gives you also original issue but at slower peace.

> And the new minimum buffer size is what is calculated now, so that should
> not be changed.
> 
> I am not sure if this fixes you original issue. It might make sense to
> allocate max_buffer_size*2 length buffers by default, but the buffer size
> revert logic must be fixed to support that...
> 
> Regards,
> Marton
> 
> 
> 
> > +buf_size += s->buffer_size;
> > +buf_size = FFMAX(buf_size, max_buffer_size);
> > +
> > av_assert0(!s->write_flag);
> > 
> > buffer = av_malloc(buf_size);
> > if (!buffer)
> > return AVERROR(ENOMEM);
> > 
> > -memcpy(buffer, s->buffer, filled);
> > +memcpy(buffer, s->buffer, s->buffer_size);
> > av_free(s->buffer);
> > s->buf_ptr = buffer + (s->buf_ptr - s->buffer);
> > s->buf_end = buffer + (s->buf_end - s->buffer);
> > -- 
> > 2.17.1
> > 
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-17 Thread Marton Balint



On Thu, 17 Sep 2020, Paul B Mahol wrote:


This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
libavformat/aviobuf.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..ce9b7d59c9 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -996,20 +996,20 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
uint8_t *buffer;
int max_buffer_size = s->max_packet_size ?
  s->max_packet_size : IO_BUFFER_SIZE;
-int filled = s->buf_end - s->buffer;
ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;

-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
-
-if (buf_size < filled || s->seekable || !s->read_packet)
+if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
return 0;


This is still not correct. You can only return if
buf_size <= s->buf_end - s->buf_ptr
OR
buf_size <= s->buffer + s->buffer_size - s->buf_ptr - max_buffer_size + 1

And the new minimum buffer size is what is calculated now, so that should 
not be changed.


I am not sure if this fixes you original issue. It might make sense to 
allocate max_buffer_size*2 length buffers by default, but the buffer size 
revert logic must be fixed to support that...


Regards,
Marton




+buf_size += s->buffer_size;
+buf_size = FFMAX(buf_size, max_buffer_size);
+
av_assert0(!s->write_flag);

buffer = av_malloc(buf_size);
if (!buffer)
return AVERROR(ENOMEM);

-memcpy(buffer, s->buffer, filled);
+memcpy(buffer, s->buffer, s->buffer_size);
av_free(s->buffer);
s->buf_ptr = buffer + (s->buf_ptr - s->buffer);
s->buf_end = buffer + (s->buf_end - s->buffer);
--
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-17 Thread Paul B Mahol
This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
 libavformat/aviobuf.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..ce9b7d59c9 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -996,20 +996,20 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
 uint8_t *buffer;
 int max_buffer_size = s->max_packet_size ?
   s->max_packet_size : IO_BUFFER_SIZE;
-int filled = s->buf_end - s->buffer;
 ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;
 
-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
-
-if (buf_size < filled || s->seekable || !s->read_packet)
+if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
 return 0;
+buf_size += s->buffer_size;
+buf_size = FFMAX(buf_size, max_buffer_size);
+
 av_assert0(!s->write_flag);
 
 buffer = av_malloc(buf_size);
 if (!buffer)
 return AVERROR(ENOMEM);
 
-memcpy(buffer, s->buffer, filled);
+memcpy(buffer, s->buffer, s->buffer_size);
 av_free(s->buffer);
 s->buf_ptr = buffer + (s->buf_ptr - s->buffer);
 s->buf_end = buffer + (s->buf_end - s->buffer);
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-17 Thread Marton Balint



On Wed, 16 Sep 2020, Paul B Mahol wrote:


This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
libavformat/aviobuf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..9dfef2377e 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,10 +999,10 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
int filled = s->buf_end - s->buffer;
ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;

-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
-
if (buf_size < filled || s->seekable || !s->read_packet)


This does not seem right, the amount of to-be-read data in 
the buffer is (buf_end - buf_ptr), so that is the amount that is seekable 
backwards, not (buf_end - buffer = filled).


Regards,
Marton


return 0;
+buf_size += s->buf_end - s->buf_ptr + max_buffer_size;
+
av_assert0(!s->write_flag);

buffer = av_malloc(buf_size);
--
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-16 Thread Paul B Mahol
This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
 libavformat/aviobuf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..9dfef2377e 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,10 +999,10 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
 int filled = s->buf_end - s->buffer;
 ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;
 
-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
-
 if (buf_size < filled || s->seekable || !s->read_packet)
 return 0;
+buf_size += s->buf_end - s->buf_ptr + max_buffer_size;
+
 av_assert0(!s->write_flag);
 
 buffer = av_malloc(buf_size);
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-16 Thread Andreas Rheinhardt
Paul B Mahol:
> This removes big CPU overhead for demuxing chained ogg streams.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/aviobuf.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index a77517d712..88cc0b4030 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -999,8 +999,6 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
>  int filled = s->buf_end - s->buffer;
>  ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
> s->buffer : -1;
>  
> -buf_size += s->buf_ptr - s->buffer + max_buffer_size;
> -
>  if (buf_size < filled || s->seekable || !s->read_packet)
>  return 0;
>  av_assert0(!s->write_flag);
> 
This will make the buffer smaller (even very small) if not enough data
is in the buffer (and if it is unseekable). There are several users of
this function that use quite small numbers and if the buffer got shrunk,
future I/O operations will be really slow.

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-16 Thread Paul B Mahol
This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
 libavformat/aviobuf.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..88cc0b4030 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,8 +999,6 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
 int filled = s->buf_end - s->buffer;
 ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;
 
-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
-
 if (buf_size < filled || s->seekable || !s->read_packet)
 return 0;
 av_assert0(!s->write_flag);
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-16 Thread Andreas Rheinhardt
Paul B Mahol:
> This removes big CPU overhead for demuxing chained ogg streams.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/aviobuf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index a77517d712..c27d564602 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -999,7 +999,7 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
>  int filled = s->buf_end - s->buffer;
>  ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
> s->buffer : -1;
>  
> -buf_size += s->buf_ptr - s->buffer + max_buffer_size;
> +buf_size += (s->buf_end - s->buf_ptr) + max_buffer_size;
>  
>  if (buf_size < filled || s->seekable || !s->read_packet)
>  return 0;
> 
Wouldn't it actually be enough to check whether buf_size < filled (which
should probably be renamed to available) without adding anything to
buf_size at all (at least not before doing the check)?
(Btw: avio_seek() does not always perform seeks within the buffer, even
when it could: It doesn't do so if the direct flag is set and if there
is a seek function; but does the existence of the latter really imply
that the stream is actually seekable?)

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-16 Thread Paul B Mahol
This removes big CPU overhead for demuxing chained ogg streams.

Signed-off-by: Paul B Mahol 
---
 libavformat/aviobuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..c27d564602 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,7 +999,7 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
 int filled = s->buf_end - s->buffer;
 ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;
 
-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
+buf_size += (s->buf_end - s->buf_ptr) + max_buffer_size;
 
 if (buf_size < filled || s->seekable || !s->read_packet)
 return 0;
-- 
2.17.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".