| Message ID | 20171013190349.24112-1-daniel.kucera@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Oct 13, 2017 at 09:03:47PM +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 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) breaks fate-indeo3-2 [...]
breaks fate-indeo3-2 [...] -- You need to apply the whole set. It passed all fate tests on my machine.
On Sat, Oct 14, 2017 at 07:10:37AM +0200, Daniel Kučera wrote: > breaks fate-indeo3-2 > > [...] > -- > > > You need to apply the whole set. It passed all fate tests on my machine. I applied the whole set and bisected to find which patch caused the failure also the i-th patch of a set must pass fate with it and the 0th to ith patches applied, otherwise there would be checkouts that fail some tests [...]
2017-10-14 17:46 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>: > On Sat, Oct 14, 2017 at 07:10:37AM +0200, Daniel Kučera wrote: >> breaks fate-indeo3-2 >> >> [...] >> -- >> >> >> You need to apply the whole set. It passed all fate tests on my machine. > > I applied the whole set and bisected to find which patch caused the > failure > > also the i-th patch of a set must pass fate with it and the 0th to ith > patches applied, otherwise there would be checkouts that fail some tests > So should I reorder them and post again?
2017-10-14 19:16 GMT+02:00 Daniel Kučera <daniel.kucera@gmail.com>: > 2017-10-14 17:46 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>: >> On Sat, Oct 14, 2017 at 07:10:37AM +0200, Daniel Kučera wrote: >>> breaks fate-indeo3-2 >>> >>> [...] >>> -- >>> >>> >>> You need to apply the whole set. It passed all fate tests on my machine. >> >> I applied the whole set and bisected to find which patch caused the >> failure >> >> also the i-th patch of a set must pass fate with it and the 0th to ith >> patches applied, otherwise there would be checkouts that fail some tests >> > > So should I reorder them and post again? > > What if they don't pass fate in any order? should I post them as one patch?
On 10/14/2017 2:22 PM, Daniel Kučera wrote: > 2017-10-14 19:16 GMT+02:00 Daniel Kučera <daniel.kucera@gmail.com>: >> 2017-10-14 17:46 GMT+02:00 Michael Niedermayer <michael@niedermayer.cc>: >>> On Sat, Oct 14, 2017 at 07:10:37AM +0200, Daniel Kučera wrote: >>>> breaks fate-indeo3-2 >>>> >>>> [...] >>>> -- >>>> >>>> >>>> You need to apply the whole set. It passed all fate tests on my machine. >>> >>> I applied the whole set and bisected to find which patch caused the >>> failure >>> >>> also the i-th patch of a set must pass fate with it and the 0th to ith >>> patches applied, otherwise there would be checkouts that fail some tests >>> >> >> So should I reorder them and post again? >> >> > > What if they don't pass fate in any order? should I post them as one patch? If one patch introduces a problem that gets fixed by a subsequent patch then yes, both should be squashed into one to make sure the problem is never introduced. The idea is that no patch/commit on its own should generate a fate failure of any kind, as it would make bisecting unrelated future issues harder.
> > If one patch introduces a problem that gets fixed by a subsequent patch > then yes, both should be squashed into one to make sure the problem is > never introduced. > > The idea is that no patch/commit on its own should generate a fate > failure of any kind, as it would make bisecting unrelated future issues > harder. Thank you for explaining. I've sent squashed patch.
Le duodi 22 vendémiaire, an CCXXVI, Daniel Kucera a écrit : > 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 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) I think the changes in this series are mostly correct, but the fact they cause failure if they are applied separately is a little worrying. There is something subtly wrong going on here. I think there is another instance of the problem hidden somewhere that is causing more complication. I have been trying to find it, but not yet. Still on it when I have time. Regards,
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;
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 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)