diff mbox series

[FFmpeg-devel,5/6] avformat/udp: clarify option description for timeout unit

Message ID 1602686103-3427-5-git-send-email-lance.lmwang@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel,1/6] avformat/rtpdec: update the previous with new seq | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

Lance Wang Oct. 14, 2020, 2:35 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nicolas George Oct. 14, 2020, 3:03 p.m. UTC | #1 
lance.lmwang@gmail.com (12020-10-14):
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/udp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 30d8041..ee5fa84 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -138,7 +138,7 @@ static const AVOption options[] = {
>      { "connect",        "set if connect() should be called on socket",     OFFSET(is_connected),   AV_OPT_TYPE_BOOL,   { .i64 =  0 },     0, 1,       .flags = D|E },
>      { "fifo_size",      "set the UDP receiving circular buffer size, expressed as a number of packets with size of 188 bytes", OFFSET(circular_buffer_size), AV_OPT_TYPE_INT, {.i64 = 7*4096}, 0, INT_MAX, D },
>      { "overrun_nonfatal", "survive in case of UDP receiving circular buffer overrun", OFFSET(overrun_nonfatal), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1,    D },
> -    { "timeout",        "set raise error timeout (only in read mode)",     OFFSET(timeout),        AV_OPT_TYPE_INT,    { .i64 = 0 },      0, INT_MAX, D },

> +    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },

You broke spacing.

I think it would be better to deprecate all these integer options and
replace them by proper durations.

>      { "sources",        "Source list",                                     OFFSET(sources),        AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
>      { "block",          "Block list",                                      OFFSET(block),          AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
>      { NULL }

Regards,
Lance Wang Oct. 15, 2020, 1:28 a.m. UTC | #2 
On Wed, Oct 14, 2020 at 05:03:56PM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12020-10-14):
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/udp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 30d8041..ee5fa84 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -138,7 +138,7 @@ static const AVOption options[] = {
> >      { "connect",        "set if connect() should be called on socket",     OFFSET(is_connected),   AV_OPT_TYPE_BOOL,   { .i64 =  0 },     0, 1,       .flags = D|E },
> >      { "fifo_size",      "set the UDP receiving circular buffer size, expressed as a number of packets with size of 188 bytes", OFFSET(circular_buffer_size), AV_OPT_TYPE_INT, {.i64 = 7*4096}, 0, INT_MAX, D },
> >      { "overrun_nonfatal", "survive in case of UDP receiving circular buffer overrun", OFFSET(overrun_nonfatal), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1,    D },
> > -    { "timeout",        "set raise error timeout (only in read mode)",     OFFSET(timeout),        AV_OPT_TYPE_INT,    { .i64 = 0 },      0, INT_MAX, D },
> 
> > +    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },
> 
> You broke spacing.

I'll break into two line to make it better.

> 
> I think it would be better to deprecate all these integer options and
> replace them by proper durations.

now too many use case and difficult to test all of them, so I prefer to make it clear for description instead of deprecated.
$grep timeout libavformat/*.c |grep AV_OPT_TYPE_INT |wc -l
18

> 
> >      { "sources",        "Source list",                                     OFFSET(sources),        AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
> >      { "block",          "Block list",                                      OFFSET(block),          AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
> >      { NULL }
> 
> Regards,
> 
> -- 
>   Nicolas George
Nicolas George Oct. 15, 2020, 8:09 a.m. UTC | #3 
lance.lmwang@gmail.com (12020-10-15):
> > > +    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },
> > You broke spacing.
> I'll break into two line to make it better.

No need. Just fix the spacing in the string.

Regards,
Lance Wang Oct. 15, 2020, 10:41 a.m. UTC | #4 
On Thu, Oct 15, 2020 at 10:09:37AM +0200, Nicolas George wrote:
> lance.lmwang@gmail.com (12020-10-15):
> > > > +    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },
> > > You broke spacing.
> > I'll break into two line to make it better.
> 
> No need. Just fix the spacing in the string.

thanks, fix it locally.

> 
> Regards,
> 
> -- 
>   Nicolas George



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Marton Balint Oct. 16, 2020, 9:03 p.m. UTC | #5 
On Wed, 14 Oct 2020, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/udp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 30d8041..ee5fa84 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -138,7 +138,7 @@ static const AVOption options[] = {
>     { "connect",        "set if connect() should be called on socket",     OFFSET(is_connected),   AV_OPT_TYPE_BOOL,   { .i64 =  0 },     0, 1,       .flags = D|E },
>     { "fifo_size",      "set the UDP receiving circular buffer size, expressed as a number of packets with size of 188 bytes", OFFSET(circular_buffer_size), AV_OPT_TYPE_INT, {.i64 = 7*4096}, 0, INT_MAX, D },
>     { "overrun_nonfatal", "survive in case of UDP receiving circular buffer overrun", OFFSET(overrun_nonfatal), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1,    D },
> -    { "timeout",        "set raise error timeout (only in read mode)",     OFFSET(timeout),        AV_OPT_TYPE_INT,    { .i64 = 0 },      0, INT_MAX, D },
> +    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },

                                                a space got removed ^^^

Thanks,
Marton

>     { "sources",        "Source list",                                     OFFSET(sources),        AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
>     { "block",          "Block list",                                      OFFSET(block),          AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
>     { NULL }
> -- 
> 1.8.3.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang Oct. 17, 2020, 1:27 a.m. UTC | #6 
On Fri, Oct 16, 2020 at 11:03:00PM +0200, Marton Balint wrote:
> 
> 
> On Wed, 14 Oct 2020, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavformat/udp.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 30d8041..ee5fa84 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -138,7 +138,7 @@ static const AVOption options[] = {
> >     { "connect",        "set if connect() should be called on socket",     OFFSET(is_connected),   AV_OPT_TYPE_BOOL,   { .i64 =  0 },     0, 1,       .flags = D|E },
> >     { "fifo_size",      "set the UDP receiving circular buffer size, expressed as a number of packets with size of 188 bytes", OFFSET(circular_buffer_size), AV_OPT_TYPE_INT, {.i64 = 7*4096}, 0, INT_MAX, D },
> >     { "overrun_nonfatal", "survive in case of UDP receiving circular buffer overrun", OFFSET(overrun_nonfatal), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1,    D },
> > -    { "timeout",        "set raise error timeout (only in read mode)",     OFFSET(timeout),        AV_OPT_TYPE_INT,    { .i64 = 0 },      0, INT_MAX, D },
> > +    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },
> 
>                                                a space got removed ^^^
> 
thanks, have fixed locally.

> Thanks,
> Marton
> 
> >     { "sources",        "Source list",                                     OFFSET(sources),        AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
> >     { "block",          "Block list",                                      OFFSET(block),          AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
> >     { NULL }
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 30d8041..ee5fa84 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -138,7 +138,7 @@  static const AVOption options[] = {
     { "connect",        "set if connect() should be called on socket",     OFFSET(is_connected),   AV_OPT_TYPE_BOOL,   { .i64 =  0 },     0, 1,       .flags = D|E },
     { "fifo_size",      "set the UDP receiving circular buffer size, expressed as a number of packets with size of 188 bytes", OFFSET(circular_buffer_size), AV_OPT_TYPE_INT, {.i64 = 7*4096}, 0, INT_MAX, D },
     { "overrun_nonfatal", "survive in case of UDP receiving circular buffer overrun", OFFSET(overrun_nonfatal), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1,    D },
-    { "timeout",        "set raise error timeout (only in read mode)",     OFFSET(timeout),        AV_OPT_TYPE_INT,    { .i64 = 0 },      0, INT_MAX, D },
+    { "timeout",        "set raise error timeout, in microseconds(only in read mode)",OFFSET(timeout),          AV_OPT_TYPE_INT,  {.i64 = 0}, 0, INT_MAX, D },
     { "sources",        "Source list",                                     OFFSET(sources),        AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
     { "block",          "Block list",                                      OFFSET(block),          AV_OPT_TYPE_STRING, { .str = NULL },               .flags = D|E },
     { NULL }