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

Michael Niedermayer michael at niedermayer.cc
Tue May 16 23:23:32 EEST 2017


On Tue, May 16, 2017 at 04:17:45PM +0000, Rob Meyers wrote:
> 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 patch makes it output all 12 bytes but this is more by luck than
by fixing the issue.

The issue is, well there are at least 2 issues
First the ff_id3v2_read_dict() Tries to read 10 bytes and failing to
find a id3 header seeks back.
This is buggy, it must call ffio_ensure_seekback() before it starts
any reading that it intends to seek back over.

the 2nd issue seems to be that fill_buffer() throws the larger
buffer ffio_ensure_seekback() created out too early.
I didnt investigate this deeply but something goes wrong there

and with ffio_ensure_seekback() and with the downallocation commented
the whole works and returns the 12 bytes



> 
> 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".

It may be that the buffer size and the maximum packet size are often
the same.  But they are not always the same, for example
ffio_ensure_seekback() makes them different

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170516/266d67c5/attachment.sig>


More information about the ffmpeg-devel mailing list