On Sat, 26 Sep 2020, Michael Niedermayer wrote:
On Sat, Sep 26, 2020 at 12:46:40PM +0200, Marton Balint wrote:
On Sat, 26 Sep 2020, Michael Niedermayer wrote:
On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:
Signed-off-by: Marton Balint <c...@passwd.hu>
---
libavformat/aviobuf.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
The commit message is too terse. It is not clear from it what the problem
is that is being fixed and how it is fixed.
Also this feels like it fixes multiple issues so it probably should be spit
unless iam missing some interdependancies
It can be seperated to two patches if that is preferred:
1) existing code does not check if the requested seekback buffer is already
read entirely. In this case, nothing has to be done. This is the newly added
if() at the beginning of the patch.
2) the new buf_size is detemined too conservatively, maybe because of the
issue which was fixed in patch 1/3. So we can safely substract 1 more from
the new buffer size. Comparing the new buf_size against filled does not make
a lot of sense to me, that just seems wrong. What makes sense is that we
want to reallocate the buffer if the new buf_size is bigger than the old,
therefore the change in the check.
i would prefer it split yes
Ok, will send new series.
ill also take a look at the "competing" patch, so i dont yet know
how they relate ...
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 9675425349..aa1d6c0830 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,9 +999,12 @@ 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 <= s->buf_end - s->buf_ptr)
+ return 0;
+
+ buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;
- if (buf_size < filled || s->seekable || !s->read_packet)
+ if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
return 0;
Did you check that this doesnt interfere with the s->buffer_size reduction
we do when it was larger from probing ?
Its maybe ok, just checking that this was considered
I don't see how that can be affected by this change, so I am not sure what
you mean here. If the new buffer size is bigger than s->orig_buffer_size,
then eventually it will be reduced, it does not seem more complicated than
that to me.
what i meant was that if its reduced then at that point the seekback
gurantee changes relative to it never being reduced.
I think you consider this in your code, i just wanted to double check.
I think that is already ensured by the check in fill_buffer before
decreaseing the buffer:
if (dst == s->buffer && s->buf_ptr != dst)
This checks that buffer size decrease can only happen when we are writing
to the start of the buffer and buf_ptr > buffer, so we are flushing
anyways.
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".
