[WIP] rotate filter(s)
Hi, in attachment a first stub at the rotate filter. I don't know if it is a good idea to keep the float variant, while the rotate_ss may be useful (and the rotation angle could be made parametric). The integer-only rotating code is taken from tests/rotozoom.c, I don't know if we have a corresponding version in the libs. fill_line_with_color() and draw_rectangle() are duplicated from vf_pad.c, so maybe we should make them public (or at least ff_ them), drawutils.[ch] may be a good place where to put them. Note that ffplay/SDL doesn't work with odd width/height sizes, so you may need to rescale to an even wxh size (this could be done with a parametric scale as discussed some months ago). What this rotate filter still lacks, apart a resolution to the refactoring problems mentioned above, is a YUV planar version of the rotating function, and possibly the extension to other RGB variants, which should be pretty easy to add. Everyone feel free to continue to work on this and post an updated patch. Regards. -- FFmpeg = Fostering and Frightening Multimedia Portable Emblematic Guru
On Oct 3, 2010, at 1:35 PM, Stefano Sabatini wrote:
Hi,
in attachment a first stub at the rotate filter. I don't know if it is a good idea to keep the float variant, while the rotate_ss may be useful (and the rotation angle could be made parametric).
The float version doesn't look much better. Is it? I'd rather see the 90/180/270 cases special cased into flips, since those are the most common. And besides those, this filter doesn't seem to have any better interpolation than that general affine transformation patch. It doesn't need it immediately, but texture mapping has been around for 4 decades now, you know. Read http://www.cs.cmu.edu/~ph/texfund/texfund.pdf if you want.
+ switch (pix_fmt) { + case PIX_FMT_ARGB: rgba_map[ALPHA] = 0; rgba_map[RED ] = 1; rgba_map[GREEN] = 2; rgba_map[BLUE ] = 3; break; + case PIX_FMT_ABGR: rgba_map[ALPHA] = 0; rgba_map[BLUE ] = 1; rgba_map[GREEN] = 2; rgba_map[RED ] = 3; break; + case PIX_FMT_RGBA: + case PIX_FMT_RGB24: rgba_map[RED ] = 0; rgba_map[GREEN] = 1; rgba_map[BLUE ] = 2; rgba_map[ALPHA] = 3; break; + case PIX_FMT_BGRA: + case PIX_FMT_BGR24: rgba_map[BLUE ] = 0; rgba_map[GREEN] = 1; rgba_map[RED ] = 2; rgba_map[ALPHA] = 3; break; + default:
Strange indentation and long lines.
+ if (*is_packed_rgba) { + line_step[0] = (av_get_bits_per_pixel(pix_desc))>>3;
Unneeded parens.
+ for (plane = 0; plane < 4; plane++) { + int line_size; + int hsub1 = (plane == 1 || plane == 2) ? hsub : 0;
This and?
+ for (plane = 0; plane < 4 && outpic->data[plane]; plane++) { + int hsub1 = plane == 1 || plane == 2 ? hsub : 0; + int vsub1 = plane == 1 || plane == 2 ? vsub : 0;
?this disagree.
+ int ang;
angle
+/** + * Compute the power of a p of a using integer values. + * Input and output values are scaled by FIXP. + */
"Compute the power a^p"?
On Sun, Oct 03, 2010 at 07:35:49PM +0200, Stefano Sabatini wrote:
Hi,
in attachment a first stub at the rotate filter. I don't know if it is a good idea to keep the float variant, while the rotate_ss may be useful (and the rotation angle could be made parametric).
The integer-only rotating code is taken from tests/rotozoom.c, I don't know if we have a corresponding version in the libs.
fill_line_with_color() and draw_rectangle() are duplicated from vf_pad.c, so maybe we should make them public (or at least ff_ them), drawutils.[ch] may be a good place where to put them.
Note that ffplay/SDL doesn't work with odd width/height sizes, so you may need to rescale to an even wxh size (this could be done with a parametric scale as discussed some months ago).
What this rotate filter still lacks, apart a resolution to the refactoring problems mentioned above, is a YUV planar version of the rotating function, and possibly the extension to other RGB variants, which should be pretty easy to add. Everyone feel free to continue to work on this and post an updated patch.
looks like you implemented nearest neighbor sampling in int and float thats not what rotozoom does, also see vf_perspective.c [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Old school: Use the lowest level language in which you can solve the problem conveniently. New school: Use the highest level language in which the latest supercomputer can solve the problem without the user falling asleep waiting.
On date Sunday 2010-10-03 21:35:00 +0200, Michael Niedermayer encoded:
On Sun, Oct 03, 2010 at 07:35:49PM +0200, Stefano Sabatini wrote:
Hi,
in attachment a first stub at the rotate filter. I don't know if it is a good idea to keep the float variant, while the rotate_ss may be useful (and the rotation angle could be made parametric).
The integer-only rotating code is taken from tests/rotozoom.c, I don't know if we have a corresponding version in the libs.
fill_line_with_color() and draw_rectangle() are duplicated from vf_pad.c, so maybe we should make them public (or at least ff_ them), drawutils.[ch] may be a good place where to put them.
Note that ffplay/SDL doesn't work with odd width/height sizes, so you may need to rescale to an even wxh size (this could be done with a parametric scale as discussed some months ago).
What this rotate filter still lacks, apart a resolution to the refactoring problems mentioned above, is a YUV planar version of the rotating function, and possibly the extension to other RGB variants, which should be pretty easy to add. Everyone feel free to continue to work on this and post an updated patch.
looks like you implemented nearest neighbor sampling in int and float thats not what rotozoom does, also see vf_perspective.c
Implemented bilinear interpolation as in rotozoom.c, merged the three variants into a single one and made the expression for angle parametric. Still missing: more packed and planar formats for the non-float path, a more efficient way for filling the background. Should I remove the float path? Regards. -- FFmpeg = Fantastic and Friendly Mysterious Programmable Extreme God
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote:
On date Sunday 2010-10-03 21:35:00 +0200, Michael Niedermayer encoded:
On Sun, Oct 03, 2010 at 07:35:49PM +0200, Stefano Sabatini wrote:
Hi,
in attachment a first stub at the rotate filter. I don't know if it is a good idea to keep the float variant, while the rotate_ss may be useful (and the rotation angle could be made parametric).
The integer-only rotating code is taken from tests/rotozoom.c, I don't know if we have a corresponding version in the libs.
fill_line_with_color() and draw_rectangle() are duplicated from vf_pad.c, so maybe we should make them public (or at least ff_ them), drawutils.[ch] may be a good place where to put them.
Note that ffplay/SDL doesn't work with odd width/height sizes, so you may need to rescale to an even wxh size (this could be done with a parametric scale as discussed some months ago).
What this rotate filter still lacks, apart a resolution to the refactoring problems mentioned above, is a YUV planar version of the rotating function, and possibly the extension to other RGB variants, which should be pretty easy to add. Everyone feel free to continue to work on this and post an updated patch.
looks like you implemented nearest neighbor sampling in int and float thats not what rotozoom does, also see vf_perspective.c
Implemented bilinear interpolation as in rotozoom.c, merged the three variants into a single one and made the expression for angle parametric.
Still missing: more packed and planar formats for the non-float path, a more efficient way for filling the background.
Should I remove the float path?
if its slower then yes. [...]
+/** + * @file + * rotation filter + * + * @todo handle planar pixel and more packed formats in the non-float path +*/ + +#include "libavutil/eval.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/pixdesc.h" +#include "avfilter.h" +#include "drawutils.h" +#include "parseutils.h" + +static const char *var_names[] = { + "E", + "PHI", + "PI", + "w", ///< width of the input video + "h", ///< height of the input video + "n", ///< number of frame + "t", ///< timestamp expressed in seconds + NULL +}; + +enum var_name { + VAR_E, + VAR_PHI, + VAR_PI, + VAR_W, + VAR_H, + VAR_N, + VAR_T, + VAR_VARS_NB +}; +
+#define FIXP (1<<16) +#define INT_PI 205887 //(M_PI * FIXP) + +/** + * Compute the power of a a^p using integer values. + * Input and output values are scaled by FIXP. + */ +static int64_t int_pow(int64_t a, int p) +{ + int64_t v = FIXP; + + for (; p; p--) { + v *= a; + v /= FIXP; + } + + return v; +} + +/** + * Compute the sin of a using integer values. + * Input and output values are scaled by FIXP. + */ +static int64_t int_sin(int64_t a) +{ + if (a < 0) a = INT_PI-a; // 0..inf + a %= 2 * INT_PI; // 0..2PI + + if (a >= INT_PI*3/2) a -= 2*INT_PI; // -PI/2 .. 3PI/2 + if (a >= INT_PI/2 ) a = INT_PI - a; // -PI/2 .. PI/2 + + return a - int_pow(a, 3)/6 + int_pow(a, 5)/120 - int_pow(a, 7)/5040;
a2= a*a/X for(i=2; a; i+=2){ r+= a; a= -a*a2/(X*i*(i+1)); } and test it against sin() please and with the 16bit fixp most things fit in 32bit
+} + +/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
+ + return dst_color; +} + +typedef struct { + const AVClass *class; + int angle; + char *angle_expr; ///< expression for the angle + AVExpr *angle_pexpr; ///< parsed expression for the angle + uint8_t bgcolor[4]; ///< color expressed either in YUVA or RGBA colorspace for the padding area + char *bgcolor_str; + int hsub, vsub; + int use_float; + int use_bilinear; + int keep_same_size; + uint8_t *line[4]; + int line_step[4]; + float transx, transy; ///< how much to translate (in pixels) + float sinx, cosx; + int output_h, output_w; + double var_values[VAR_VARS_NB]; +} RotContext; + +#define OFFSET(x) offsetof(RotContext, x) + +static const AVOption rot_options[]= { + {"angle", "set angle expression", OFFSET(angle_expr), FF_OPT_TYPE_STRING, 0, CHAR_MIN, CHAR_MAX }, + {"bgcolor", "set background color", OFFSET(bgcolor_str), FF_OPT_TYPE_STRING, 0, CHAR_MIN, CHAR_MAX }, + {"float", "use float path", OFFSET(use_float), FF_OPT_TYPE_INT, 0, 0, 1 }, + {"ss", "keep same size", OFFSET(keep_same_size), FF_OPT_TYPE_INT, 0, 0, 1 }, + {"bilinear", "use bilinear interpolation", OFFSET(use_bilinear), FF_OPT_TYPE_INT, 1, 0, 1 }, + {NULL}, +};
bilinear should be default and this filter looks kinda big for what it does if i compare it to lets say vf_perspective that does more [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire
On date Tuesday 2010-10-05 14:36:19 +0200, Michael Niedermayer encoded:
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote:
On date Sunday 2010-10-03 21:35:00 +0200, Michael Niedermayer encoded:
On Sun, Oct 03, 2010 at 07:35:49PM +0200, Stefano Sabatini wrote:
Hi,
in attachment a first stub at the rotate filter. I don't know if it is a good idea to keep the float variant, while the rotate_ss may be useful (and the rotation angle could be made parametric).
The integer-only rotating code is taken from tests/rotozoom.c, I don't know if we have a corresponding version in the libs.
fill_line_with_color() and draw_rectangle() are duplicated from vf_pad.c, so maybe we should make them public (or at least ff_ them), drawutils.[ch] may be a good place where to put them.
Note that ffplay/SDL doesn't work with odd width/height sizes, so you may need to rescale to an even wxh size (this could be done with a parametric scale as discussed some months ago).
What this rotate filter still lacks, apart a resolution to the refactoring problems mentioned above, is a YUV planar version of the rotating function, and possibly the extension to other RGB variants, which should be pretty easy to add. Everyone feel free to continue to work on this and post an updated patch.
looks like you implemented nearest neighbor sampling in int and float thats not what rotozoom does, also see vf_perspective.c
Implemented bilinear interpolation as in rotozoom.c, merged the three variants into a single one and made the expression for angle parametric.
Still missing: more packed and planar formats for the non-float path, a more efficient way for filling the background.
Should I remove the float path?
if its slower then yes.
[...]
+/** + * @file + * rotation filter + * + * @todo handle planar pixel and more packed formats in the non-float path +*/ + +#include "libavutil/eval.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/pixdesc.h" +#include "avfilter.h" +#include "drawutils.h" +#include "parseutils.h" + +static const char *var_names[] = { + "E", + "PHI", + "PI", + "w", ///< width of the input video + "h", ///< height of the input video + "n", ///< number of frame + "t", ///< timestamp expressed in seconds + NULL +}; + +enum var_name { + VAR_E, + VAR_PHI, + VAR_PI, + VAR_W, + VAR_H, + VAR_N, + VAR_T, + VAR_VARS_NB +}; +
+#define FIXP (1<<16) +#define INT_PI 205887 //(M_PI * FIXP) + +/** + * Compute the power of a a^p using integer values. + * Input and output values are scaled by FIXP. + */ +static int64_t int_pow(int64_t a, int p) +{ + int64_t v = FIXP; + + for (; p; p--) { + v *= a; + v /= FIXP; + } + + return v; +} + +/** + * Compute the sin of a using integer values. + * Input and output values are scaled by FIXP. + */ +static int64_t int_sin(int64_t a) +{ + if (a < 0) a = INT_PI-a; // 0..inf + a %= 2 * INT_PI; // 0..2PI + + if (a >= INT_PI*3/2) a -= 2*INT_PI; // -PI/2 .. 3PI/2 + if (a >= INT_PI/2 ) a = INT_PI - a; // -PI/2 .. PI/2 + + return a - int_pow(a, 3)/6 + int_pow(a, 5)/120 - int_pow(a, 7)/5040;
a2= a*a/X for(i=2; a; i+=2){ r+= a; a= -a*a2/(X*i*(i+1)); }
and test it against sin() please and with the 16bit fixp most things fit in 32bit
+} + +/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
+ + return dst_color; +} + +typedef struct { + const AVClass *class; + int angle; + char *angle_expr; ///< expression for the angle + AVExpr *angle_pexpr; ///< parsed expression for the angle + uint8_t bgcolor[4]; ///< color expressed either in YUVA or RGBA colorspace for the padding area + char *bgcolor_str; + int hsub, vsub; + int use_float; + int use_bilinear; + int keep_same_size; + uint8_t *line[4]; + int line_step[4]; + float transx, transy; ///< how much to translate (in pixels) + float sinx, cosx; + int output_h, output_w; + double var_values[VAR_VARS_NB]; +} RotContext; + +#define OFFSET(x) offsetof(RotContext, x) + +static const AVOption rot_options[]= { + {"angle", "set angle expression", OFFSET(angle_expr), FF_OPT_TYPE_STRING, 0, CHAR_MIN, CHAR_MAX }, + {"bgcolor", "set background color", OFFSET(bgcolor_str), FF_OPT_TYPE_STRING, 0, CHAR_MIN, CHAR_MAX }, + {"float", "use float path", OFFSET(use_float), FF_OPT_TYPE_INT, 0, 0, 1 }, + {"ss", "keep same size", OFFSET(keep_same_size), FF_OPT_TYPE_INT, 0, 0, 1 }, + {"bilinear", "use bilinear interpolation", OFFSET(use_bilinear), FF_OPT_TYPE_INT, 1, 0, 1 }, + {NULL}, +};
bilinear should be default
and this filter looks kinda big for what it does if i compare it to lets say vf_perspective that does more
Most comments yet to be addressed. The filter is big because of the float path and the parametric option (and maybe can be simplified more). Regards. -- FFmpeg = Funny & Faithful Mere Puristic Everlasting Gargoyle
On date Tuesday 2010-10-05 14:36:19 +0200, Michael Niedermayer encoded:
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote: [...]
+/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
I can do: int s0 = ((1<<16) - frac_x)*s00 + frac_x*s01; int s1 = ((1<<16) - frac_x)*s10 + frac_x*s11; dst_color[i] = (((int64_t)(1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32; Is this what you suggest? Stupid question: what's this supposed to make us gain? (I suppose may be in term of precision, speed-wise I suppose this is slower...). Regards. -- FFmpeg = Friendly and Fantastic Martial Peaceful Elfic Glue
On Tue, Oct 05, 2010 at 09:38:06PM +0200, Stefano Sabatini wrote:
On date Tuesday 2010-10-05 14:36:19 +0200, Michael Niedermayer encoded:
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote: [...]
+/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
I can do: int s0 = ((1<<16) - frac_x)*s00 + frac_x*s01; int s1 = ((1<<16) - frac_x)*s10 + frac_x*s11;
dst_color[i] = (((int64_t)(1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32;
Is this what you suggest?
frac_x >>=6 frac_y >>=6 outside the loop [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt.
On date Wednesday 2010-10-06 16:21:16 +0200, Michael Niedermayer wrote:
On Tue, Oct 05, 2010 at 09:38:06PM +0200, Stefano Sabatini wrote:
On date Tuesday 2010-10-05 14:36:19 +0200, Michael Niedermayer encoded:
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote: [...]
+/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
I can do: int s0 = ((1<<16) - frac_x)*s00 + frac_x*s01; int s1 = ((1<<16) - frac_x)*s10 + frac_x*s11;
dst_color[i] = (((int64_t)(1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32;
Is this what you suggest?
frac_x >>=6 frac_y >>=6 outside the loop
Updated work in progress, with *lots* interface improvements, I have yet to fix the problems spotted by Michael, also IIRC I seem to remeber there was a problem with formats for which hsub != vsub (which are currently disabled). I'd like to committ the filter, even if we develop a more advanced affine/prospective filter, since this will be possible more efficient (no float arithmetic) and more suited to this particular task (and the filter is laying on ML since too much time IMO).
On date Tuesday 2011-05-03 23:40:10 +0200, Stefano Sabatini encoded:
On date Wednesday 2010-10-06 16:21:16 +0200, Michael Niedermayer wrote:
On Tue, Oct 05, 2010 at 09:38:06PM +0200, Stefano Sabatini wrote:
On date Tuesday 2010-10-05 14:36:19 +0200, Michael Niedermayer encoded:
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote: [...]
+/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
I can do: int s0 = ((1<<16) - frac_x)*s00 + frac_x*s01; int s1 = ((1<<16) - frac_x)*s10 + frac_x*s11;
dst_color[i] = (((int64_t)(1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32;
Is this what you suggest?
frac_x >>=6 frac_y >>=6 outside the loop
Updated work in progress, with *lots* interface improvements, I have yet to fix the problems spotted by Michael, also IIRC I seem to remeber there was a problem with formats for which hsub != vsub (which are currently disabled).
I'd like to committ the filter, even if we develop a more advanced affine/prospective filter, since this will be possible more efficient (no float arithmetic) and more suited to this particular task (and the filter is laying on ML since too much time IMO).
Update with some minor fixes, and with regression tests added. Bikeshed: I'm not sure if it is better to make the angle mandatory, and require thus: rotate=123 rather than make it an option, and thus settable via: rotate=a=123 the first is slightly simpler (and compatible with the old rotate), but doesn't allow rotate with no arguments. Michael please comment on the ipol >> issue, I can't find a way as you suggest. -- FFmpeg = Fostering Friendly Mind-dumbing Programmable Elegant God
On Wed, May 04, 2011 at 01:32:46AM +0200, Stefano Sabatini wrote:
On date Tuesday 2011-05-03 23:40:10 +0200, Stefano Sabatini encoded:
On date Wednesday 2010-10-06 16:21:16 +0200, Michael Niedermayer wrote:
On Tue, Oct 05, 2010 at 09:38:06PM +0200, Stefano Sabatini wrote:
On date Tuesday 2010-10-05 14:36:19 +0200, Michael Niedermayer encoded:
On Mon, Oct 04, 2010 at 10:27:29PM +0200, Stefano Sabatini wrote: [...]
+/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, const int src_linesize, int x, int y, + int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + + for (i = 0; i < 3; i++) { + int s00 = src[3 * int_x + i + src_linesize * int_y]; + int s01 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * int_y]; + int s10 = src[3 * int_x + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s11 = src[3 * FFMIN(int_x+1,max_x) + i + src_linesize * FFMIN(int_y+1, max_y)]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01)>>8; + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11)>>8; + + dst_color[i] = (((1<<16) - frac_y)*s0 + frac_y*s1)>>24; + }
the >>8 can be avoided by adjusting perecission sanely the FFMIN doesnt belong in the loop
I can do: int s0 = ((1<<16) - frac_x)*s00 + frac_x*s01; int s1 = ((1<<16) - frac_x)*s10 + frac_x*s11;
dst_color[i] = (((int64_t)(1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32;
Is this what you suggest?
frac_x >>=6 frac_y >>=6 outside the loop
Updated work in progress, with *lots* interface improvements, I have yet to fix the problems spotted by Michael, also IIRC I seem to remeber there was a problem with formats for which hsub != vsub (which are currently disabled).
I'd like to committ the filter, even if we develop a more advanced affine/prospective filter, since this will be possible more efficient (no float arithmetic) and more suited to this particular task (and the filter is laying on ML since too much time IMO).
yes, rotting patches arent good
Update with some minor fixes, and with regression tests added.
Bikeshed: I'm not sure if it is better to make the angle mandatory, and require thus: rotate=123
rather than make it an option, and thus settable via: rotate=a=123
pick whatever you prefer
the first is slightly simpler (and compatible with the old rotate), but doesn't allow rotate with no arguments.
Michael please comment on the ipol >> issue, I can't find a way as you suggest.
[...]
+/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, int src_linesize, int src_linestep, + int x, int y, int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + int int_x1 = FFMIN(int_x+1,max_x); + int int_y1 = FFMIN(int_y+1,max_y); +
+ for (i = 0; i < src_linestep; i++) { + int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; + int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; + int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; + int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01); + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11); + + dst_color[i] = ((int64_t)((1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32; + }
frac_x >>= 6; frac_y >>= 6; for (i = 0; i < src_linestep; i++) { int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01); int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11); dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20; } you can also shift by just 4 instead of 6 and use unsigned values [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The worst form of inequality is to try to make unequal things equal. -- Aristotle
On date Wednesday 2011-05-04 02:53:57 +0200, Michael Niedermayer encoded:
On Wed, May 04, 2011 at 01:32:46AM +0200, Stefano Sabatini wrote: [...]
Update with some minor fixes, and with regression tests added.
Bikeshed: I'm not sure if it is better to make the angle mandatory, and require thus: rotate=123
rather than make it an option, and thus settable via: rotate=a=123
pick whatever you prefer
Opted for making the rotate argument mandatory.
the first is slightly simpler (and compatible with the old rotate), but doesn't allow rotate with no arguments.
Michael please comment on the ipol >> issue, I can't find a way as you suggest. [...] +/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, int src_linesize, int src_linestep, + int x, int y, int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + int int_x1 = FFMIN(int_x+1,max_x); + int int_y1 = FFMIN(int_y+1,max_y); +
+ for (i = 0; i < src_linestep; i++) { + int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; + int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; + int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; + int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01); + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11); + + dst_color[i] = ((int64_t)((1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32; + }
frac_x >>= 6; frac_y >>= 6; for (i = 0; i < src_linestep; i++) { int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01); int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11);
dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20; }
you can also shift by just 4 instead of 6 and use unsigned values
Isn't this losing precision (and indeed producing a different output)? Which are the pros/cons? -- FFmpeg = Foolish and Forgiving MultiPurpose Elected Goblin
On Wed, May 04, 2011 at 12:31:29PM +0200, Stefano Sabatini wrote:
On date Wednesday 2011-05-04 02:53:57 +0200, Michael Niedermayer encoded:
On Wed, May 04, 2011 at 01:32:46AM +0200, Stefano Sabatini wrote: [...]
Update with some minor fixes, and with regression tests added.
Bikeshed: I'm not sure if it is better to make the angle mandatory, and require thus: rotate=123
rather than make it an option, and thus settable via: rotate=a=123
pick whatever you prefer
Opted for making the rotate argument mandatory.
the first is slightly simpler (and compatible with the old rotate), but doesn't allow rotate with no arguments.
Michael please comment on the ipol >> issue, I can't find a way as you suggest. [...] +/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + * + * @param dst_color put here the destination color + */ +static uint8_t *ipol(uint8_t *dst_color, + const uint8_t *src, int src_linesize, int src_linestep, + int x, int y, int max_x, int max_y) +{ + int int_x = x>>16; + int int_y = y>>16; + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + int int_x1 = FFMIN(int_x+1,max_x); + int int_y1 = FFMIN(int_y+1,max_y); +
+ for (i = 0; i < src_linestep; i++) { + int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; + int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; + int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; + int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01); + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11); + + dst_color[i] = ((int64_t)((1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32; + }
frac_x >>= 6; frac_y >>= 6; for (i = 0; i < src_linestep; i++) { int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01); int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11);
dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20; }
you can also shift by just 4 instead of 6 and use unsigned values
Isn't this losing precision (and indeed producing a different output)? Which are the pros/cons?
64bit arithmetic is slow on hw that does not have native 64bit support and with infinite precisse coeffs you will have +-0.5 errors with 16bit ~0.502 with 12bit ~0.531 these worst case errors happen on black white transitions. in flatly colored areas the difference is significantly smaller that is its all pretty much 0.5 there also mmx/sse optimized code would not use 64bit arithmetic so it would produce different output when C is using 64bit arithmetic [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The misfortune of the wise is better than the prosperity of the fool. -- Epicurus
On date Wednesday 2011-05-04 14:56:12 +0200, Michael Niedermayer encoded:
On Wed, May 04, 2011 at 12:31:29PM +0200, Stefano Sabatini wrote: [...]
frac_x >>= 6; frac_y >>= 6; for (i = 0; i < src_linestep; i++) { int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01); int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11);
dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20; }
you can also shift by just 4 instead of 6 and use unsigned values
Isn't this losing precision (and indeed producing a different output)? Which are the pros/cons?
64bit arithmetic is slow on hw that does not have native 64bit support and with infinite precisse coeffs you will have +-0.5 errors with 16bit ~0.502 with 12bit ~0.531
these worst case errors happen on black white transitions. in flatly colored areas the difference is significantly smaller that is its all pretty much 0.5 there
also mmx/sse optimized code would not use 64bit arithmetic so it would produce different output when C is using 64bit arithmetic
Yes I see the point but the code changed like that as a rather visible quality loss. BTW I also see another problem with the current code: rotate=0:fillcolor=yellow (or any other color, but with yellow is more visible) will produce a yellow line on the top and right sides of the image. So it looks I need to develop a better understanding of the code (which may also help with the hsub!=vsub problem). Last WIP version attached. -- FFmpeg = Faithful and Fostering Merciless Patchable Erroneous Gospel
On Thu, May 05, 2011 at 01:31:26AM +0200, Stefano Sabatini wrote:
On date Wednesday 2011-05-04 14:56:12 +0200, Michael Niedermayer encoded:
On Wed, May 04, 2011 at 12:31:29PM +0200, Stefano Sabatini wrote: [...]
frac_x >>= 6; frac_y >>= 6; for (i = 0; i < src_linestep; i++) { int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01); int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11);
dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20; }
you can also shift by just 4 instead of 6 and use unsigned values
Isn't this losing precision (and indeed producing a different output)? Which are the pros/cons?
64bit arithmetic is slow on hw that does not have native 64bit support and with infinite precisse coeffs you will have +-0.5 errors with 16bit ~0.502 with 12bit ~0.531
these worst case errors happen on black white transitions. in flatly colored areas the difference is significantly smaller that is its all pretty much 0.5 there
also mmx/sse optimized code would not use 64bit arithmetic so it would produce different output when C is using 64bit arithmetic
Yes I see the point but the code changed like that as a rather visible quality loss.
well ive not tested, it could contain bugs. you could commit the int64 version and i could test and change to int32 later maybe you could publish your work in a branch or something this way we could easier both work on the code. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire
On date Thursday 2011-05-05 04:17:55 +0200, Michael Niedermayer encoded:
On Thu, May 05, 2011 at 01:31:26AM +0200, Stefano Sabatini wrote:
On date Wednesday 2011-05-04 14:56:12 +0200, Michael Niedermayer encoded:
On Wed, May 04, 2011 at 12:31:29PM +0200, Stefano Sabatini wrote: [...]
frac_x >>= 6; frac_y >>= 6; for (i = 0; i < src_linestep; i++) { int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01); int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11);
dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20; }
you can also shift by just 4 instead of 6 and use unsigned values
Isn't this losing precision (and indeed producing a different output)? Which are the pros/cons?
64bit arithmetic is slow on hw that does not have native 64bit support and with infinite precisse coeffs you will have +-0.5 errors with 16bit ~0.502 with 12bit ~0.531
these worst case errors happen on black white transitions. in flatly colored areas the difference is significantly smaller that is its all pretty much 0.5 there
also mmx/sse optimized code would not use 64bit arithmetic so it would produce different output when C is using 64bit arithmetic
Yes I see the point but the code changed like that as a rather visible quality loss.
well ive not tested, it could contain bugs. you could commit the int64 version and i could test and change to int32 later
maybe you could publish your work in a branch or something this way we could easier both work on the code.
Updated, please review. Fixed the border problem, updated/fixed/extended the code. -- FFmpeg = Frightening & Fierce Miracolous Patchable Exploitable Gadget
Stefano Sabatini <stefasab <at> gmail.com> writes:
+ PIX_FMT_RGBA
(AV_ missing.) Doesn't it also work for RGB0 and friends? [...]
+ PIX_FMT_YUV444P, PIX_FMT_YUVJ444P + PIX_FMT_YUV420P, PIX_FMT_YUVJ444P + PIX_FMT_YUVA420P, PIX_FMT_YUVJ444P
Looks like several typos to me. (Too many YUVJ444P, YUVA444P missing.) Carl Eugen
On date Tuesday 2013-06-11 13:58:54 +0000, Carl Eugen Hoyos encoded:
Stefano Sabatini <stefasab <at> gmail.com> writes:
+ PIX_FMT_RGBA
(AV_ missing.)
Fixed locally.
Doesn't it also work for RGB0 and friends?
[...]
+ PIX_FMT_YUV444P, PIX_FMT_YUVJ444P + PIX_FMT_YUV420P, PIX_FMT_YUVJ444P + PIX_FMT_YUVA420P, PIX_FMT_YUVJ444P
Looks like several typos to me. (Too many YUVJ444P, YUVA444P missing.)
Feel free to add more formats once the filter is committed. -- FFmpeg = Funny and Fascinating Mega Problematic Exciting Guru
On Tue, Jun 11, 2013 at 03:12:31PM +0200, Stefano Sabatini wrote: [..]
From e92156a05861c55699e4460fc1b9df1204dc7171 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini <stefasab@gmail.com> Date: Tue, 11 Jun 2013 10:31:59 +0200 Subject: [PATCH] lavfi: add rotate filter
Based on the libavfilter SOC filter by Vitor Sessak, with the following additions: * integer arithmetic * bilinear interpolation * RGB path * configurable parametric angle, output width and height
Address trac issue #1500. --- doc/filters.texi | 111 +++++++++++ libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_rotate.c | 456 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 569 insertions(+) create mode 100644 libavfilter/vf_rotate.c
diff --git a/doc/filters.texi b/doc/filters.texi index 4cb6710..7e8fff9 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -5741,6 +5741,117 @@ pp=hb|y/vb|a @end example @end itemize
+@section rotate +
+Rotate video by a chosen amount in degrees. By default, 45 degrees.
chosen → arbitrary? "chosen" sounds like a selection between a number of choice; at first I thought it was a transpose like filter.
+ +The filter accepts the following options: + +A description of the optional parameters follows. +@table @option +@item angle, a +Set an expression for the angle by which to rotate the input video +clockwise, expressed in radians degrees.
radians or degrees?
A negative value will result in +a counter-clockwise rotation. By default it is set to "0". +
+This expression is evaluated for each frame.
This is going to be fun... You spin me right round baby right round~
+ +@item out_w, ow +Set the output width expression, default value is "rotw(a)". +This expression is evaluated just once during configuration. + +@item out_h, oh +Set the output height expression, default value is "roth(a)". +This expression is evaluated just once during configuration. + +@item bilinear +Enable bilinear interpolation if set to 1, a value of 0 disables +it. Default value is 1. + +@item fillcolor, c +Set the color used to fill the output area not covered by the rotated +image. If the special value "none" is selected then no background is +printed (useful for example if the background is never shown). Default +value is "black". +@end table + +The expressions for the angle and the output size can contain the +following constants and functions: + +@table @option +@item n +sequential number of the input frame, starting from 0 + +@item t +time in seconds of the input frame, it is set to 0 when the filter is +configured + +@item hsub +@item vsub +horizontal and vertical chroma subsample values. For example for the +pixel format "yuv422p" @var{hsub} is 2 and @var{vsub} is 1. + +@item in_w, iw +@tiem in_h, ih +the input video width and heigth + +@item out_w, ow +@item out_h, oh +the output width and heigth, that is the size of the padded area as +specified by the @var{width} and @var{height} expressions + +@item rotw(a) +the minimal width required for completely containing the input video +rotated by @var{a} degress + +@item roth(a) +the minimal height required for completely containing the input video +rotated by @var{a} degress +@end table + +@section Examples + +@itemize +@item +Rotate the input by PI/6 degrees clockwise: +@example +rotate=PI/6 +@end example + +@item +Rotate the input by PI/6 degrees counter-clockwise: +@example +rotate=-PI/6 +@end example + +@item
+Apply a constant rotation with period $var{T}, starting from an angle of PI/3:
$ → @ Make sure the rest of the HTML output is OK.
+@example +rotate=PI/3+2*PI*t/@var{T} +@end example + +@item +Make the input video rotation oscillating with a period of @var{T} +seconds and an amplitude of @var{A} degrees: +@example +rotate=@var{A}*sin(2*PI/@var{T}*t) +@end example + +@item +Rotate the video, output size is choosen so that the whole rotating +input video is always completely contained in the output: +@example
+rotate=2*PI*t:ow=sqrt(iw*iw+ih*ih):oh=ow
hypot()
+@end example + +@item +Rotate the video, reduce the output size so that no background is ever +shown: +@example +rotate=2*PI*t:ow='min(iw,ih)/sqrt(2)':oh=ow +@end example +@end itemize + @section removelogo
Suppress a TV station logo, using an image file to determine which diff --git a/libavfilter/Makefile b/libavfilter/Makefile index 2d2ea45..9746bbb 100644 --- a/libavfilter/Makefile +++ b/libavfilter/Makefile @@ -168,6 +168,7 @@ OBJS-$(CONFIG_PERMS_FILTER) += f_perms.o OBJS-$(CONFIG_PIXDESCTEST_FILTER) += vf_pixdesctest.o OBJS-$(CONFIG_PP_FILTER) += vf_pp.o OBJS-$(CONFIG_REMOVELOGO_FILTER) += bbox.o lswsutils.o lavfutils.o vf_removelogo.o +OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o OBJS-$(CONFIG_SEPARATEFIELDS_FILTER) += vf_separatefields.o OBJS-$(CONFIG_SAB_FILTER) += vf_sab.o OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c index f9d9391..e802601 100644 --- a/libavfilter/allfilters.c +++ b/libavfilter/allfilters.c @@ -163,6 +163,7 @@ void avfilter_register_all(void) REGISTER_FILTER(PIXDESCTEST, pixdesctest, vf); REGISTER_FILTER(PP, pp, vf); REGISTER_FILTER(REMOVELOGO, removelogo, vf); + REGISTER_FILTER(ROTATE, rotate, vf); REGISTER_FILTER(SAB, sab, vf); REGISTER_FILTER(SCALE, scale, vf); REGISTER_FILTER(SELECT, select, vf); diff --git a/libavfilter/vf_rotate.c b/libavfilter/vf_rotate.c new file mode 100644 index 0000000..6a2c33f --- /dev/null +++ b/libavfilter/vf_rotate.c @@ -0,0 +1,456 @@ +/* + * Copyright (c) 2013 Stefano Sabatini + * Copyright (c) 2008 Vitor Sessak + * + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * @file + * rotation filter, partially based on the tests/rotozoom.c program +*/ + +#include "libavutil/avstring.h" +#include "libavutil/eval.h" +#include "libavutil/opt.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/parseutils.h" +#include "libavutil/pixdesc.h" + +#include "avfilter.h" +#include "drawutils.h" +#include "internal.h" +#include "video.h" + +static const char *var_names[] = { + "in_w" , "iw", ///< width of the input video + "in_h" , "ih", ///< height of the input video ^^^^^^ + "out_w", "ow", ///< width of the input video + "out_h", "oh", ///< height of the input video ^^^^^^ You should be consistent with the spacing
+ "hsub", "vsub", + "a", + "n", ///< number of frame + "t", ///< timestamp expressed in seconds + NULL +}; + +enum var_name { + VAR_IN_W , VAR_IW, + VAR_IN_H , VAR_IH, + VAR_OUT_W, VAR_OW, + VAR_OUT_H, VAR_OH, + VAR_HSUB, VAR_VSUB, + VAR_A, + VAR_N, + VAR_T, + VAR_VARS_NB +}; + +typedef struct { + const AVClass *class; + float angle; + char *angle_expr_str; ///< expression for the angle + AVExpr *angle_expr; ///< parsed expression for the angle + char *outw_expr_str, *outh_expr_str; + int outh, outw; + uint8_t fillcolor[4]; ///< color expressed either in YUVA or RGBA colorspace for the padding area + char *fillcolor_str; + int fillcolor_enable; + int hsub, vsub; + int nb_planes; + int use_bilinear; + uint8_t *line[4]; + int linestep[4]; + float sinx, cosx; + double var_values[VAR_VARS_NB]; +} RotContext; + +#define OFFSET(x) offsetof(RotContext, x) +#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM + +static const AVOption rotate_options[] = { + { "angle", "set angle (in radians)", OFFSET(angle_expr_str), AV_OPT_TYPE_STRING, {.str="0"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "a", "set angle (in radians)", OFFSET(angle_expr_str), AV_OPT_TYPE_STRING, {.str="0"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "out_w", "set output width expression", OFFSET(outw_expr_str), AV_OPT_TYPE_STRING, {.str="rotw(a)"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "ow", "set output width expression", OFFSET(outw_expr_str), AV_OPT_TYPE_STRING, {.str="rotw(a)"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "out_h", "set output height expression", OFFSET(outh_expr_str), AV_OPT_TYPE_STRING, {.str="roth(a)"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "oh", "set output width expression", OFFSET(outh_expr_str), AV_OPT_TYPE_STRING, {.str="roth(a)"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "fillcolor", "set background fill color", OFFSET(fillcolor_str), AV_OPT_TYPE_STRING, {.str="black"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "c", "set background fill color", OFFSET(fillcolor_str), AV_OPT_TYPE_STRING, {.str="black"}, CHAR_MIN, CHAR_MAX, .flags=FLAGS }, + { "bilinear", "use bilinear interpolation", OFFSET(use_bilinear), AV_OPT_TYPE_INT, {.i64=1}, 0, 1, .flags=FLAGS }, + { NULL } +}; + +AVFILTER_DEFINE_CLASS(rotate); + +static av_cold int init(AVFilterContext *ctx) +{ + RotContext *rot = ctx->priv; + + if (!strcmp(rot->fillcolor_str, "none")) + rot->fillcolor_enable = 0; + else if (av_parse_color(rot->fillcolor, rot->fillcolor_str, -1, ctx) >= 0) + rot->fillcolor_enable = 1; + else + return AVERROR(EINVAL); + return 0; +} + +static av_cold void uninit(AVFilterContext *ctx) +{ + RotContext *rot = ctx->priv; + int i; + + for (i = 0; i < 4; i++) { + av_freep(&rot->line[i]); + rot->linestep[i] = 0; + } + + av_expr_free(rot->angle_expr); + rot->angle_expr = NULL; +} + +static int query_formats(AVFilterContext *ctx) +{ + static enum PixelFormat pix_fmts[] = { + PIX_FMT_ARGB, PIX_FMT_RGBA, + PIX_FMT_ABGR, PIX_FMT_BGRA, + PIX_FMT_RGB24, PIX_FMT_BGR24, + PIX_FMT_GRAY8, + PIX_FMT_YUV444P, PIX_FMT_YUVJ444P, + PIX_FMT_YUV420P, PIX_FMT_YUVJ444P, + PIX_FMT_YUVA420P, PIX_FMT_YUVJ444P, + PIX_FMT_YUVJ420P, + PIX_FMT_NONE + }; + + ff_set_common_formats(ctx, ff_make_format_list(pix_fmts)); + return 0; +} + +static double get_rotated_w(void *opaque, double angle) +{ + RotContext *rot = opaque; + double inw = rot->var_values[VAR_IN_W]; + double inh = rot->var_values[VAR_IN_H]; + float sinx = sin(angle); + float cosx = cos(angle); + + return FFMAX(0, inh * sinx) + FFMAX(0, -inw * cosx) + + FFMAX(0, inw * cosx) + FFMAX(0, -inh * sinx); +} + +static double get_rotated_h(void *opaque, double angle) +{ + RotContext *rot = opaque; + double inw = rot->var_values[VAR_IN_W]; + double inh = rot->var_values[VAR_IN_H]; + float sinx = sin(angle); + float cosx = cos(angle); + + return FFMAX(0, -inh * cosx) + FFMAX(0, -inw * sinx) + + FFMAX(0, inh * cosx) + FFMAX(0, inw * sinx); +} + +static double (* const func1[])(void *, double) = {
+ (void *)get_rotated_w, + (void *)get_rotated_h,
Unless I'm missing something, the cast should not be needed, or there is something wrong.
+ NULL +}; + +static const char * const func1_names[] = { + "rotw", + "roth", + NULL +}; + +static int config_props(AVFilterLink *outlink) +{ + AVFilterContext *ctx = outlink->src; + RotContext *rot = ctx->priv; + AVFilterLink *inlink = ctx->inputs[0]; + const AVPixFmtDescriptor *pixdesc = av_pix_fmt_desc_get(inlink->format); + uint8_t rgba_color[4]; + int i, is_packed_rgba, ret; + double res; + char *expr; + + rot->hsub = pixdesc->log2_chroma_w; + rot->vsub = pixdesc->log2_chroma_h; + + rot->var_values[VAR_IN_W] = rot->var_values[VAR_IW] = inlink->w; + rot->var_values[VAR_IN_H] = rot->var_values[VAR_IH] = inlink->h; + rot->var_values[VAR_HSUB] = 1<<rot->hsub; + rot->var_values[VAR_VSUB] = 1<<rot->vsub; + rot->var_values[VAR_N] = 0; + rot->var_values[VAR_T] = 0; + rot->var_values[VAR_A] = NAN; + rot->var_values[VAR_OUT_W] = rot->var_values[VAR_OW] = NAN; + rot->var_values[VAR_OUT_H] = rot->var_values[VAR_OH] = NAN; + + if ((ret = av_expr_parse(&rot->angle_expr, rot->angle_expr_str, var_names,
(expr = rot->angle_expr_str) and goto eval_fail?
+ func1_names, func1, NULL, NULL, 0, ctx)) < 0) { + av_log(ctx, AV_LOG_ERROR, + "Error occurred parsing expression '%s'\n", rot->angle_expr_str); + return ret; + } + + res = av_expr_eval(rot->angle_expr, rot->var_values, NULL); + if (isnan(res)) { + av_log(ctx, AV_LOG_ERROR,
+ "Invalid angle expression '%s', evaluates to nan.\n",
nit: NaN
+ rot->angle_expr_str); + return AVERROR(EINVAL); + } + rot->angle = rot->var_values[VAR_A] = res; + + /* evaluate width and height */ + av_expr_parse_and_eval(&res, (expr = rot->outw_expr_str), + var_names, rot->var_values, + func1_names, func1, NULL, NULL, rot, 0, ctx); + rot->var_values[VAR_OUT_W] = rot->var_values[VAR_OW] = res; + rot->outw = res + 0.5; + if ((ret = av_expr_parse_and_eval(&res, (expr = rot->outh_expr_str), + var_names, rot->var_values, + func1_names, func1, NULL, NULL, rot, 0, ctx)) < 0) + goto eval_fail; + rot->var_values[VAR_OUT_H] = rot->var_values[VAR_OH] = res; + rot->outh = res + 0.5; + /* evaluate the width again, as it may depend on the evaluated output height */ + if ((ret = av_expr_parse_and_eval(&res, (expr = rot->outw_expr_str), + var_names, rot->var_values, + func1_names, func1, NULL, NULL, rot, 0, ctx)) < 0) + goto eval_fail; + rot->var_values[VAR_OUT_W] = rot->var_values[VAR_OW] = res; + rot->outw = res + 0.5; + + /* compute number of planes */ + rot->nb_planes = 0; + for (i = 0; i < 4; i++) { + const AVComponentDescriptor *comp = &(pixdesc->comp[i]);
nit: pointless ( )
+ rot->nb_planes = FFMAX(rot->nb_planes, comp->plane); + } + rot->nb_planes++; +
+ rot->outw &= ~((1 << rot->hsub) - 1); + rot->outh &= ~((1 << rot->vsub) - 1);
Can't you use on of the available macro for this?
+ outlink->w = rot->outw; + outlink->h = rot->outh; + + memcpy(rgba_color, rot->fillcolor, sizeof(rgba_color)); + ff_fill_line_with_color(rot->line, rot->linestep, outlink->w, rot->fillcolor, + outlink->format, rgba_color, &is_packed_rgba, NULL); + av_log(ctx, AV_LOG_INFO, + "angle:%f/PI w:%d h:%d -> w:%d h:%d bgcolor:0x%02X%02X%02X%02X[%s]\n", + rot->angle/M_PI, inlink->w, inlink->h, outlink->w, outlink->h, + rot->fillcolor[0], rot->fillcolor[1], rot->fillcolor[2], rot->fillcolor[3], + is_packed_rgba ? "rgba" : "yuva"); + + return 0; + +eval_fail: + av_log(NULL, AV_LOG_ERROR, + "Error when evaluating the expression '%s'\n", expr); + return ret; +} + +#define FIXP (1<<16) +#define INT_PI 205887 //(M_PI * FIXP) + +/** + * Compute the sin of a using integer values. + * Input and output values are scaled by FIXP. + */ +static int64_t int_sin(int64_t a) +{ + int64_t a2, res = 0; + int i; + if (a < 0) a = INT_PI-a; // 0..inf + a %= 2 * INT_PI; // 0..2PI + + if (a >= INT_PI*3/2) a -= 2*INT_PI; // -PI/2 .. 3PI/2 + if (a >= INT_PI/2 ) a = INT_PI - a; // -PI/2 .. PI/2 + + /* compute sin using Taylor series approximated to the third term */ + a2 = (a*a)/FIXP; + for (i = 2; i < 7; i += 2) { + res += a; + a = -a*a2 / (FIXP*i*(i+1)); + } + return res; +} + +/** + * Interpolate the color in src at position x and y using bilinear + * interpolation. + */ +static uint8_t *interpolate_bilinear(uint8_t *dst_color, + const uint8_t *src, int src_linesize, int src_linestep, + int x, int y, int max_x, int max_y) +{ + int int_x = av_clip(x>>16, 0, max_x); + int int_y = av_clip(y>>16, 0, max_y); + int frac_x = x&0xFFFF; + int frac_y = y&0xFFFF; + int i; + int int_x1 = FFMIN(int_x+1, max_x); + int int_y1 = FFMIN(int_y+1, max_y); + + for (i = 0; i < src_linestep; i++) { + int s00 = src[src_linestep * int_x + i + src_linesize * int_y ]; + int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ]; + int s10 = src[src_linestep * int_x + i + src_linesize * int_y1]; + int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1]; + int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01); + int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11); + + dst_color[i] = ((int64_t)((1<<16) - frac_y)*s0 + (int64_t)frac_y*s1) >> 32; + } + + return dst_color; +} + +static int filter_frame(AVFilterLink *inlink, AVFrame *in) +{ + AVFilterContext *ctx = inlink->dst; + AVFilterLink *outlink = ctx->outputs[0]; + AVFrame *out; + RotContext *rot = ctx->priv; + int angle_int, s, c, plane; + double res; + + out = ff_get_video_buffer(outlink, outlink->w, outlink->h); + if (!out) { + av_frame_free(&in); + return AVERROR(ENOMEM); + } + av_frame_copy_props(out, in); + + rot->var_values[VAR_N] = inlink->frame_count; + rot->var_values[VAR_T] = in->pts * av_q2d(inlink->time_base); + res = av_expr_eval(rot->angle_expr, rot->var_values, rot); + av_log(ctx, AV_LOG_DEBUG, "n:%f time:%f angle:%f/PI\n", + rot->var_values[VAR_N], rot->var_values[VAR_T], res/M_PI); + + angle_int = res * FIXP; + s = int_sin(angle_int); + c = int_sin(angle_int + INT_PI/2); + + /* fill background */ + if (rot->fillcolor_enable) + ff_draw_rectangle(out->data, out->linesize, + rot->line, rot->linestep, rot->hsub, rot->vsub, + 0, 0, outlink->w, outlink->h); + + for (plane = 0; plane < rot->nb_planes; plane++) { + int hsub = plane == 1 || plane == 2 ? rot->hsub : 0; + int vsub = plane == 1 || plane == 2 ? rot->vsub : 0; + int inw = FF_CEIL_RSHIFT(inlink->w, hsub); + int inh = FF_CEIL_RSHIFT(inlink->h, vsub); + int outw = FF_CEIL_RSHIFT(outlink->w, hsub); + int outh = FF_CEIL_RSHIFT(outlink->h, hsub); + + const int xi = -outw/2 * c; + const int yi = outw/2 * s; + int xprime = -outh/2 * s; + int yprime = -outh/2 * c; + int i, j, x, y; + + for (j = 0; j < outh; j++) { + x = xprime + xi + FIXP*inw/2; + y = yprime + yi + FIXP*inh/2; + + for (i = 0; i < outw; i++) { + int32_t v; + int x1, y1; + uint8_t *pin, *pout; + x += c; + y -= s; + x1 = x>>16; + y1 = y>>16; + + /* the out-of-range values avoid border artifacts */ + if (x1 >= -1 && x1 <= inw && y1 >= -1 && y1 <= inh) { + uint8_t inp_inv[4]; /* interpolated input value */ + pout = out->data[plane] + j * out->linesize[plane] + i * rot->linestep[plane]; + if (rot->use_bilinear) { + pin = interpolate_bilinear(inp_inv, + in->data[plane], in->linesize[plane], rot->linestep[plane], + x, y, inw-1, inh-1); + } else { + int x2 = av_clip(x1, 0, inw-1); + int y2 = av_clip(y1, 0, inh-1); + pin = in->data[plane] + y2 * in->linesize[plane] + x2 * rot->linestep[plane]; + } + switch (rot->linestep[plane]) { + case 1: + *pout = *pin; + break; + case 2: + *((uint16_t *)pout) = *((uint16_t *)pin); + break; + case 3: + v = AV_RB24(pin); + AV_WB24(pout, v); + break; + case 4: + *((uint32_t *)pout) = *((uint32_t *)pin); + break; + default: + memcpy(pout, pin, rot->linestep[plane]); + break; + } + } + } + xprime += s; + yprime += c; + } + } + + av_frame_free(&in); + return ff_filter_frame(outlink, out); +} + +static const AVFilterPad rotate_inputs[] = { + { + .name = "default", + .type = AVMEDIA_TYPE_VIDEO, + .filter_frame = filter_frame, + }, + { NULL } +}; + +static const AVFilterPad rotate_outputs[] = { + { + .name = "default", + .type = AVMEDIA_TYPE_VIDEO, + .config_props = config_props, + }, + { NULL } +}; + +AVFilter avfilter_vf_rotate = { + .name = "rotate", + .description = NULL_IF_CONFIG_SMALL("Rotate the input image."), + .priv_size = sizeof(RotContext), + .init = init, + .uninit = uninit,
+ .query_formats = query_formats,
nit++: use this one for align basis.
+ .inputs = rotate_inputs, + .outputs = rotate_outputs, + .priv_class = &rotate_class, +};
I didn't find anything obviously wrong, but I didn't look at the math involved. Patch LGTM. -- Clément B.
On Wed, Jun 12, 2013 at 09:56:36AM +0200, Clément Bœsch wrote: [...]
+AVFilter avfilter_vf_rotate = { + .name = "rotate", + .description = NULL_IF_CONFIG_SMALL("Rotate the input image."), + .priv_size = sizeof(RotContext), + .init = init, + .uninit = uninit,
+ .query_formats = query_formats,
nit++: use this one for align basis.
+ .inputs = rotate_inputs, + .outputs = rotate_outputs, + .priv_class = &rotate_class, +};
I didn't find anything obviously wrong, but I didn't look at the math involved. Patch LGTM.
Ah I forgot; what about timeline support? -- Clément B.
On date Wednesday 2013-06-12 09:56:36 +0200, Clément Bœsch encoded:
On Tue, Jun 11, 2013 at 03:12:31PM +0200, Stefano Sabatini wrote: [..]
From e92156a05861c55699e4460fc1b9df1204dc7171 Mon Sep 17 00:00:00 2001 From: Stefano Sabatini <stefasab@gmail.com> Date: Tue, 11 Jun 2013 10:31:59 +0200 Subject: [PATCH] lavfi: add rotate filter
Based on the libavfilter SOC filter by Vitor Sessak, with the following additions: * integer arithmetic * bilinear interpolation * RGB path * configurable parametric angle, output width and height
Address trac issue #1500. --- doc/filters.texi | 111 +++++++++++ libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + libavfilter/vf_rotate.c | 456 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 569 insertions(+) create mode 100644 libavfilter/vf_rotate.c
diff --git a/doc/filters.texi b/doc/filters.texi index 4cb6710..7e8fff9 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -5741,6 +5741,117 @@ pp=hb|y/vb|a @end example @end itemize
+@section rotate +
+Rotate video by a chosen amount in degrees. By default, 45 degrees.
chosen → arbitrary?
"chosen" sounds like a selection between a number of choice; at first I thought it was a transpose like filter.
+ +The filter accepts the following options: + +A description of the optional parameters follows. +@table @option +@item angle, a +Set an expression for the angle by which to rotate the input video +clockwise, expressed in radians degrees.
radians or degrees?
Fixed here and everywhere. [...]
+@item +Rotate the input by PI/6 degrees counter-clockwise: +@example +rotate=-PI/6 +@end example + +@item
+Apply a constant rotation with period $var{T}, starting from an angle of PI/3:
$ → @
Make sure the rest of the HTML output is OK.
[...]
+@item +Rotate the video, output size is choosen so that the whole rotating +input video is always completely contained in the output: +@example
+rotate=2*PI*t:ow=sqrt(iw*iw+ih*ih):oh=ow
hypot()
Nice find. [...]
+static int config_props(AVFilterLink *outlink) +{ + AVFilterContext *ctx = outlink->src; + RotContext *rot = ctx->priv; + AVFilterLink *inlink = ctx->inputs[0]; + const AVPixFmtDescriptor *pixdesc = av_pix_fmt_desc_get(inlink->format); + uint8_t rgba_color[4]; + int i, is_packed_rgba, ret; + double res; + char *expr; + + rot->hsub = pixdesc->log2_chroma_w; + rot->vsub = pixdesc->log2_chroma_h; + + rot->var_values[VAR_IN_W] = rot->var_values[VAR_IW] = inlink->w; + rot->var_values[VAR_IN_H] = rot->var_values[VAR_IH] = inlink->h; + rot->var_values[VAR_HSUB] = 1<<rot->hsub; + rot->var_values[VAR_VSUB] = 1<<rot->vsub; + rot->var_values[VAR_N] = 0; + rot->var_values[VAR_T] = 0; + rot->var_values[VAR_A] = NAN; + rot->var_values[VAR_OUT_W] = rot->var_values[VAR_OW] = NAN; + rot->var_values[VAR_OUT_H] = rot->var_values[VAR_OH] = NAN; + + if ((ret = av_expr_parse(&rot->angle_expr, rot->angle_expr_str, var_names,
(expr = rot->angle_expr_str) and goto eval_fail?
Changed.
+ func1_names, func1, NULL, NULL, 0, ctx)) < 0) { + av_log(ctx, AV_LOG_ERROR, + "Error occurred parsing expression '%s'\n", rot->angle_expr_str); + return ret; + } + + res = av_expr_eval(rot->angle_expr, rot->var_values, NULL); + if (isnan(res)) { + av_log(ctx, AV_LOG_ERROR,
+ "Invalid angle expression '%s', evaluates to nan.\n",
nit: NaN
[...]
+ /* compute number of planes */ + rot->nb_planes = 0; + for (i = 0; i < 4; i++) { + const AVComponentDescriptor *comp = &(pixdesc->comp[i]);
nit: pointless ( )
+ rot->nb_planes = FFMAX(rot->nb_planes, comp->plane); + } + rot->nb_planes++;
Now I'm using av_pix_fmt_count_planes().
+
+ rot->outw &= ~((1 << rot->hsub) - 1); + rot->outh &= ~((1 << rot->vsub) - 1);
Can't you use on of the available macro for this?
Looks like they are not required after all... [...]
+AVFilter avfilter_vf_rotate = { + .name = "rotate", + .description = NULL_IF_CONFIG_SMALL("Rotate the input image."), + .priv_size = sizeof(RotContext), + .init = init, + .uninit = uninit,
+ .query_formats = query_formats,
nit++: use this one for align basis.
Done.
+ .inputs = rotate_inputs, + .outputs = rotate_outputs, + .priv_class = &rotate_class, +};
I also added timeline support, which works for free out of the box.
I didn't find anything obviously wrong, but I didn't look at the math involved. Patch LGTM.
I'll test it more, I'll add a test, and push it if I don't see more comments. -- FFmpeg = Free and Forgiving Murdering Portentous Elastic Geek
Stefano Sabatini <stefasab <at> gmail.com> writes:
+ AV_PIX_FMT_YUV444P, AV_PIX_FMT_YUVJ444P, + AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUVJ444P, + AV_PIX_FMT_YUVA420P, AV_PIX_FMT_YUVJ444P,
Strange typos (YUVA444P?) still there. Carl Eugen
On Wed, 12 Jun 2013 14:57:53 +0000 (UTC), Carl Eugen Hoyos wrote:
Stefano Sabatini <stefasab <at> gmail.com> writes:
AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ444P, duplicated typo is what carl is trying to point your attention to. -compn
On date Wednesday 2013-06-12 12:29:07 -0400, compn encoded:
On Wed, 12 Jun 2013 14:57:53 +0000 (UTC), Carl Eugen Hoyos wrote:
Stefano Sabatini <stefasab <at> gmail.com> writes:
AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ444P,
duplicated typo is what carl is trying to point your attention to.
Fixed and pushed with various other changes. -- FFmpeg = Fundamentalist and Freak Majestic Prodigious Elitarian Glue
participants (9)
-
astrange＠ithinksw.com -
Carl Eugen Hoyos -
Clément Bœsch -
compn -
Michael Niedermayer -
michaelni＠gmx.at
-
Stefano Sabatini -
Stefano Sabatini -
stefano.sabatini-lala＠poste.it