On Thu, 8 Jan 2015, Luca Barbato wrote:

On 02/01/15 18:58, Martin Storsjö wrote:
On Fri, 2 Jan 2015, Luca Barbato wrote:

On 02/01/15 17:04, Martin Storsjö wrote:
On Fri, 2 Jan 2015, Martin Storsjö wrote:

On Fri, 2 Jan 2015, Luca Barbato wrote:

CC: [email protected]
---

It is a really ridiculous corner case but happens in real life.

libavformat/aviobuf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 6923b78..b7786f7 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -125,8 +125,8 @@ static void flush_buffer(AVIOContext *s)
{
    if (s->buf_ptr > s->buffer) {
        if (s->write_packet && !s->error) {
-            int ret = s->write_packet(s->opaque, s->buffer,
-                                      s->buf_ptr - s->buffer);
+            int len = FFMIN(s->buf_ptr - s->buffer, s->buffer_size);
+            int ret = s->write_packet(s->opaque, s->buffer, len);
            if (ret < 0) {
                s->error = ret;
            }
--
2.1.0

Hmm, looks like a pretty nasty issue if this happens - if this happens
I think we might need to add a similar FFMIN() in a number of
different places as well. Can you pinpoint where the pointers end up
being set out of bounds?

from what I could see you have avio_w8 doing

    write
    pointer++
    check if (pointer is >= end) and flush

Flush uses start-pointer to decide the amount to write down.

I'm not 100% sure when avio_seek do not trigger a flush when it
position itself right at the last byte of the buffer (the code is
sufficiently convoluted to warrant a refactor).

Ok, that sounds like the potential cause, or if avio_write or some other
function only needs a (pointer >= end) check at the end.


I'd merge this patch and refactor later.

Hmm, with this patch, doesn't it mean that the one byte written by avio_w8 is dropped/ignored/forgotten, so you basically write a corrupted output stream?

Also, in this case, it increases s->pos by s->buf_ptr - s->buffer; won't that mean that s->pos gets out of sync with the actual position in the stream?

Does the issue go away if you change this part of avio_seek

    if (!s->must_flush &&
        offset1 >= 0 && offset1 <= (s->buf_end - s->buffer)) {

like this?

    if (!s->must_flush &&
        offset1 >= 0 && offset1 < (s->buf_end - s->buffer)) {


Also, can you share what calling code causes this, i.e. what muxer does it happen with? Is it reproducible easily?


// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to