Thanks,
Eran
-----Original Message-----
From: Eran Kornblau
Sent: Wednesday, December 23, 2020 8:14 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: FW: [FFmpeg-devel] [PATCH] http: support retry on connection error
Pinging again... hope this can be merged...
-----Original Message-----
From: Eran Kornblau
Sent: Wednesday, December 16, 2020 9:18 PM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: FW: [FFmpeg-devel] [PATCH] http: support retry on connection error
Ping...
-----Original Message-----
From: Eran Kornblau
Sent: Thursday, December 10, 2020 8:25 AM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: RE: [FFmpeg-devel] [PATCH] http: support retry on connection error
Resending...
-----Original Message-----
From: Eran Kornblau
Sent: Thursday, December 3, 2020 10:52 AM
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: RE: [FFmpeg-devel] [PATCH] http: support retry on connection error
Hi Marton,
Thank you for the feedback, and sorry for my late reply :) Please see my
responses inline, updated patch attached.
Thanks
Eran
-----Original Message-----
From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of
Marton Balint
Sent: Wednesday, November 18, 2020 10:46 PM
To: FFmpeg development discussions and patches
<ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] http: support retry on connection
error
- reconnect_on_status - a list of http status codes that should be
retried. the list can contain explicit status codes / the strings
4xx/5xx.
Maybe better name this option reconnect_on_http_error. Also this is a
flags-like option, so use AV_OPT_TYPE_FLAGS with 4xx and 5xx as flags.
Rename - done.
Flags - the patch supports both explicit status codes (e.g. 503) and status
groups (e.g. 4xx).
For example, it is possible to specify -reconnect_on_http_error '4xx,503'.
In my understanding, AV_OPT_TYPE_FLAGS is a simple int, so the only to support
what I wrote above, is to assign bits to specific status codes, and I don't
think we want to go there... (because every time someone will want to retry on
a new status code, we will need to update the code)
- reconnect_on_err - reconnects on arbitrary errors during connect, e.g.
ECONNRESET/ETIMEDOUT
Maybe reconnect_on_network_error better reflects that this reconnects
on underlying protocol (TCP/TLS) error.
Anyhow, the new options should be added to docs/protocols.texi.
Rename & docs - done.
+ { "reconnect_on_err", "auto reconnect in case of tcp/tls error during
connect", OFFSET(reconnect_on_err), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, D },
+ { "reconnect_on_status", "list of http status codes to
+ reconnect on. the list can include specific status codes / 4xx /
+ 5xx", OFFSET(reconnect_on_status), AV_OPT_TYPE_STRING, { .str =
+ NULL }, 0, 0, D },
AV_OPT_TYPE_FLAGS, as I explained above.
Replied above
location_changed = http_open_cnx_internal(h, options);
Are you sure you get AVERROR_HTTP_* here? Also if you follow the code
there is special handling of 401/407, that should be done first before
your checks, no?
1. Yes, I tested the change before submitting, used some test PHP server to
simulate errors.
This is the call stack that propagates the AVERROR_HTTP_* codes -
http_open_cnx_internal
http_connect
http_read_header
process_line
check_http_code
2. In case of 401/407, check_http_code returns 0, and therefore
http_open_cnx_internal return 0 as well (assuming there is no other error...).
So, it doesn't enter 'if (location_changed < 0)' and it behaves the same way
it did before my changes.
Another comment is that if you want to reconnect based on errors
classified as 4xx or 5xx, then probably you should revisit
ff_http_averror(400, xxx) calls and check if they are indeed caused by
client behaviour. Because if the server is violating the protocol,
then they should be ff_http_averror(500, xxx) IMHO.
Checked the code, I see 2 cases of ff_http_averror(400, xxx) - 1. HTTP server
received an unexpected http method (would have been more correct to use 405,
but don't think it matters...) 2. HTTP server received an invalid http version
string So, I think it's fine.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fffmp
eg.org%2Fmailman%2Flistinfo%2Fffmpeg-devel&data=04%7C01%7Ceran.kor
nblau%40kaltura.com%7C590db1ef6c4d484bb34508d88c030453%7C0c503748de3f4
e2597e26819d53a42b6%7C0%7C0%7C637413291875784243%7CUnknown%7CTWFpbGZsb
3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
7C1000&sdata=hemm499pmQQQEDOc4FXeq1EThvOT7FyfjWRsylFvdm0%3D&re
served=0
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
