[FFmpeg-devel] [PATCH] libstr.c: Fixed rendezvous mode

[Moritz Barsnick]

On Fri, Dec 07, 2018 at 14:01:03 +0000, Adrian Grzeca wrote:

Please read the coding guidelines here:
https://www.ffmpeg.org/developer.html#Coding-Rules-1

Your indentation is incorrect, incl. use of tabs, which is not
supported in the ffmpeg code base. Also, your brackets and whitespace
are incorrect.

Furthermore, your commit message says "Fixed rendezvous mode", when
your change gives the impression that you're adding features:

> +	{ "localip",     "IP address of network card to use in rendezvous mode (Req. in rendezvous mode)",             OFFSET(localip),       AV_OPT_TYPE_STRING,   { .str = NULL },              .flags = D|E },
> +	{ "localport",     "Source port to use in rendezvous mode (Req. in rendezvous mode)",             OFFSET(localport),       AV_OPT_TYPE_STRING,   { .str = NULL },              .flags = D|E },

You need to explain in more detail what the actual issue is (is there a
trac ticket for this?), and how you are fixing it.

Furthermore:
> Subject: libstr.c: Fixed rendezvous mode

An ffmpeg commit message, or at least its header line, would read:
  avformat/libsrt: fix rendezvous mode

> +	{ "localip",     "IP address of network card to use in rendezvous mode (Req. in rendezvous mode)",             OFFSET(localip),       AV_OPT_TYPE_STRING,   { .str = NULL },              .flags = D|E },
                                                                                ^
What does "Req." mean and why is it capitalized?

> +		if(s->localip == NULL || s->localport == NULL) {

ffmpeg's style would be
    if (!s->localip || !s->localport) {

Your code may have further issues which I cannot judge on.

Cheers,
Moritz