[FFmpeg-devel] [PATCH] libavformat aviobuf: Fixed dst pointer initialization in fill_buffer

[Rob Meyers]

Here's a simple server we used to reproduce the problem:

#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>

#define INGOING "/tmp/audio.fifo"

ssize_t write_fifo(int fd, char* msg, int size) {
  ssize_t write_rc;

  write_rc = write(fd, msg, size);
  printf("msg '%s' write_rc = %ld\n", msg, write_rc);
  if (write_rc != size) {
    perror("write sent fewer bytes than expected");
  }
  if (write_rc == -1) {
    perror("write error");
  }

  return write_rc;
}

int main(int argc, char *argv[]) {
  mkfifo(INGOING, 0666);

  printf("Welcome to server.\n");
  printf("channel for reading messages to server is %s\n", INGOING);

  int in_fd = open(INGOING, O_WRONLY);
  if (in_fd == -1) {
    perror("open error");
    return 1;
  }

  int argi = 1;
  while (argi < argc) {
    char *msg = argv[argi];
    int len = strlen(msg);
    ++argi;

    write_fifo(in_fd, msg, len);
    sleep(1);
  }

  return 0;
}

--

The command line arguments are sent as individual messages. To demonstrate
the problem I run with "abcdef ghijkl" as the args. This sends two 6 byte
messages with a short gap between.

I run ffmpeg like so:
ffmpeg -ar 1000 -ac 1 -acodec pcm_s16le -f s16le -probesize 5000000 -i
 /tmp/audio.fifo -map_metadata -1 -vn -ac:a:0 1 -ar:a:0 1000 -codec:a:0
pcm_s16le -sn -f s16le -y  -

Without my patch, you won't see all 12 bytes output, just "kl". My patch
fixes this.

The buffer is in AVIOContext. The problem with the original code was that
max_buffer_size is added to the request, to see if it's larger than another
value that is also max_buffer_size. Like, "if (A + B) < B".


On Tue, May 16, 2017 at 9:06 AM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Mon, May 15, 2017 at 05:55:28PM +0000, Rob Meyers wrote:
> > Of course.
> >
> > We noticed when reading data from a named pipe the first 10 bytes would
> get
> > dropped.
>
> from were are bytes droped ?
> this is the internal buffer, not something a user should touch
> directly.
>
>
> > I traced this to the affected code in fill_buffer(). The
> > assignment of "dst" was always set to the beginning of the buffer, and if
>
> The buffer pointer resets to the start if there is not enough space
> available to hold another packet.
> with your patch it resets either always or when the buffer is filled
> to the last byte randomly truncating a read.
> If the read function is incapable to fullfill the partial packet read
> that will not work out i think
> But i may be missing something of course
>
>
>
> > it hadn't been consumed yet the data would be overwritten. We could
> > reproduce this by setting up a server that writes to the named pipe in
> two
> > small (6 byte) messages with a 1 second gap between. Without the gap, or
> if
> > the data is sent as one message, there's no problem. It's in the
> > accumulation of data between messages to fulfill a read that this bug is
> > triggered.
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who are best at talking, realize last or never when they are wrong.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>