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 > > > > > 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. thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Into a blind darkness they enter who follow after the Ignorance, they as if into a greater darkness enter who devote themselves to the Knowledge alone. -- Isha Upanishad
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".
