[FFmpeg-devel] [PATCH 1/3] libavfilter/scale2ref: Add constants for the non-ref input
Variables pertaining to the non-reference video are now available when
using the scale2ref filter. This allows scaling a video with another
as a reference point while maintaining the original aspect ratio of
the non-reference video.
Consider the following graph: scale2ref=iw/6:-1 [scaled][ref]
This will scale [scaled] to 1/6 the width of [ref] while maintaining
the aspect ratio. This works well when the AR of [ref] is equal to
the AR of [scaled] only. What the above filter really does is
maintain the AR of [ref] when scaling [scaled]. So in all non-same-AR
situations [scaled] will appear stretched or compressed to conform to
the same AR of the reference video. Without doing this calculation
externally there is no way to scale in reference to another input
while maintaining AR in libavfilter.
To make this possible, we introduce seven new constants to be used
in the w and h expressions only in the scale2ref filter:
* other_w/other_h: width/height of the input video being scaled
* other_a: aspect ratio of the input video being scaled
* other_sar: sample aspect ratio of the input video being scaled
* other_dar: display aspect ratio of the input video being scaled
* other_hsub/other_vsub: horiz/vert chroma subsample vals of input
Of course the only new constants needed for maintaining the AR is
other_w/other_h or other_a but adding additional constants in line of
what is available for in/out allows for other scaling possibilities I
have not imagined.
So to now scale a video to 1/6 the size of another video using the
width and maintaining its own aspect ratio you can do this:
scale2ref=iw/6:iw/(6*other_a) [scaled][ref]
This is ideal for picture-in-picture configurations where you could
have a square or 4:3 video overlaid on a corner of a larger 16:9
feed all while keeping the scaled video in the corner at its correct
aspect ratio and always the same size relative to the larger video.
I am not fully sold on the other_ prefix. It works but I'm open to
suggestions on something that is more concise or clear. Unfortunately
in_ is most clear but that is taken by the reference input, which in
my opinion, should've been labeled as such (not as in_ but perhaps
ref_). What is the best way to refer to this? The other input? The
scaled input? The non-reference input?
I've tried to re-use as much code as possible. I could not find a way
to avoid duplication of the var_names array. It must now be kept in
sync with the other (the normal one and the scale2ref one) for
everything to work which does not seem ideal. For every new variable
introduced/removed into/from the normal scale filter one must be
added/removed to/from the scale2ref version. Suggestions on how to
avoid var_names duplication are welcome.
var_values has been increased to always be large enough for the
additional scale2ref variables. I do not forsee this being a problem
as the names variable will always be the correct size. From my
understanding of av_expr_parse_and_eval it will stop processing
variables when it runs out of names even though there may be
additional (potentially uninitialized) entries in the values array.
The ideal solution here would be using a variable-length array but
that is unsupported in C90.
This patch does not remove any functionality and is strictly a
feature patch. There are no API changes. Behavior does not change for
any previously valid inputs.
Signed-off-by: Kevin Mark
---
libavfilter/scale.c | 72 ++---
1 file changed, 68 insertions(+), 4 deletions(-)
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 50cd442849..9b4846719d 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -60,6 +60,49 @@ enum var_name {
VARS_NB
};
+/**
+ * This must be kept in sync with var_names so that it is always a
+ * complete list of var_names with the scale2ref specific names
+ * appended. scale2ref values must appear in the order they appear
+ * in the var_name_scale2ref enum but also be below all of the
+ * non-scale2ref specific values.
+ */
+static const char *const var_names_scale2ref[] = {
+"PI",
+"PHI",
+"E",
+"in_w", "iw",
+"in_h", "ih",
+"out_w", "ow",
+"out_h", "oh",
+"a",
+"sar",
+"dar",
+"hsub",
+"vsub",
+"ohsub",
+"ovsub",
+"other_w",
+"other_h",
+"other_a",
+"other_sar",
+"other_dar",
+"other_hsub",
+"other_vsub",
+NULL
+};
+
+enum var_name_scale2ref {
+VAR_S2R_OTHER_W,
+VAR_S2R_OTHER_H,
+VAR_S2R_OTHER_A,
+VAR_S2R_OTHER_SAR,
+VAR_S2R_OTHER_DAR,
+VAR_S2R_OTHER_HSUB,
+VAR_S2R_OTHER_VSUB,
+VARS_S2R_NB
+};
+
int ff_scale_eval_dimensions(void *log_ctx,
const
[FFmpeg-devel] [PATCH 2/3] doc/filters: Add new scale2ref constants
Update the filters documentation to include the new scale2ref
constants.
Signed-off-by: Kevin Mark
---
doc/filters.texi | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index a0ab2fb0c7..ff41402f7e 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12429,7 +12429,31 @@ Supersampling
Scale (resize) the input video, based on a reference video.
See the scale filter for available options, scale2ref supports the same but
-uses the reference video instead of the main input as basis.
+uses the reference video instead of the main input as basis. scale2ref also
+supports the following additional constants for the @option{w} and
+@option{h} options:
+
+@table @var
+@item other_w
+@item other_h
+The non-reference video's width and height
+
+@item other_a
+The same as @var{other_w} / @var{other_h}
+
+@item other_sar
+The non-reference video's sample aspect ratio
+
+@item other_dar
+The non-reference video's display aspect ratio. Calculated from
+@code{(other_w / other_h) * other_sar}.
+
+@item other_hsub
+@item other_vsub
+The non-reference video's horizontal and vertical chroma subsample values.
+For example for the pixel format "yuv422p" @var{hsub} is 2 and @var{vsub}
+is 1.
+@end table
@subsection Examples
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 3/3] doc/filters: Clarify scale2ref example
Signed-off-by: Kevin Mark --- doc/filters.texi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/filters.texi b/doc/filters.texi index ff41402f7e..948e43deca 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -12459,7 +12459,7 @@ is 1. @itemize @item -Scale a subtitle stream to match the main video in size before overlaying +Scale a subtitle stream (b) to match the main video (a) in size before overlaying @example 'scale2ref[b][a];[a][b]overlay' @end example -- 2.13.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] libavfilter/scale2ref: Add constants for the non-ref input
Thank you, I was not aware there was a ticket already for this issue. I wouldn't be comfortable calling this a bug fix (or that ticket being a bug/defect) considering the current functionality, while not particularly intuitive, is consistent with the documentation. Patch 3 helps clarify the scale2ref documentation a tad but there's still more room to clarify the wording, no doubt. It's not explicit that "as basis" means all the scale variables will be set to that of the reference input. I'll consider submitting another patch for that after this is applied/discussed. On Sat, May 27, 2017 at 10:49 AM, Gyan wrote: > On Sat, May 27, 2017 at 7:40 PM, Kevin Mark wrote: > > > Variables pertaining to the non-reference video are now available when > > using the scale2ref filter. This allows scaling a video with another > > as a reference point while maintaining the original aspect ratio of > > the non-reference video. > > > > Once applied, leave a note at (and close) > https://trac.ffmpeg.org/ticket/5392 > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/3] doc/filters: Add new scale2ref constants
On Sat, May 27, 2017 at 8:30 PM, Michael Niedermayer wrote: > should be merged with the patch that adds these constants > > thx I'll include this with the other updated patch. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] libavfilter/scale2ref: Add constants for the non-ref input
On Sat, May 27, 2017 at 1:36 PM, Gyan wrote: > It's weird, given those uses cases - overlays - to override the operand's > native aspect ratio. I'd say, a design oversight, at least. I agree. I'm not aware of any common workflow that you would have subtitles (transparent PNGs or something) that would be the same AR as the main video but not the same size (as evidence by the fact that scale2ref is needed at all). Oh well. These additional constants/variables should tackle that ticket without needing to introduce overly specific special cases like the suggested keep-AR flag. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] libavfilter/scale2ref: Add constants for the non-ref input
On Sat, May 27, 2017 at 8:29 PM, Michael Niedermayer wrote: > main_* > and > ref_* > > are maybe prefixes that would work without conflicting with the > existing ones I'm partial to main_* since when I think of ref_* that makes me think of the input being used "as basis" (in the words of the documentation) or reference. Any thoughts on creating shorthand variables? I don't see a particular need for them but it would put these new variables in line with the others. mw for main_w, mh for main_h. Potentially ma for main_a. I don't have a strong feeling on it one way or another. This is my first patch set to FFmpeg (huzzah!) so I have a few quick questions before continuing. What is the preferred way to revise a patch? From my understanding git send-email is only useful for the initial patch. Unless revised patches should be new threads? Or should I just reply to this thread with attachments from git format-patch? Although now that I'm looking at it, git send-email has an --in-reply-to option that could potentially do the trick of submitting the revised patch. This mailing list patch submission workflow is new to me so I appreciate any guidance. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/3] libavfilter/scale2ref: Add constants for the non-ref input
On Sun, May 28, 2017 at 12:14 PM, Gyan wrote: > If you do, I suggest 'mdar' for main_dar which is the only one that ought > to be required. > > The scale filter adjusts the SAR of the result so that the DAR is preserved > i.e. when a 100x100 canvas w/ SAR of 2.0 is scaled to 50x100, the result > will have SAR 4.0. In the scale2ref filter, this adjustment is made so that > the ref's DAR is effected onto the scaled result. I don't believe your > patch changes this behaviour. Now, the overlay filter does not respect the > overlay input's SAR - a 100x100 canvas of SAR 2 is overlaid as a square - > so having a shorthand for the main DAR and promoting its use will silently > enforce a visually correct-looking input for overlays and other compositing > uses. Sounds good. If my understanding is correct, you would recommend changing the scale2ref=iw/6:iw/(6*main_a) [main][ref] example to scale2ref=iw/6:iw/(6*mdar) [main][ref] correct? Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
Variables pertaining to the main video are now available when
using the scale2ref filter. This allows, as an example, scaling a
video with another as a reference point while maintaining the
original aspect ratio of the primary/non-reference video.
Consider the following graph: scale2ref=iw/6:-1 [main][ref]
This will scale [main] to 1/6 the width of [ref] while maintaining
the aspect ratio. This works well when the AR of [ref] is equal to
the AR of [main] only. What the above filter really does is
maintain the AR of [ref] when scaling [main]. So in all non-same-AR
situations [main] will appear stretched or compressed to conform to
the same AR of the reference video. Without doing this calculation
externally there is no way to scale in reference to another input
while maintaining AR in libavfilter.
To make this possible, we introduce eight new constants to be used
in the w and h expressions only in the scale2ref filter:
* main_w/main_h: width/height of the main input video
* main_a: aspect ratio of the main input video
* main_sar: sample aspect ratio of the main input video
* main_dar: display aspect ratio of the main input video
* main_hsub/main_vsub: horiz/vert chroma subsample vals of main
* mdar: a shorthand alias of main_dar
Of course, not all of these constants are needed for maintaining the
AR, but adding additional constants in line of what is available for
in/out allows for other scaling possibilities I have not imagined.
So to now scale a video to 1/6 the size of another video using the
width and maintaining its own aspect ratio you can do this:
scale2ref=iw/6:ow/mdar [main][ref]
This is ideal for picture-in-picture configurations where you could
have a square or 4:3 video overlaid on a corner of a larger 16:9
feed all while keeping the scaled video in the corner at its correct
aspect ratio and always the same size relative to the larger video.
I've tried to re-use as much code as possible. I could not find a way
to avoid duplication of the var_names array. It must now be kept in
sync with the other (the normal one and the scale2ref one) for
everything to work which does not seem ideal. For every new variable
introduced/removed into/from the normal scale filter one must be
added/removed to/from the scale2ref version. Suggestions on how to
avoid var_names duplication are welcome.
var_values has been increased to always be large enough for the
additional scale2ref variables. I do not forsee this being a problem
as the names variable will always be the correct size. From my
understanding of av_expr_parse_and_eval it will stop processing
variables when it runs out of names even though there may be
additional (potentially uninitialized) entries in the values array.
The ideal solution here would be using a variable-length array but
that is unsupported in C90.
This patch does not remove any functionality and is strictly a
feature patch. There are no API changes. Behavior does not change for
any previously valid inputs.
The applicable documentation has also been updated.
Signed-off-by: Kevin Mark
---
doc/filters.texi| 26 ++-
libavfilter/scale.c | 72 ++---
2 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 107fe61447..2cea6b74e6 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12429,7 +12429,31 @@ Supersampling
Scale (resize) the input video, based on a reference video.
See the scale filter for available options, scale2ref supports the same but
-uses the reference video instead of the main input as basis.
+uses the reference video instead of the main input as basis. scale2ref also
+supports the following additional constants for the @option{w} and
+@option{h} options:
+
+@table @var
+@item main_w
+@item main_h
+The main input video's width and height
+
+@item main_a
+The same as @var{main_w} / @var{main_h}
+
+@item main_sar
+The main input video's sample aspect ratio
+
+@item main_dar, mdar
+The main input video's display aspect ratio. Calculated from
+@code{(main_w / main_h) * main_sar}.
+
+@item main_hsub
+@item main_vsub
+The main input video's horizontal and vertical chroma subsample values.
+For example for the pixel format "yuv422p" @var{hsub} is 2 and @var{vsub}
+is 1.
+@end table
@subsection Examples
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 50cd442849..552b7cbb04 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -60,6 +60,49 @@ enum var_name {
VARS_NB
};
+/**
+ * This must be kept in sync with var_names so that it is always a
+ * complete list of var_names with the scale2ref specific names
+ * appended. scale2ref values must appear in the order they appear
+ * in the var_name_scale2ref enum but also be below all of the
+ * non-scale2ref specific values.
+ */
+static const char *const var_names_scale2ref[] = {
+"PI",
+"PHI",
+&qu
Re: [FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
I'm hoping this is the proper means of submitting an updated patch. I used git send-email with the --in-reply-to option set to the Message-Id of my original patch. It looks like it created a new patch in Patchwork (instead of updating the old one) and I'm not sure if that's what we want it to do. This workflow is all new to me so I'm open to being told the proper way of doing things. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
On Tue, May 30, 2017 at 10:40 AM, Kevin Mark wrote: > +const AVFilterLink *main; Unfortunately that line results in a warning on GCC (but not LLVM): libavfilter/scale.c: In function ‘ff_scale_eval_dimensions’: libavfilter/scale.c:121:25: warning: ‘main’ is usually a function [-Wmain] const AVFilterLink *main; ^ An updated patch is on the way to address this. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
Variables pertaining to the main video are now available when
using the scale2ref filter. This allows, as an example, scaling a
video with another as a reference point while maintaining the
original aspect ratio of the primary/non-reference video.
Consider the following graph: scale2ref=iw/6:-1 [main][ref]
This will scale [main] to 1/6 the width of [ref] while maintaining
the aspect ratio. This works well when the AR of [ref] is equal to
the AR of [main] only. What the above filter really does is
maintain the AR of [ref] when scaling [main]. So in all non-same-AR
situations [main] will appear stretched or compressed to conform to
the same AR of the reference video. Without doing this calculation
externally there is no way to scale in reference to another input
while maintaining AR in libavfilter.
To make this possible, we introduce eight new constants to be used
in the w and h expressions only in the scale2ref filter:
* main_w/main_h: width/height of the main input video
* main_a: aspect ratio of the main input video
* main_sar: sample aspect ratio of the main input video
* main_dar: display aspect ratio of the main input video
* main_hsub/main_vsub: horiz/vert chroma subsample vals of main
* mdar: a shorthand alias of main_dar
Of course, not all of these constants are needed for maintaining the
AR, but adding additional constants in line of what is available for
in/out allows for other scaling possibilities I have not imagined.
So to now scale a video to 1/6 the size of another video using the
width and maintaining its own aspect ratio you can do this:
scale2ref=iw/6:ow/mdar [main][ref]
This is ideal for picture-in-picture configurations where you could
have a square or 4:3 video overlaid on a corner of a larger 16:9
feed all while keeping the scaled video in the corner at its correct
aspect ratio and always the same size relative to the larger video.
I've tried to re-use as much code as possible. I could not find a way
to avoid duplication of the var_names array. It must now be kept in
sync with the other (the normal one and the scale2ref one) for
everything to work which does not seem ideal. For every new variable
introduced/removed into/from the normal scale filter one must be
added/removed to/from the scale2ref version. Suggestions on how to
avoid var_names duplication are welcome.
var_values has been increased to always be large enough for the
additional scale2ref variables. I do not forsee this being a problem
as the names variable will always be the correct size. From my
understanding of av_expr_parse_and_eval it will stop processing
variables when it runs out of names even though there may be
additional (potentially uninitialized) entries in the values array.
The ideal solution here would be using a variable-length array but
that is unsupported in C90.
This patch does not remove any functionality and is strictly a
feature patch. There are no API changes. Behavior does not change for
any previously valid inputs.
The applicable documentation has also been updated.
Signed-off-by: Kevin Mark
---
doc/filters.texi| 26 ++-
libavfilter/scale.c | 72 ++---
2 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 107fe61447..2cea6b74e6 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12429,7 +12429,31 @@ Supersampling
Scale (resize) the input video, based on a reference video.
See the scale filter for available options, scale2ref supports the same but
-uses the reference video instead of the main input as basis.
+uses the reference video instead of the main input as basis. scale2ref also
+supports the following additional constants for the @option{w} and
+@option{h} options:
+
+@table @var
+@item main_w
+@item main_h
+The main input video's width and height
+
+@item main_a
+The same as @var{main_w} / @var{main_h}
+
+@item main_sar
+The main input video's sample aspect ratio
+
+@item main_dar, mdar
+The main input video's display aspect ratio. Calculated from
+@code{(main_w / main_h) * main_sar}.
+
+@item main_hsub
+@item main_vsub
+The main input video's horizontal and vertical chroma subsample values.
+For example for the pixel format "yuv422p" @var{hsub} is 2 and @var{vsub}
+is 1.
+@end table
@subsection Examples
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 50cd442849..e3a2fb5923 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -60,6 +60,49 @@ enum var_name {
VARS_NB
};
+/**
+ * This must be kept in sync with var_names so that it is always a
+ * complete list of var_names with the scale2ref specific names
+ * appended. scale2ref values must appear in the order they appear
+ * in the var_name_scale2ref enum but also be below all of the
+ * non-scale2ref specific values.
+ */
+static const char *const var_names_scale2ref[] = {
+"PI",
+"PHI",
+&qu
Re: [FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
On Fri, Jun 2, 2017 at 9:16 PM, James Almer wrote: > This broke 820 tests with Valgrind. All of them seem to point to > > ==1253== Invalid read of size 8 > ==1253==at 0x697ECA: ff_scale_eval_dimensions (scale.c:118) scale.c:118: const char scale2ref = outlink->src->inputs[1] == inlink; I believe the issue must be that ff_scale_eval_dimensions blindly assumes a second input exists. A potential fix may be: const char scale2ref = outlink->src->nb_inputs == 2 && outlink->src->inputs[1] == inlink; As long as I am correct in assuming that nb_inputs will equal the number of input links. I'll have to reproduce the valgrind tests myself and see if this fixes it. Thanks James! ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
On Fri, Jun 2, 2017 at 9:37 PM, Kevin Mark wrote: > On Fri, Jun 2, 2017 at 9:16 PM, James Almer wrote: >> This broke 820 tests with Valgrind. Patch has been submitted. Thanks again for the report! ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale2ref: Fix out-of-bounds array access
ff_scale_eval_dimensions blindly assumes that two inputs are always available as of 3385989b98be7940044e4f0a6b431a0a00abf2fa. This is notably not the case when the function is called for the scale filter. With the scale filter inputs[1] does not exist. ff_scale_eval_dimensions now has an updated scale2ref check that makes certain two inputs are actually available before attempting to access the second one. Thanks to James Almer for reporting this bug. This should fix the 820 Valgrind tests I single-handedly managed to break. Signed-off-by: Kevin Mark --- libavfilter/scale.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavfilter/scale.c b/libavfilter/scale.c index e3a2fb5923..03745ddcb8 100644 --- a/libavfilter/scale.c +++ b/libavfilter/scale.c @@ -115,7 +115,7 @@ int ff_scale_eval_dimensions(void *log_ctx, int factor_w, factor_h; int eval_w, eval_h; int ret; -const char scale2ref = outlink->src->inputs[1] == inlink; +const char scale2ref = outlink->src->nb_inputs == 2 && outlink->src->inputs[1] == inlink; double var_values[VARS_NB + VARS_S2R_NB], res; const AVPixFmtDescriptor *main_desc; const AVFilterLink *main_link; -- 2.13.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] FATE: Add test for libavfilter/scale2ref
This new FATE test for the scale2ref filter makes use of the recently added scale2ref-specific variables to maintain the aspect ratio of a test input. Filtergraph explanation: [main] has an AR of 4:3. [ref] has an AR of 16:9. 640 / 4 = 160. So the new width for [main] is 160. 160 / ((320 / 240) * (1 / 1)) = 160 / (4 / 3) = 120. So the new height for [main] is 120. 160 / 120 = 4 / 3 so [main]'s aspect ratio has been maintained while using [ref]'s width as a reference point. [ref] is nullsink'd since it is left unchanged by scale2ref (and so shouldn't need to be tested). If we were to use "iw/4:-1" in place of "iw/4:ow/mdar": 640 / 4 = 160. So the new width for [main] would be 160. 360 / 4 = 90. So the new height for [main] would be 90. 160 / 90 = 16 / 9 so [main] now has the same aspect ratio as [ref] which is probably what you do not want. This is currently the only test for scale2ref. Signed-off-by: Kevin Mark --- tests/fate/filter-video.mak | 4 tests/filtergraphs/scale2ref_keep_aspect| 5 + tests/ref/fate/filter-scale2ref_keep_aspect | 14 ++ 3 files changed, 23 insertions(+) create mode 100644 tests/filtergraphs/scale2ref_keep_aspect create mode 100644 tests/ref/fate/filter-scale2ref_keep_aspect diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak index 1ca29e8d32..53fc7a6528 100644 --- a/tests/fate/filter-video.mak +++ b/tests/fate/filter-video.mak @@ -413,6 +413,10 @@ fate-filter-scale200: CMD = video_filter "scale=w=200:h=200" FATE_FILTER_VSYNTH-$(CONFIG_SCALE_FILTER) += fate-filter-scale500 fate-filter-scale500: CMD = video_filter "scale=w=500:h=500" +FATE_FILTER_VSYNTH-$(CONFIG_SCALE2REF_FILTER) += fate-filter-scale2ref_keep_aspect +fate-filter-scale2ref_keep_aspect: tests/data/filtergraphs/scale2ref_keep_aspect +fate-filter-scale2ref_keep_aspect: CMD = framemd5 -frames:v 5 -filter_complex_script $(TARGET_PATH)/tests/data/filtergraphs/scale2ref_keep_aspect -map "[main]" + FATE_FILTER_VSYNTH-$(CONFIG_SCALE_FILTER) += fate-filter-scalechroma fate-filter-scalechroma: tests/data/vsynth1.yuv fate-filter-scalechroma: CMD = framecrc -flags bitexact -s 352x288 -pix_fmt yuv444p -i tests/data/vsynth1.yuv -pix_fmt yuv420p -sws_flags +bitexact -vf scale=out_v_chr_pos=33:out_h_chr_pos=151 diff --git a/tests/filtergraphs/scale2ref_keep_aspect b/tests/filtergraphs/scale2ref_keep_aspect new file mode 100644 index 00..f407460ec7 --- /dev/null +++ b/tests/filtergraphs/scale2ref_keep_aspect @@ -0,0 +1,5 @@ +sws_flags=+accurate_rnd+bitexact; +testsrc=size=320x240 [main]; +testsrc=size=640x360 [ref]; +[main][ref] scale2ref=iw/4:ow/mdar [main][ref]; +[ref] nullsink diff --git a/tests/ref/fate/filter-scale2ref_keep_aspect b/tests/ref/fate/filter-scale2ref_keep_aspect new file mode 100644 index 00..ca03277446 --- /dev/null +++ b/tests/ref/fate/filter-scale2ref_keep_aspect @@ -0,0 +1,14 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#tb 0: 1/25 +#media_type 0: video +#codec_id 0: rawvideo +#dimensions 0: 160x120 +#sar 0: 4/3 +#stream#, dts,pts, duration, size, hash +0, 0, 0,1,57600, 9a19c23dc3a557786840d0098606d5f1 +0, 1, 1,1,57600, e6fbdabaf1bb0d28afc648ed4d27e9f0 +0, 2, 2,1,57600, 52924ed0a751214c50fb2e7a626c8cc5 +0, 3, 3,1,57600, 67d5fd6ee71793f1cf8794d1c27afdce +0, 4, 4,1,57600, 85f7775f7b01afd369fc8919dc759d30 -- 2.13.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale2ref: Add constants for the primary input
On Thu, Jun 1, 2017 at 5:37 PM, Michael Niedermayer wrote: > applied > > can you add a fate test that uses the new identifers ? > > Thanks Certainly. Submitted a patch for a scale2ref-specific test. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] FATE: Add test for libavfilter/scale2ref
On Sun, Jun 4, 2017 at 4:19 AM, Gyan wrote: > Now that this feature has been applied, it may be helpful to check if one > of the W/H arguments is of the form '-n' and log a warning that this will > force the ref's display aspect ratio. The warning should also convey the > expression to use: 'oh*mdar' or 'ow/mdar' , as the case may be. > > Additionally, since the output SAR will be adjusted to force the ref's DAR, > it would be helpful to add a warning for that (tell users to append > setsar=1 after the scale) --or-- (ideally) patch the filter so that the > main's DAR is preserved. I was thinking about this as well. The next step for improving the scale2ref filter is probably patching it to preserve main's DAR, as you mentioned. It's definitely easy enough to correct with the setsar filter but given scale2ref's purpose the current behavior still feels dramatically unintuitive. Perhaps I'm just being unimaginative when it comes to use-cases but it certainly seems like the vast majority of uses would involve keeping main's DAR. This would be a breaking change, unlike the previous patches. The warning for the "-n" stuff sounds helpful. I'll take a look at this. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
This change makes it more clear when using the scale and scale2ref
filters what is actually happening. The old format did not
differentiate between scale and scale2ref which would make it seem
that, when using scale2ref, the ref was what was truly being scaled.
Old format for both scale and scale2ref:
w:640 h:360 fmt:rgb24 sar:1/1 -> w:160 h:120 fmt:rgb24 sar:4/3 flags:0x2
The left side is the input and the right side is the output. While
this is sufficiently clear for scale, for scale2ref it appears to
conflate the main input with the reference input. To be fair that is
exactly what the code is doing (and on purpose) but that's not a very
intuitive implementation detail to expose to the user. Now that the
main input's constants are exposed in scale2ref it makes even more
sense to correct this.
New format for scale:
in w:320 h:240 fmt:rgb24 sar:1/1
out w:80 h:60 fmt:rgb24 sar:1/1 flags:0xc
New format for scale2ref:
in w:320 h:240 fmt:rgb24 sar:1/1
ref w:640 h:360 fmt:rgb24 sar:1/1
out w:160 h:120 fmt:rgb24 sar:4/3 flags:0x2
The increase in clarity is self-evident.
Signed-off-by: Kevin Mark
---
libavfilter/vf_scale.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index c59ac6b0ea..9232bc4439 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -342,9 +342,18 @@ static int config_props(AVFilterLink *outlink)
} else
outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
-av_log(ctx, AV_LOG_VERBOSE, "w:%d h:%d fmt:%s sar:%d/%d -> w:%d h:%d
fmt:%s sar:%d/%d flags:0x%0x\n",
- inlink ->w, inlink ->h, av_get_pix_fmt_name( inlink->format),
- inlink->sample_aspect_ratio.num, inlink->sample_aspect_ratio.den,
+if (ctx->filter == &ff_vf_scale2ref) {
+av_log(ctx, AV_LOG_VERBOSE, "in w:%d h:%d fmt:%s sar:%d/%d\n",
+ inlink0->w, inlink0->h, av_get_pix_fmt_name(inlink0->format),
+ inlink0->sample_aspect_ratio.num,
inlink0->sample_aspect_ratio.den);
+}
+
+av_log(ctx, AV_LOG_VERBOSE, "%s w:%d h:%d fmt:%s sar:%d/%d\n",
+ ctx->filter == &ff_vf_scale2ref ? "ref" : "in ",
+ inlink->w, inlink->h, av_get_pix_fmt_name(inlink->format),
+ inlink->sample_aspect_ratio.num, inlink->sample_aspect_ratio.den);
+
+av_log(ctx, AV_LOG_VERBOSE, "out w:%d h:%d fmt:%s sar:%d/%d flags:0x%0x\n",
outlink->w, outlink->h, av_get_pix_fmt_name(outlink->format),
outlink->sample_aspect_ratio.num, outlink->sample_aspect_ratio.den,
scale->flags);
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 2/2] libavfilter/scale2ref: Maintain main input's DAR
The scale2ref filter will now maintain the DAR of the main input and
not the DAR of the reference input. This previous behavior was deemed
counterintuitive for most (all?) use-cases.
Before:
scale2ref=iw/4:ow/mdar
in w:320 h:240 fmt:rgb24 sar:1/1
ref w:640 h:360 fmt:rgb24 sar:1/1
out w:160 h:120 fmt:rgb24 sar:4/3 flags:0x2
SAR: ((120 * 640) / (160 * 360)) * (1 / 1) = 4 / 3
DAR: (160 / 120) * (4 / 3) = 16 / 9
(main out now same DAR as ref)
Now:
scale2ref=iw/4:ow/mdar
in w:320 h:240 fmt:rgb24 sar:1/1
ref w:640 h:360 fmt:rgb24 sar:1/1
out w:160 h:120 fmt:rgb24 sar:1/1 flags:0x2
SAR: ((120 * 320) / (160 * 240)) * (1 / 1) = 1 / 1
DAR: (160 / 120) * (1 / 1) = 4 / 3
(main out same DAR as main in)
The scale2ref FATE test has also been updated.
Signed-off-by: Kevin Mark
---
libavfilter/vf_scale.c | 6 +++---
tests/ref/fate/filter-scale2ref_keep_aspect | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 9232bc4439..5e55f9344b 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -337,10 +337,10 @@ static int config_props(AVFilterLink *outlink)
}
}
-if (inlink->sample_aspect_ratio.num){
-outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
inlink->w, outlink->w * inlink->h}, inlink->sample_aspect_ratio);
+if (inlink0->sample_aspect_ratio.num){
+outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
inlink0->w, outlink->w * inlink0->h}, inlink0->sample_aspect_ratio);
} else
-outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
+outlink->sample_aspect_ratio = inlink0->sample_aspect_ratio;
if (ctx->filter == &ff_vf_scale2ref) {
av_log(ctx, AV_LOG_VERBOSE, "in w:%d h:%d fmt:%s sar:%d/%d\n",
diff --git a/tests/ref/fate/filter-scale2ref_keep_aspect
b/tests/ref/fate/filter-scale2ref_keep_aspect
index ca03277446..8dd0dbb13b 100644
--- a/tests/ref/fate/filter-scale2ref_keep_aspect
+++ b/tests/ref/fate/filter-scale2ref_keep_aspect
@@ -5,7 +5,7 @@
#media_type 0: video
#codec_id 0: rawvideo
#dimensions 0: 160x120
-#sar 0: 4/3
+#sar 0: 1/1
#stream#, dts,pts, duration, size, hash
0, 0, 0,1,57600, 9a19c23dc3a557786840d0098606d5f1
0, 1, 1,1,57600, e6fbdabaf1bb0d28afc648ed4d27e9f0
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavutil/eval: Add round function to expression parser
We have floor, ceil, and trunc. Let's add round.
Signed-off-by: Kevin Mark
---
doc/utils.texi | 3 +++
libavutil/eval.c | 5 -
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/doc/utils.texi b/doc/utils.texi
index 1734439459..d65bdf0b0e 100644
--- a/doc/utils.texi
+++ b/doc/utils.texi
@@ -914,6 +914,9 @@ various input values that the expression can access through
@code{ld(0)}. When the expression evaluates to 0 then the
corresponding input value will be returned.
+@item round(expr)
+Round the value of expression @var{expr} to the nearest integer. For example,
"round(1.5)" is "2.0".
+
@item sin(x)
Compute sine of @var{x}.
diff --git a/libavutil/eval.c b/libavutil/eval.c
index 7e866155db..638259adef 100644
--- a/libavutil/eval.c
+++ b/libavutil/eval.c
@@ -153,7 +153,7 @@ struct AVExpr {
e_squish, e_gauss, e_ld, e_isnan, e_isinf,
e_mod, e_max, e_min, e_eq, e_gt, e_gte, e_lte, e_lt,
e_pow, e_mul, e_div, e_add,
-e_last, e_st, e_while, e_taylor, e_root, e_floor, e_ceil, e_trunc,
+e_last, e_st, e_while, e_taylor, e_root, e_floor, e_ceil, e_trunc,
e_round,
e_sqrt, e_not, e_random, e_hypot, e_gcd,
e_if, e_ifnot, e_print, e_bitand, e_bitor, e_between, e_clip, e_atan2
} type;
@@ -189,6 +189,7 @@ static double eval_expr(Parser *p, AVExpr *e)
case e_floor: return e->value * floor(eval_expr(p, e->param[0]));
case e_ceil : return e->value * ceil (eval_expr(p, e->param[0]));
case e_trunc: return e->value * trunc(eval_expr(p, e->param[0]));
+case e_round: return e->value * round(eval_expr(p, e->param[0]));
case e_sqrt: return e->value * sqrt (eval_expr(p, e->param[0]));
case e_not:return e->value * (eval_expr(p, e->param[0]) == 0);
case e_if: return e->value * (eval_expr(p, e->param[0]) ?
eval_expr(p, e->param[1]) :
@@ -440,6 +441,7 @@ static int parse_primary(AVExpr **e, Parser *p)
else if (strmatch(next, "floor" )) d->type = e_floor;
else if (strmatch(next, "ceil" )) d->type = e_ceil;
else if (strmatch(next, "trunc" )) d->type = e_trunc;
+else if (strmatch(next, "round" )) d->type = e_round;
else if (strmatch(next, "sqrt" )) d->type = e_sqrt;
else if (strmatch(next, "not" )) d->type = e_not;
else if (strmatch(next, "pow" )) d->type = e_pow;
@@ -637,6 +639,7 @@ static int verify_expr(AVExpr *e)
case e_floor:
case e_ceil:
case e_trunc:
+case e_round:
case e_sqrt:
case e_not:
case e_random:
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] doc/filters: Correct scale doc regarding w/h <= 0
According to libavfilter/scale.c, if the width and height are both less than or equal to 0 then the input size is used for both dimensions. It does not need to be -1. -1:-1 is the same as 0:0 which is the same as -10:-42, etc. if (w < 0 && h < 0) eval_w = eval_h = 0; Signed-off-by: Kevin Mark --- doc/filters.texi | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index 65eef89d07..b9d6eafc74 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -12125,12 +12125,13 @@ the complete list of scaler options. Set the output video dimension expression. Default value is the input dimension. -If the value is 0, the input width is used for the output. +If the width value is 0, the input width is used for the output. If the +height value is 0, the input height is used for the output. If one of the values is -1, the scale filter will use a value that maintains the aspect ratio of the input image, calculated from the -other specified dimension. If both of them are -1, the input size is -used +other specified dimension. If both of them are negative, the behavior +will be identical to both values being set to 0 as explained above. If one of the values is -n with n > 1, the scale filter will also use a value that maintains the aspect ratio of the input image, calculated from the other -- 2.13.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
On Tue, Jun 6, 2017 at 11:49 AM, Michael Niedermayer wrote: > yes but its much harder to grep for as its not a single line anymore I agree that it's not going to be as pretty a regular expression to grep through, as there is 33% more data, but it should still be doable without too much effort. How important is it that we maintain "API" compatibility on verbose CLI output? ffmpeg [...] scale2ref=0:0 [...] -v verbose - 2>&1 >/dev/null | grep -oP 'regex' Where regex is: (in|out|ref) +w:(\d+) h:(\d+) fmt:(\w+) sar:(\d+)\/(\d+)(?: flags:0x[[:xdigit:]]+)? Assuming GNU grep 2.25+, you'll get: in w:320 h:240 fmt:rgb24 sar:1/1 ref w:640 h:360 fmt:rgb24 sar:1/1 out w:640 h:360 fmt:rgb24 sar:3/4 flags:0x2 It also works with BSD grep 2.5.1-FreeBSD included in macOS if you use the -E option instead of -P. These would be considered three separate matches so if you're using a good regex engine it'd be pretty easy to loop over each match, check the first group to determine if it's in, ref, or out and act accordingly on the rest of the captured data. You could also, if you wanted, assume that the first line is in and the second line is out if you only have two matches (or lines even) and if you have three matches/lines the first is in, second is ref, third is out. If you needed it to work with less sophisticated engines it shouldn't be too hard to dumb down the regex above. Live-ish example: https://regex101.com/r/wvHLpa/1 Is there a special property that makes single lines much easier to grep? Something specific to bash? I wouldn't think bash would have any problems looping over this by line. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] doc/filters: Correct scale doc regarding w/h <= 0
Hey Moritz, Thanks for the feedback. On Tue, Jun 6, 2017 at 7:59 AM, Moritz Barsnick wrote: > This makes it obvious that the paragraph following the one you fixed is > a bit misleading (to me): > > If one of the values is -n with n > 1, the scale filter will also > use a value that maintains the aspect ratio of the input image, > calculated from the other specified dimension. After that it will, > however, make sure that the calculated dimension is divisible by n > and adjust the value if necessary. > > This is true only if *exactly* one of the two values is "-n with n > 1". > (It also doesn't apply to "-1:-2". Good luck finding words to describe > this behavior. ;-)) Haha, you're right. The best approach seems to be involve removing the paragraph about the -1 stuff (since it's technically -n) and expand the -n paragraph to include -1. So something like: If the width value is 0, the input width is used for the output. If the height value is 0, the input height is used for the output. If one and only one of the values is -n with n >= 1, the scale filter will use a value that maintains the aspect ratio of the input image, calculated from the other specified dimension. After that it will, however, make sure that the calculated dimension is divisible by n and adjust the value if necessary. If both values are -n with n >= 1, the behavior will be identical to both values being set to 0 as previously detailed. - I noticed the z-scale documentation is very similar. I might need to look at that too. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
The input width and height is known at parse time so there's no
reason ow/oh should not be usable when using 0 as the width or
height expression.
Previously in "scale=0:ow" ow would be set to "0" which works,
conveniently, as "scale=0:0" is perfectly valid input but this breaks
down when you do something like "scale=0:ow/4" which one could
reasonably expect to work as well, but does not as ow is 0 not the
real value.
This change handles the 0 case for w/h immediately so the ow/oh
variables work as expected. Consequently, the rest of the code does
not need to handle 0 input. w/h will always be > 0 or < 0.
Signed-off-by: Kevin Mark
---
libavfilter/scale.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 03745ddcb8..cc2bea5caf 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
-eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res == 0.0 ?
inlink->w : res;
if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
+eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res == 0.0 ?
inlink->h : res;
/* evaluate again the width, as it may depend on the output height */
if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_w = res;
+eval_w = res == 0.0 ? inlink->w : res;
w = eval_w;
h = eval_h;
@@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
factor_h = -h;
}
-if (w < 0 && h < 0)
-eval_w = eval_h = 0;
-
-if (!(w = eval_w))
+if (w < 0 && h < 0) {
w = inlink->w;
-if (!(h = eval_h))
h = inlink->h;
+}
/* Make sure that the result is divisible by the factor we determined
* earlier. If no factor was set, it is nothing will happen as the default
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
Unfortunately I have to withdraw this patch. I'll be submitting an updated one shortly. The below lines allow through values that are 0 when casted/truncated to an integer but are not zero as doubles (like -0.1). On Wed, Jun 7, 2017 at 2:11 AM, Kevin Mark wrote: > -eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; > +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res == 0.0 ? > inlink->w : res; > > if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr), >names, var_values, >NULL, NULL, NULL, NULL, NULL, 0, > log_ctx)) < 0) > goto fail; > -eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res; > +eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res == 0.0 ? > inlink->h : res; > /* evaluate again the width, as it may depend on the output height */ > if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr), >names, var_values, >NULL, NULL, NULL, NULL, NULL, 0, > log_ctx)) < 0) > goto fail; > -eval_w = res; > +eval_w = res == 0.0 ? inlink->w : res; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
The input width and height is known at parse time so there's no
reason ow/oh should not be usable when using 0 as the width or
height expression.
Previously in "scale=0:ow" ow would be set to "0" which works,
conveniently, as "scale=0:0" is perfectly valid input but this breaks
down when you do something like "scale=0:ow/4" which one could
reasonably expect to work as well, but does not as ow is 0 not the
real value.
This change handles the 0 case for w/h immediately so the ow/oh
variables work as expected. Consequently, the rest of the code does
not need to handle 0 input. w/h will always be > 0 or < 0.
Signed-off-by: Kevin Mark
---
libavfilter/scale.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 03745ddcb8..a6c32e3978 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
-eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ?
inlink->w : res;
if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
+eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ?
inlink->h : res;
/* evaluate again the width, as it may depend on the output height */
if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_w = res;
+eval_w = (int) res == 0 ? inlink->w : res;
w = eval_w;
h = eval_h;
@@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
factor_h = -h;
}
-if (w < 0 && h < 0)
-eval_w = eval_h = 0;
-
-if (!(w = eval_w))
+if (w < 0 && h < 0) {
w = inlink->w;
-if (!(h = eval_h))
h = inlink->h;
+}
/* Make sure that the result is divisible by the factor we determined
* earlier. If no factor was set, it is nothing will happen as the default
--
2.13.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
I also have to wonder if it would be advantageous to add the cast on the right side as well. That way the var_values variables will have the proper truncated values on future evaluations. Open to comments on that. On Wed, Jun 7, 2017 at 3:45 AM, Kevin Mark wrote: > -eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; > +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? > inlink->w : res; to perhaps: +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res; Without that extra cast I assume the values in eval_w and var_values[VAR_OUT_W], var_values[VAR_OW] could be different. I doubt most users expect that those values could ever be non-integers which has implications for how you're writing your expression. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
Hi Michael, Hoping to get your thoughts on the grepability issue (wrt my previous email). If it needs to be on a single line there's no reason the new format cannot be changed to do so (removing the \n and adding a separator, really). However I'm a big fan of it as-is (for both scale and scale2ref) and I can't find a case of my own where the regex I proposed would be troublesome. Thanks, Kevin On Tue, Jun 6, 2017 at 4:17 PM Kevin Mark wrote: > On Tue, Jun 6, 2017 at 11:49 AM, Michael Niedermayer > wrote: > > yes but its much harder to grep for as its not a single line anymore > > I agree that it's not going to be as pretty a regular expression to > grep through, as there is 33% more data, but it should still be doable > without too much effort. How important is it that we maintain "API" > compatibility on verbose CLI output? > > ffmpeg [...] scale2ref=0:0 [...] -v verbose - 2>&1 >/dev/null | grep -oP > 'regex' > > Where regex is: > > (in|out|ref) +w:(\d+) h:(\d+) fmt:(\w+) sar:(\d+)\/(\d+)(?: > flags:0x[[:xdigit:]]+)? > > Assuming GNU grep 2.25+, you'll get: > > in w:320 h:240 fmt:rgb24 sar:1/1 > ref w:640 h:360 fmt:rgb24 sar:1/1 > out w:640 h:360 fmt:rgb24 sar:3/4 flags:0x2 > > It also works with BSD grep 2.5.1-FreeBSD included in macOS if you use > the -E option instead of -P. These would be considered three separate > matches so if you're using a good regex engine it'd be pretty easy to > loop over each match, check the first group to determine if it's in, > ref, or out and act accordingly on the rest of the captured data. You > could also, if you wanted, assume that the first line is in and the > second line is out if you only have two matches (or lines even) and if > you have three matches/lines the first is in, second is ref, third is > out. If you needed it to work with less sophisticated engines it > shouldn't be too hard to dumb down the regex above. > > Live-ish example: https://regex101.com/r/wvHLpa/1 > > Is there a special property that makes single lines much easier to > grep? Something specific to bash? I wouldn't think bash would have any > problems looping over this by line. > > Thanks, > Kevin > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] libavfilter/scale2ref: Maintain main input's DAR
I've been using this patch for the past week now and I believe it's
good to go. Does someone want to take a second look before merging?
On Mon, Jun 5, 2017 at 6:55 AM, Kevin Mark wrote:
> The scale2ref filter will now maintain the DAR of the main input and
> not the DAR of the reference input. This previous behavior was deemed
> counterintuitive for most (all?) use-cases.
>
> Before:
> scale2ref=iw/4:ow/mdar
> in w:320 h:240 fmt:rgb24 sar:1/1
> ref w:640 h:360 fmt:rgb24 sar:1/1
> out w:160 h:120 fmt:rgb24 sar:4/3 flags:0x2
> SAR: ((120 * 640) / (160 * 360)) * (1 / 1) = 4 / 3
> DAR: (160 / 120) * (4 / 3) = 16 / 9
> (main out now same DAR as ref)
>
> Now:
> scale2ref=iw/4:ow/mdar
> in w:320 h:240 fmt:rgb24 sar:1/1
> ref w:640 h:360 fmt:rgb24 sar:1/1
> out w:160 h:120 fmt:rgb24 sar:1/1 flags:0x2
> SAR: ((120 * 320) / (160 * 240)) * (1 / 1) = 1 / 1
> DAR: (160 / 120) * (1 / 1) = 4 / 3
> (main out same DAR as main in)
>
> The scale2ref FATE test has also been updated.
>
> Signed-off-by: Kevin Mark
> ---
> libavfilter/vf_scale.c | 6 +++---
> tests/ref/fate/filter-scale2ref_keep_aspect | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 9232bc4439..5e55f9344b 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -337,10 +337,10 @@ static int config_props(AVFilterLink *outlink)
> }
> }
>
> -if (inlink->sample_aspect_ratio.num){
> -outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
> inlink->w, outlink->w * inlink->h}, inlink->sample_aspect_ratio);
> +if (inlink0->sample_aspect_ratio.num){
> +outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
> inlink0->w, outlink->w * inlink0->h}, inlink0->sample_aspect_ratio);
> } else
> -outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +outlink->sample_aspect_ratio = inlink0->sample_aspect_ratio;
>
> if (ctx->filter == &ff_vf_scale2ref) {
> av_log(ctx, AV_LOG_VERBOSE, "in w:%d h:%d fmt:%s sar:%d/%d\n",
> diff --git a/tests/ref/fate/filter-scale2ref_keep_aspect
> b/tests/ref/fate/filter-scale2ref_keep_aspect
> index ca03277446..8dd0dbb13b 100644
> --- a/tests/ref/fate/filter-scale2ref_keep_aspect
> +++ b/tests/ref/fate/filter-scale2ref_keep_aspect
> @@ -5,7 +5,7 @@
> #media_type 0: video
> #codec_id 0: rawvideo
> #dimensions 0: 160x120
> -#sar 0: 4/3
> +#sar 0: 1/1
> #stream#, dts,pts, duration, size, hash
> 0, 0, 0,1,57600,
> 9a19c23dc3a557786840d0098606d5f1
> 0, 1, 1,1,57600,
> e6fbdabaf1bb0d28afc648ed4d27e9f0
> --
> 2.13.0
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
I screwed up my git-send-email. Please ignore this patch as I already
submitted what should be an identical one on June 7th. My apologies.
On Mon, Jun 12, 2017 at 1:51 AM, Kevin Mark wrote:
> The input width and height is known at parse time so there's no
> reason ow/oh should not be usable when using 0 as the width or
> height expression.
>
> Previously in "scale=0:ow" ow would be set to "0" which works,
> conveniently, as "scale=0:0" is perfectly valid input but this breaks
> down when you do something like "scale=0:ow/4" which one could
> reasonably expect to work as well, but does not as ow is 0 not the
> real value.
>
> This change handles the 0 case for w/h immediately so the ow/oh
> variables work as expected. Consequently, the rest of the code does
> not need to handle 0 input. w/h will always be > 0 or < 0.
>
> Signed-off-by: Kevin Mark
> ---
> libavfilter/scale.c | 13 +
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/libavfilter/scale.c b/libavfilter/scale.c
> index 03745ddcb8..a6c32e3978 100644
> --- a/libavfilter/scale.c
> +++ b/libavfilter/scale.c
> @@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
> av_expr_parse_and_eval(&res, (expr = w_expr),
> names, var_values,
> NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> -eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
> +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ?
> inlink->w : res;
>
> if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
>names, var_values,
>NULL, NULL, NULL, NULL, NULL, 0,
> log_ctx)) < 0)
> goto fail;
> -eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
> +eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ?
> inlink->h : res;
> /* evaluate again the width, as it may depend on the output height */
> if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
>names, var_values,
>NULL, NULL, NULL, NULL, NULL, 0,
> log_ctx)) < 0)
> goto fail;
> -eval_w = res;
> +eval_w = (int) res == 0 ? inlink->w : res;
>
> w = eval_w;
> h = eval_h;
> @@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
> factor_h = -h;
> }
>
> -if (w < 0 && h < 0)
> -eval_w = eval_h = 0;
> -
> -if (!(w = eval_w))
> +if (w < 0 && h < 0) {
> w = inlink->w;
> -if (!(h = eval_h))
> h = inlink->h;
> +}
>
> /* Make sure that the result is divisible by the factor we determined
> * earlier. If no factor was set, it is nothing will happen as the
> default
> --
> 2.13.1
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] doc/filters: Correct scale doc regarding w/h <= 0
According to libavfilter/scale.c, if the width and height are both
less than or equal to 0 then the input size is used for both
dimensions. It does not need to be -1. -1:-1 is the same as 0:0 which
is the same as -10:-42, etc.
if (w < 0 && h < 0)
eval_w = eval_h = 0;
The documentation for the zscale filter has also been updated since the
behavior is identical.
Signed-off-by: Kevin Mark
---
doc/filters.texi | 39 ---
1 file changed, 20 insertions(+), 19 deletions(-)
diff --git a/doc/filters.texi b/doc/filters.texi
index 65eef89d07..6f2cc89b1f 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12125,17 +12125,18 @@ the complete list of scaler options.
Set the output video dimension expression. Default value is the input
dimension.
-If the value is 0, the input width is used for the output.
+If the @var{width} or @var{w} value is 0, the input width is used for
+the output. If the @var{height} or @var{h} value is 0, the input height
+is used for the output.
-If one of the values is -1, the scale filter will use a value that
-maintains the aspect ratio of the input image, calculated from the
-other specified dimension. If both of them are -1, the input size is
-used
+If one and only one of the values is -n with n >= 1, the scale filter
+will use a value that maintains the aspect ratio of the input image,
+calculated from the other specified dimension. After that it will,
+however, make sure that the calculated dimension is divisible by n and
+adjust the value if necessary.
-If one of the values is -n with n > 1, the scale filter will also use a value
-that maintains the aspect ratio of the input image, calculated from the other
-specified dimension. After that it will, however, make sure that the calculated
-dimension is divisible by n and adjust the value if necessary.
+If both values are -n with n >= 1, the behavior will be identical to
+both values being set to 0 as previously detailed.
See below for the list of accepted constants for use in the dimension
expression.
@@ -15268,18 +15269,18 @@ The filter accepts the following options.
Set the output video dimension expression. Default value is the input
dimension.
-If the @var{width} or @var{w} is 0, the input width is used for the output.
-If the @var{height} or @var{h} is 0, the input height is used for the output.
+If the @var{width} or @var{w} value is 0, the input width is used for
+the output. If the @var{height} or @var{h} value is 0, the input height
+is used for the output.
-If one of the values is -1, the zscale filter will use a value that
-maintains the aspect ratio of the input image, calculated from the
-other specified dimension. If both of them are -1, the input size is
-used
+If one and only one of the values is -n with n >= 1, the zscale filter
+will use a value that maintains the aspect ratio of the input image,
+calculated from the other specified dimension. After that it will,
+however, make sure that the calculated dimension is divisible by n and
+adjust the value if necessary.
-If one of the values is -n with n > 1, the zscale filter will also use a value
-that maintains the aspect ratio of the input image, calculated from the other
-specified dimension. After that it will, however, make sure that the calculated
-dimension is divisible by n and adjust the value if necessary.
+If both values are -n with n >= 1, the behavior will be identical to
+both values being set to 0 as previously detailed.
See below for the list of accepted constants for use in the dimension
expression.
--
2.13.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
The input width and height is known at parse time so there's no
reason ow/oh should not be usable when using 0 as the width or
height expression.
Previously in "scale=0:ow" ow would be set to "0" which works,
conveniently, as "scale=0:0" is perfectly valid input but this breaks
down when you do something like "scale=0:ow/4" which one could
reasonably expect to work as well, but does not as ow is 0 not the
real value.
This change handles the 0 case for w/h immediately so the ow/oh
variables work as expected. Consequently, the rest of the code does
not need to handle 0 input. w/h will always be > 0 or < 0.
Signed-off-by: Kevin Mark
---
libavfilter/scale.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 03745ddcb8..a6c32e3978 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
-eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ?
inlink->w : res;
if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
+eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ?
inlink->h : res;
/* evaluate again the width, as it may depend on the output height */
if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_w = res;
+eval_w = (int) res == 0 ? inlink->w : res;
w = eval_w;
h = eval_h;
@@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
factor_h = -h;
}
-if (w < 0 && h < 0)
-eval_w = eval_h = 0;
-
-if (!(w = eval_w))
+if (w < 0 && h < 0) {
w = inlink->w;
-if (!(h = eval_h))
h = inlink->h;
+}
/* Make sure that the result is divisible by the factor we determined
* earlier. If no factor was set, it is nothing will happen as the default
--
2.13.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
Still interested in thoughts on this patch/discussion. Thanks, Kevin On Wed, Jun 7, 2017 at 3:54 AM, Kevin Mark wrote: > I also have to wonder if it would be advantageous to add the cast on > the right side as well. That way the var_values variables will have > the proper truncated values on future evaluations. Open to comments on > that. > > On Wed, Jun 7, 2017 at 3:45 AM, Kevin Mark wrote: >> -eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res; >> +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? >> inlink->w : res; > > to perhaps: > +eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res > == 0 ? inlink->w : (int) res; > > Without that extra cast I assume the values in eval_w and > var_values[VAR_OUT_W], var_values[VAR_OW] could be different. I doubt > most users expect that those values could ever be non-integers which > has implications for how you're writing your expression. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
On Mon, Jun 12, 2017 at 9:42 PM, Michael Niedermayer wrote: > why is there a cast at all ? The cast is there because if you run this: ffmpeg -frames:v 5 -filter_complex "sws_flags=+accurate_rnd+bitexact;testsrc=size=320x240 [main];testsrc=size=640x360 [ref];[main][ref] scale2ref=0:print(ow/641) [main][ref];[ref] nullsink" -map "[main]" -flags +bitexact -fflags +bitexact -f md5 - This works just fine without a cast for ow. 0 == 0 is true so we set it to 640. But for oh, the print() shows that ow/641 is 0.998440. When it is truncated from a double to an integer (eval_h = res) it becomes 0. But in our comparison, 0.998440 == 0 is false so in this case eval_h will be truncated to 0 which is exactly the behavior we're trying to correct. Adding that cast resolves the issue. 0.998440 == 0 is false but (int) 0.998440 == 0 is true. For the extra cast I was talking about consider this: ffmpeg -frames:v 5 -filter_complex "sws_flags=+accurate_rnd+bitexact;testsrc=size=320x240 [main];testsrc=size=640x360 [ref];[main][ref] scale2ref=500/6:print(print(ow)*5) [main][ref];[ref] nullsink" -map "[main]" -flags +bitexact -fflags +bitexact -f md5 - That will print() 83.33 and then 416.67. A user might (reasonably, in my opinion) expect that the ow value (or oh) is always an integer. With the extra cast you'll see 83.00 and 415.00 printed. 83.33 truncates to 83 so no (noticeable) change for ow but 416.67 does not truncate to 415 so this is an example of a place where the lack of truncation for ow/oh does change the outcome. I hope this clears it up. Perhaps that code should just be entirely refactored to be a little more clear? Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
On Tue, Jun 13, 2017 at 10:04 PM, Michael Niedermayer wrote: > ok, makes sense > i agree the 2nd cast seems a good idea Great, thanks Michael. I'll submit an updated patch with the additional cast. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] libavfilter/scale: Populate ow/oh when using 0 as w/h
The input width and height is known at parse time so there's no
reason ow/oh should not be usable when using 0 as the width or
height expression.
Previously in "scale=0:ow" ow would be set to "0" which works,
conveniently, as "scale=0:0" is perfectly valid input but this breaks
down when you do something like "scale=0:ow/4" which one could
reasonably expect to work as well, but does not as ow is 0 not the
real value.
This change handles the 0 case for w/h immediately so the ow/oh
variables work as expected. Consequently, the rest of the code does
not need to handle 0 input. w/h will always be > 0 or < 0.
The second explicit (int) cast ensures that ow/oh appear as integers
as a user might expect when dealing with pixel dimensions.
Signed-off-by: Kevin Mark
---
libavfilter/scale.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/libavfilter/scale.c b/libavfilter/scale.c
index 03745ddcb8..eaee95fac6 100644
--- a/libavfilter/scale.c
+++ b/libavfilter/scale.c
@@ -158,19 +158,19 @@ int ff_scale_eval_dimensions(void *log_ctx,
av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
-eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
+eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ?
inlink->w : (int) res;
if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
+eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ?
inlink->h : (int) res;
/* evaluate again the width, as it may depend on the output height */
if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
names, var_values,
NULL, NULL, NULL, NULL, NULL, 0,
log_ctx)) < 0)
goto fail;
-eval_w = res;
+eval_w = (int) res == 0 ? inlink->w : (int) res;
w = eval_w;
h = eval_h;
@@ -186,13 +186,10 @@ int ff_scale_eval_dimensions(void *log_ctx,
factor_h = -h;
}
-if (w < 0 && h < 0)
-eval_w = eval_h = 0;
-
-if (!(w = eval_w))
+if (w < 0 && h < 0) {
w = inlink->w;
-if (!(h = eval_h))
h = inlink->h;
+}
/* Make sure that the result is divisible by the factor we determined
* earlier. If no factor was set, it is nothing will happen as the default
--
2.13.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] Requesting push access to FFmpeg repo
Ronald Bultje via IRC recommended I ask on the mailing list for push access to the FFmpeg repository so that I may push my own patches once approved. If someone with the access to do so could please consider this request it would be greatly appreciated. Thank you, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Requesting push access to FFmpeg repo
On Tue, Jun 20, 2017 at 7:14 AM, Michael Niedermayer wrote: > If you want push access, please send a patch that adds you to the > MAINTAINERs file > after the patch has been applied, you get push access > > (by going throgh this extra step changes to the git access list are > public, are logged and allow people to comment/veto ...) > > thx Thank you for the clarification and rationale on the process! On Tue, Jun 20, 2017 at 7:36 AM, Hendrik Leppkes wrote: > Personally I think people that have been around like a month should > perhaps excercise some more patience while they get more familiar with > our codebase and our workflows. :) I do not believe patience has been an issue but I can understand how this request could appear premature. I would personally require more than a month of activity before allowing push access to one of my own projects, but it was suggested that I ask by another project member with push access so I assumed things might be different here. It doesn't appear that is the case so it may be best to withdraw my request. I'm open to any further suggestions. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
Hey Ronald, Was this able to solve the issue for you? On Mon, Jul 24, 2017 at 6:27 AM, Kevin Mark wrote: > Hi Ronald, > > On Wed, Jul 19, 2017 at 3:44 AM, Ronald S. Bultje wrote: >> But my grep (Mac) has no -P option... I'd encourage you to keep things >> simple and use a non-\n delimiter between the lines so non-GNU grep can >> continue to be used for this also... > > Does this work for you: > > On Tue, Jun 6, 2017 at 1:17 PM, Kevin Mark wrote: >> It also works with BSD grep 2.5.1-FreeBSD included in macOS if you use >> the -E option instead of -P. > > Thanks, > Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
Hi Ronald, On Sat, Sep 23, 2017 at 11:54 AM, Ronald S. Bultje wrote: > > Yes, -E works on Mac. Thanks! Are there any remaining blockers for a potential merge? Best regards, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 2/2] libavfilter/scale2ref: Maintain main input's DAR
Ping :)
On Mon, Jun 12, 2017 at 1:08 AM, Kevin Mark wrote:
> I've been using this patch for the past week now and I believe it's
> good to go. Does someone want to take a second look before merging?
>
> On Mon, Jun 5, 2017 at 6:55 AM, Kevin Mark wrote:
>> The scale2ref filter will now maintain the DAR of the main input and
>> not the DAR of the reference input. This previous behavior was deemed
>> counterintuitive for most (all?) use-cases.
>>
>> Before:
>> scale2ref=iw/4:ow/mdar
>> in w:320 h:240 fmt:rgb24 sar:1/1
>> ref w:640 h:360 fmt:rgb24 sar:1/1
>> out w:160 h:120 fmt:rgb24 sar:4/3 flags:0x2
>> SAR: ((120 * 640) / (160 * 360)) * (1 / 1) = 4 / 3
>> DAR: (160 / 120) * (4 / 3) = 16 / 9
>> (main out now same DAR as ref)
>>
>> Now:
>> scale2ref=iw/4:ow/mdar
>> in w:320 h:240 fmt:rgb24 sar:1/1
>> ref w:640 h:360 fmt:rgb24 sar:1/1
>> out w:160 h:120 fmt:rgb24 sar:1/1 flags:0x2
>> SAR: ((120 * 320) / (160 * 240)) * (1 / 1) = 1 / 1
>> DAR: (160 / 120) * (1 / 1) = 4 / 3
>> (main out same DAR as main in)
>>
>> The scale2ref FATE test has also been updated.
>>
>> Signed-off-by: Kevin Mark
>> ---
>> libavfilter/vf_scale.c | 6 +++---
>> tests/ref/fate/filter-scale2ref_keep_aspect | 2 +-
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
>> index 9232bc4439..5e55f9344b 100644
>> --- a/libavfilter/vf_scale.c
>> +++ b/libavfilter/vf_scale.c
>> @@ -337,10 +337,10 @@ static int config_props(AVFilterLink *outlink)
>> }
>> }
>>
>> -if (inlink->sample_aspect_ratio.num){
>> -outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
>> inlink->w, outlink->w * inlink->h}, inlink->sample_aspect_ratio);
>> +if (inlink0->sample_aspect_ratio.num){
>> +outlink->sample_aspect_ratio = av_mul_q((AVRational){outlink->h *
>> inlink0->w, outlink->w * inlink0->h}, inlink0->sample_aspect_ratio);
>> } else
>> -outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> +outlink->sample_aspect_ratio = inlink0->sample_aspect_ratio;
>>
>> if (ctx->filter == &ff_vf_scale2ref) {
>> av_log(ctx, AV_LOG_VERBOSE, "in w:%d h:%d fmt:%s sar:%d/%d\n",
>> diff --git a/tests/ref/fate/filter-scale2ref_keep_aspect
>> b/tests/ref/fate/filter-scale2ref_keep_aspect
>> index ca03277446..8dd0dbb13b 100644
>> --- a/tests/ref/fate/filter-scale2ref_keep_aspect
>> +++ b/tests/ref/fate/filter-scale2ref_keep_aspect
>> @@ -5,7 +5,7 @@
>> #media_type 0: video
>> #codec_id 0: rawvideo
>> #dimensions 0: 160x120
>> -#sar 0: 4/3
>> +#sar 0: 1/1
>> #stream#, dts,pts, duration, size, hash
>> 0, 0, 0,1,57600,
>> 9a19c23dc3a557786840d0098606d5f1
>> 0, 1, 1,1,57600,
>> e6fbdabaf1bb0d28afc648ed4d27e9f0
>> --
>> 2.13.0
>>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
Hello all, Just wondering if anyone had any thoughts on the grepability issue, which I believe is the only potential issue/breaking change with this patch. Thanks, Kevin On Sun, Jun 11, 2017 at 9:59 PM, Kevin Mark wrote: > Hi Michael, > > Hoping to get your thoughts on the grepability issue (wrt my previous > email). If it needs to be on a single line there's no reason the new format > cannot be changed to do so (removing the \n and adding a separator, really). > However I'm a big fan of it as-is (for both scale and scale2ref) and I can't > find a case of my own where the regex I proposed would be troublesome. > > Thanks, > Kevin > > On Tue, Jun 6, 2017 at 4:17 PM Kevin Mark wrote: >> >> On Tue, Jun 6, 2017 at 11:49 AM, Michael Niedermayer >> wrote: >> > yes but its much harder to grep for as its not a single line anymore >> >> I agree that it's not going to be as pretty a regular expression to >> grep through, as there is 33% more data, but it should still be doable >> without too much effort. How important is it that we maintain "API" >> compatibility on verbose CLI output? >> >> ffmpeg [...] scale2ref=0:0 [...] -v verbose - 2>&1 >/dev/null | grep -oP >> 'regex' >> >> Where regex is: >> >> (in|out|ref) +w:(\d+) h:(\d+) fmt:(\w+) sar:(\d+)\/(\d+)(?: >> flags:0x[[:xdigit:]]+)? >> >> Assuming GNU grep 2.25+, you'll get: >> >> in w:320 h:240 fmt:rgb24 sar:1/1 >> ref w:640 h:360 fmt:rgb24 sar:1/1 >> out w:640 h:360 fmt:rgb24 sar:3/4 flags:0x2 >> >> It also works with BSD grep 2.5.1-FreeBSD included in macOS if you use >> the -E option instead of -P. These would be considered three separate >> matches so if you're using a good regex engine it'd be pretty easy to >> loop over each match, check the first group to determine if it's in, >> ref, or out and act accordingly on the rest of the captured data. You >> could also, if you wanted, assume that the first line is in and the >> second line is out if you only have two matches (or lines even) and if >> you have three matches/lines the first is in, second is ref, third is >> out. If you needed it to work with less sophisticated engines it >> shouldn't be too hard to dumb down the regex above. >> >> Live-ish example: https://regex101.com/r/wvHLpa/1 >> >> Is there a special property that makes single lines much easier to >> grep? Something specific to bash? I wouldn't think bash would have any >> problems looping over this by line. >> >> Thanks, >> Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 1/2] libavfilter/scale: More descriptive in/ref/out logging
Hi Ronald, On Wed, Jul 19, 2017 at 3:44 AM, Ronald S. Bultje wrote: > But my grep (Mac) has no -P option... I'd encourage you to keep things > simple and use a non-\n delimiter between the lines so non-GNU grep can > continue to be used for this also... Does this work for you: On Tue, Jun 6, 2017 at 1:17 PM, Kevin Mark wrote: > It also works with BSD grep 2.5.1-FreeBSD included in macOS if you use > the -E option instead of -P. Thanks, Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
