On Fri, 7 Dec 2018, Moritz Barsnick wrote:
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 },
Please use option name "localaddr" instead because I believe UDP protocol
has a similar option.
+ { "localport", "Source port to use in rendezvous mode (Req. in
rendezvous mode)", OFFSET(localport), AV_OPT_TYPE_STRING, { .str = NULL },
.flags = D|E },
AV_OPT_TYPE_INT
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?
I'd just drop the extra info parenthesis, the text before that makes it
clear that these options are used in rendezvous mode only.
+ 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.
doc/protocols.texi update is missing.
Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
