On 10.02.2016 10:59, Paul B Mahol wrote:
On 2/10/16, Tobias Rapp <t.r...@noa-archive.com> wrote:
On 10.02.2016 10:01, Paul B Mahol wrote:
On 2/10/16, Tobias Rapp <t.r...@noa-archive.com> wrote:
On 06.02.2016 23:30, Paul B Mahol wrote:
On 2/6/16, Paul B Mahol <one...@gmail.com> wrote:
Hi,

patch attached.


Improved version attached.

[...]
+
+static int string(const char *value1, const char *value2, size_t
length)
+{
+    return !strncmp(value1, value2, length);
+}

If I understand correctly this function is used to compare if the start
of value2 matches value1. Maybe this function should be called
"starts_with" (also in the "function" option enum)?

+
+static int equal(const char *value1, const char *value2, size_t length)
+{
+    float f1, f2;
+
+    if (sscanf(value1, "%f", &f1) + sscanf(value2, "%f", &f2) != 2)
+        return 0;
+
+    return f1 != f2;
+}
+
+static int less(const char *value1, const char *value2, size_t length)
+{
+    float f1, f2;
+
+    if (sscanf(value1, "%f", &f1) + sscanf(value2, "%f", &f2) != 2)
+        return 0;
+
+    return f1 > f2;
+}
+
+static int greater(const char *value1, const char *value2, size_t
length)
+{
+    float f1, f2;
+
+    if (sscanf(value1, "%f", &f1) + sscanf(value2, "%f", &f2) != 2)
+        return 0;
+
+    return f1 < f2;
+}
+
[...]

I think it would be better to not compare float values directly with
"==", "<" or ">". Instead use some code like "fabsf(f1 - f2) <= epsilon".

BTW: Is the return value of "equal", "less" and "greater" inverse on
purpose?

Not for equal, but for other it is.

OK, now I got it that the argument order is flipped but matches the
documentation. In that case it is a bit counter-intuitive that one has
to do something like

mode=print:key=mykey:value=1.0:function=less

to print all metadata values of "mykey" which are *bigger* than 1.0.


Nope, this works fine here. It prints values less than 1.0

Then I have problems understanding the documentation text of "less" and "greater".


Another sidenote: I have seen that some filters use a common expression
language (e.g. aeval, crop, scale). Not sure if this expression language
supports string operators or if it is limited to numbers but in my
opinion it would be great to re-use it here.

Added and applied. Filter is easily extendable.

Thanks for adding expression support.

I know my response has come late and I'm only an occasional FFmpeg
developer so my thoughts might not have big weight. Still I think it
would have been more polite to first respond on the list to all my
comments (see the "starts_with" suggestion above) before applying the patch.

You can post another patch anytime here and if it makes sense I will
gladly apply it.

OK.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to