| Message ID | CANWt732WNW_chAH2KQemmShph_5HzbEtPn7gSDh7Ex0vmQZ2sw@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sat, 25 Aug 2018, Tudor Suciu wrote: > Hello, > > Is this patch in a better shape for inclusion? The code seems OK, please resend the patch with a docs/protocols.texi update for the new option. Thanks, Marton > > Regards, > > On Fri, Aug 24, 2018 at 2:46 AM mypopy@gmail.com <mypopy@gmail.com> wrote: > >> On Fri, Aug 24, 2018 at 2:55 AM Marton Balint <cus@passwd.hu> wrote: >>> >>> >>> >>> On Thu, 23 Aug 2018, mypopy@gmail.com wrote: >>> >>>> On Wed, Aug 22, 2018 at 4:30 AM Tudor Suciu <tudor.suciu@gmail.com> >> wrote: >>>>> >>>>> Hello, >>>>> >>>>> I get errors when I try to send a live srt stream that the first >> packet is >>>>> too big: >>>>> 21:30:39.896626/ffmpeg*E: SRT.c: LiveSmoother: payload size: 1504 >> exceeds >>>>> maximum allowed 1316 >>>>> >>>>> Here are example commands for server and client: >>>>> ffmpeg -re -i ~/Downloads/ToS-4k-1920.mov -vcodec libx264 -g 50 -refs >> 1 -s >>>>> 640x360 -b:v 1000k -acodec aac -b:a 64k -flush_packets 0 -f mpegts >> "srt:// >>>>> 127.0.0.1:5555?mode=listener" >>>>> ffplay srt://127.0.0.1:5555 >>>>> >>>>> A patch that fully solves the issue is: >>>>> diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c >>>>> index 0f9529d263..156a4776e2 100644 >>>>> --- a/libavformat/libsrt.c >>>>> +++ b/libavformat/libsrt.c >>>>> @@ -412,6 +412,8 @@ static int libsrt_open(URLContext *h, const char >> *uri, >>>>> int flags) >>>>> return AVERROR_UNKNOWN; >>>>> } >>>>> >>>>> + h->max_packet_size = 1316; >>>>> + >>>>> /* SRT options (srt/srt.h) */ >>>>> p = strchr(uri, '?'); >>>>> if (p) { >>>>> >>>>> How would you like this option to be made work in a way that can be >>>>> accepted in ffmpeg? >>>>> Is there a way to change the max packet size without this patch? >>>>> >>>> In your case, I don't think hard coding max packet size == 1316 is not >>>> a good idea in loopback device, and after deep in the srt >>>> library(https://github.com/Haivision/srt) source code, I think srt >>>> library need to fix the hardcode about packet size limition. >>> >>> I can't think of a scenario where limiting the packet size actually >> causes >>> problems, but if you insist on not limiting it in general, then the max >>> packet size should be settable using an option, same way it is for UDP. >>> (pkt_size option). I'd suggest using 1316 as default, because that >>> is the more common (and currently the only working) use case... >>> >>> Thanks, >>> Marton >> I agree with you, giving an option like pkt_size and using 1316 as the >> default in this case is a better way to fix this issue. >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >
On Sat, 25 Aug 2018, Marton Balint wrote: > > > On Sat, 25 Aug 2018, Tudor Suciu wrote: > >> Hello, >> >> Is this patch in a better shape for inclusion? > > The code seems OK, please resend the patch with a docs/protocols.texi > update for the new option. On the second look, there are some strange things: > @@ -73,6 +74,7 @@ static const AVOption libsrt_options[] = { > { "listen_timeout", "Connection awaiting timeout", OFFSET(listen_timeout), AV_OPT_TYPE_INT64, { .i64 = -1 }, -1, INT64_MAX, .flags = D|E }, > { "send_buffer_size", "Socket send buffer size (in bytes)", OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, .flags = D|E }, > { "recv_buffer_size", "Socket receive buffer size (in bytes)", OFFSET(recv_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, .flags = D|E }, > + { "pkt_size", "Maximum SRT packet size", OFFSET(pkt_size), AV_OPT_TYPE_INT, { .i64 = 1316 }, -1, INT_MAX, .flags = D|E }, minimum value is 1, not -1. > + if (flags & AVIO_FLAG_WRITE) { > + h->max_packet_size = 1316; > + } > + See below. > @@ -466,6 +475,9 @@ static int libsrt_open(URLContext *h, const char *uri, int flags) > } > } > } > + if (flags & AVIO_FLAG_WRITE && s->pkt_size > 0) { > + h->max_packet_size = s->pkt_size; > + } You can simplify this to: if (flags & AVIO_FLAG_WRITE) h->max_packet_size = s->pkt_size > 0 ? s->pkt_size : 1316; This way the chunk before this one is not needed. Thanks, Marton
From 5fa19ee0d27eab5181cec57a858f086fecc0615e Mon Sep 17 00:00:00 2001 From: Tudor Suciu <tudor.suciu@gmail.com> Date: Sat, 25 Aug 2018 11:50:07 +0200 Subject: [PATCH] add pkt_length to libsrt --- libavformat/libsrt.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/libavformat/libsrt.c b/libavformat/libsrt.c index 0f9529d263..6c147d39e4 100644 --- a/libavformat/libsrt.c +++ b/libavformat/libsrt.c @@ -48,6 +48,7 @@ typedef struct SRTContext { int64_t listen_timeout; int recv_buffer_size; int send_buffer_size; + int pkt_size; int64_t maxbw; int pbkeylen; @@ -73,6 +74,7 @@ static const AVOption libsrt_options[] = { { "listen_timeout", "Connection awaiting timeout", OFFSET(listen_timeout), AV_OPT_TYPE_INT64, { .i64 = -1 }, -1, INT64_MAX, .flags = D|E }, { "send_buffer_size", "Socket send buffer size (in bytes)", OFFSET(send_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, .flags = D|E }, { "recv_buffer_size", "Socket receive buffer size (in bytes)", OFFSET(recv_buffer_size), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, INT_MAX, .flags = D|E }, + { "pkt_size", "Maximum SRT packet size", OFFSET(pkt_size), AV_OPT_TYPE_INT, { .i64 = 1316 }, -1, INT_MAX, .flags = D|E }, { "maxbw", "Maximum bandwidth (bytes per second) that the connection can use", OFFSET(maxbw), AV_OPT_TYPE_INT64, { .i64 = -1 }, -1, INT64_MAX, .flags = D|E }, { "pbkeylen", "Crypto key len in bytes {16,24,32} Default: 16 (128-bit)", OFFSET(pbkeylen), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 32, .flags = D|E }, { "passphrase", "Crypto PBKDF2 Passphrase size[0,10..64] 0:disable crypto", OFFSET(passphrase), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = D|E }, @@ -412,6 +414,10 @@ static int libsrt_open(URLContext *h, const char *uri, int flags) return AVERROR_UNKNOWN; } + if (flags & AVIO_FLAG_WRITE) { + h->max_packet_size = 1316; + } + /* SRT options (srt/srt.h) */ p = strchr(uri, '?'); if (p) { @@ -442,6 +448,9 @@ static int libsrt_open(URLContext *h, const char *uri, int flags) if (av_find_info_tag(buf, sizeof(buf), "oheadbw", p)) { s->oheadbw = strtoll(buf, NULL, 10); } + if (av_find_info_tag(buf, sizeof(buf), "pkt_size", p)) { + s->pkt_size = strtol(buf, NULL, 10); + } if (av_find_info_tag(buf, sizeof(buf), "tsbpddelay", p)) { s->tsbpddelay = strtol(buf, NULL, 10); } @@ -466,6 +475,9 @@ static int libsrt_open(URLContext *h, const char *uri, int flags) } } } + if (flags & AVIO_FLAG_WRITE && s->pkt_size > 0) { + h->max_packet_size = s->pkt_size; + } return libsrt_setup(h, uri, flags); } -- 2.11.0