| Message ID | 20171014172734.17488-1-daniel.kucera@gmail.com |
|---|---|
| State | Withdrawn |
| Headers | show |
On Sat, Oct 14, 2017 at 19:27:34 +0200, Daniel Kucera wrote: > Subject: [FFmpeg-devel] [PATCH] libavformat/cache: don't treat 0 as EOF > libavformat/aviobuf: don't treat 0 as EOF libavformat/avio: > retry_transfer_wrapper: don't treat 0 as EOF Something went wrong with your commit message. (I think, after squashing by cherry-picking, you just need to re-edit the commit message explicitly.) Moritz
On Sat, Oct 14, 2017 at 07:27:34PM +0200, Daniel Kucera wrote: > transfer_func variable passed to retry_transfer_wrapper > are h->prot->url_read and h->prot->url_write functions. > These need to return EOF or other error properly. > In case of returning >= 0, url_read/url_write is retried > until error is returned. > > Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> > --- > libavformat/avio.c | 6 ++++-- > libavformat/aviobuf.c | 20 ++++++++++++-------- > libavformat/cache.c | 4 ++-- > 3 files changed, 18 insertions(+), 12 deletions(-) This changes: ./ffmpeg -y -i 'concat:matrixbench_mpeg2.mpg|matrixbench_mpeg2.mpg' -vsync 0 -an file.avi [...]
On 10/15/2017 9:38 PM, Michael Niedermayer wrote: > On Sat, Oct 14, 2017 at 07:27:34PM +0200, Daniel Kucera wrote: >> transfer_func variable passed to retry_transfer_wrapper >> are h->prot->url_read and h->prot->url_write functions. >> These need to return EOF or other error properly. >> In case of returning >= 0, url_read/url_write is retried >> until error is returned. >> >> Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> >> --- >> libavformat/avio.c | 6 ++++-- >> libavformat/aviobuf.c | 20 ++++++++++++-------- >> libavformat/cache.c | 4 ++-- >> 3 files changed, 18 insertions(+), 12 deletions(-) > > This changes: > ./ffmpeg -y -i 'concat:matrixbench_mpeg2.mpg|matrixbench_mpeg2.mpg' -vsync 0 -an file.avi In case he doesn't have this sample: http://samples.ffmpeg.org/benchmark/testsuite1/matrixbench_mpeg2.mpg Basically, with the patch applied the above command doesn't concatenate the input as requested.
> This changes: > ./ffmpeg -y -i 'concat:matrixbench_mpeg2.mpg|matrixbench_mpeg2.mpg' -vsync 0 -an file.avi In case he doesn't have this sample: http://samples.ffmpeg.org/benchmark/testsuite1/matrixbench_mpeg2.mpg Basically, with the patch applied the above command doesn't concatenate the input as requested. Is there a reason why this test is not included in fate?
On 10/16/2017 12:10 AM, Daniel Kučera wrote: >> This changes: >> ./ffmpeg -y -i 'concat:matrixbench_mpeg2.mpg|matrixbench_mpeg2.mpg' > -vsync 0 -an file.avi > > In case he doesn't have this sample: > http://samples.ffmpeg.org/benchmark/testsuite1/matrixbench_mpeg2.mpg > > Basically, with the patch applied the above command doesn't concatenate > the input as requested. > > > Is there a reason why this test is not included in fate? Tests are added as regressions are found in some cases, and as features are added in others. There's no concat protocol test in FATE currently, which is obviously not ideal. I see there's a cut version of this file in the fate samples suite, mpeg2/matrixbench_mpeg2.lq1.mpg, which is enough to reproduce this issue. A test could be created with it, but i'm not sure how to make a working command line with it for fate that will not trip in some cases depending on what's the path to the samples folder (Windows paths are especially annoying in this regard).
> > This changes: > ./ffmpeg -y -i 'concat:matrixbench_mpeg2.mpg|matrixbench_mpeg2.mpg' -vsync 0 -an file.avi > fixed, sending new patch.
diff --git a/libavformat/avio.c b/libavformat/avio.c index 64248e098b..d3004c007f 100644 --- a/libavformat/avio.c +++ b/libavformat/avio.c @@ -391,8 +391,10 @@ static inline int retry_transfer_wrapper(URLContext *h, uint8_t *buf, } av_usleep(1000); } - } else if (ret < 1) - return (ret < 0 && ret != AVERROR_EOF) ? ret : len; + } else if (ret == AVERROR_EOF) + return (len > 0) ? len : AVERROR_EOF; + else if (ret < 0) + return ret; if (ret) { fast_retries = FFMAX(fast_retries, 2); wait_since = 0; diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c index 636cb46161..0d4eb051e1 100644 --- a/libavformat/aviobuf.c +++ b/libavformat/aviobuf.c @@ -572,13 +572,14 @@ static void fill_buffer(AVIOContext *s) if (s->read_packet) len = s->read_packet(s->opaque, dst, len); else - len = 0; - if (len <= 0) { + len = AVERROR_EOF; + if (len == AVERROR_EOF) { /* do not modify buffer if EOF reached so that a seek back can be done without rereading data */ s->eof_reached = 1; - if (len < 0) - s->error = len; + } else if (len < 0) { + s->eof_reached = 1; + s->error= len; } else { s->pos += len; s->buf_ptr = dst; @@ -646,13 +647,16 @@ int avio_read(AVIOContext *s, unsigned char *buf, int size) // bypass the buffer and read data directly into buf if(s->read_packet) len = s->read_packet(s->opaque, buf, size); - - if (len <= 0) { + else + len = AVERROR_EOF; + if (len == AVERROR_EOF) { /* do not modify buffer if EOF reached so that a seek back can be done without rereading data */ s->eof_reached = 1; - if(len<0) - s->error= len; + break; + } else if (len < 0) { + s->eof_reached = 1; + s->error= len; break; } else { s->pos += len; diff --git a/libavformat/cache.c b/libavformat/cache.c index 6aabca2e78..66bbbf54c9 100644 --- a/libavformat/cache.c +++ b/libavformat/cache.c @@ -201,7 +201,7 @@ static int cache_read(URLContext *h, unsigned char *buf, int size) } r = ffurl_read(c->inner, buf, size); - if (r == 0 && size>0) { + if (r == AVERROR_EOF && size>0) { c->is_true_eof = 1; av_assert0(c->end >= c->logical_pos); } @@ -263,7 +263,7 @@ resolve_eof: if (whence == SEEK_SET) size = FFMIN(sizeof(tmp), pos - c->logical_pos); ret = cache_read(h, tmp, size); - if (ret == 0 && whence == SEEK_END) { + if (ret == AVERROR_EOF && whence == SEEK_END) { av_assert0(c->is_true_eof); goto resolve_eof; }
transfer_func variable passed to retry_transfer_wrapper are h->prot->url_read and h->prot->url_write functions. These need to return EOF or other error properly. In case of returning >= 0, url_read/url_write is retried until error is returned. Signed-off-by: Daniel Kucera <daniel.kucera@gmail.com> --- libavformat/avio.c | 6 ++++-- libavformat/aviobuf.c | 20 ++++++++++++-------- libavformat/cache.c | 4 ++-- 3 files changed, 18 insertions(+), 12 deletions(-)