diff mbox

[FFmpeg-devel,v1,1/2] lavf/vf_transpose: add exif orientation support

Message ID 20190525000427.25546-1-junli1026@gmail.com
State Superseded
Headers show

Commit Message

Jun Li May 25, 2019, 12:04 a.m. UTC
Add exif orientation support and expose an option.
---
 libavfilter/vf_transpose.c | 258 +++++++++++++++++++++++++++++--------
 1 file changed, 207 insertions(+), 51 deletions(-)

Comments

Nicolas George May 25, 2019, 9:45 a.m. UTC | #1
Jun Li (12019-05-24):
> Add exif orientation support and expose an option.
> ---
>  libavfilter/vf_transpose.c | 258 +++++++++++++++++++++++++++++--------
>  1 file changed, 207 insertions(+), 51 deletions(-)

If I read the code correctly, when orientation=1 (unchanged), this code
will copy the frame instead of just passing it unchanged. Did I miss
something?

Regards,
Jun Li May 26, 2019, 4:44 a.m. UTC | #2
On Sat, May 25, 2019 at 2:46 AM Nicolas George <george@nsup.org> wrote:

> Jun Li (12019-05-24):
> > Add exif orientation support and expose an option.
> > ---
> >  libavfilter/vf_transpose.c | 258 +++++++++++++++++++++++++++++--------
> >  1 file changed, 207 insertions(+), 51 deletions(-)
>
> If I read the code correctly, when orientation=1 (unchanged), this code
> will copy the frame instead of just passing it unchanged. Did I miss
> something?
>

    return 0;
@@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
*in)
     ThreadData td;
     AVFrame *out;

-    if (s->passthrough)
+    if (s->passthrough || s->orientation == 1)

I believe this line skip the frame copy ?

         return ff_filter_frame(outlink, in);


>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol May 26, 2019, 8:16 a.m. UTC | #3
On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
> Add exif orientation support and expose an option.
> ---
>  libavfilter/vf_transpose.c | 258 +++++++++++++++++++++++++++++--------
>  1 file changed, 207 insertions(+), 51 deletions(-)
>
> diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c
> index dd54947bd9..4aebfb9ee4 100644
> --- a/libavfilter/vf_transpose.c
> +++ b/libavfilter/vf_transpose.c
> @@ -46,6 +46,9 @@ typedef struct TransVtable {
>      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
>                              uint8_t *dst, ptrdiff_t dst_linesize,
>                              int w, int h);
> +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
> +                           uint8_t *dst, ptrdiff_t dst_linesize,
> +                           int line_dir, int w, int h);
>  } TransVtable;
>

This is slow, the operations should be in one loop.

>  typedef struct TransContext {
> @@ -56,7 +59,7 @@ typedef struct TransContext {
>
>      int passthrough;    ///< PassthroughType, landscape passthrough mode
> enabled
>      int dir;            ///< TransposeDir
> -
> +    int orientation;    ///< Orientation
>      TransVtable vtables[4];
>  } TransContext;
>
> @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src, ptrdiff_t
> src_linesize,
>      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8, 8);
>  }
>
> +
> +static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
> +                             uint8_t *dst, ptrdiff_t dst_linesize,
> +                             int line_dir, int w, int h)
> +{
> +    int x, y, i;
> +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> +        for (x = 0; x < w; x ++) {
> +            i = line_dir < 0 ? w-x-1 : x;
> +            dst[i] = src[x];
> +        }
> +    }
> +}
> +
> +static void copyline_block_16(uint8_t *src, ptrdiff_t src_linesize,
> +                              uint8_t *dst, ptrdiff_t dst_linesize,
> +                              int line_dir, int w, int h)
> +{
> +    int x, y, i;
> +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> +        for (x = 0; x < w; x ++) {
> +            i = line_dir < 0 ? w-x-1 : x;
> +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src + 2*x));
> +        }
> +    }
> +}
> +
> +static void copyline_block_24(uint8_t *src, ptrdiff_t src_linesize,
> +                              uint8_t *dst, ptrdiff_t dst_linesize,
> +                              int line_dir, int w, int h)
> +{
> +    int x, y, i;
> +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> +        for (x = 0; x < w; x++) {
> +            int32_t v = AV_RB24(src + 3*x);
> +            i = line_dir < 0 ? w-x-1 : x;
> +            AV_WB24(dst + 3*i, v);
> +        }
> +    }
> +}
> +
> +static void copyline_block_32(uint8_t *src, ptrdiff_t src_linesize,
> +                              uint8_t *dst, ptrdiff_t dst_linesize,
> +                              int line_dir, int w, int h)
> +{
> +    int x, y, i;
> +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> +        for (x = 0; x < w; x ++) {
> +            i = line_dir < 0 ? w-x-1 : x;
> +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src + 4*x));
> +        }
> +    }
> +}
> +
> +static void copyline_block_48(uint8_t *src, ptrdiff_t src_linesize,
> +                              uint8_t *dst, ptrdiff_t dst_linesize,
> +                              int line_dir, int w, int h)
> +{
> +    int x, y, i;
> +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> +        for (x = 0; x < w; x++) {
> +            int64_t v = AV_RB48(src + 6*x);
> +            i = line_dir < 0 ? w-x-1 : x;
> +            AV_WB48(dst + 6*i, v);
> +        }
> +    }
> +}
> +
> +static void copyline_block_64(uint8_t *src, ptrdiff_t src_linesize,
> +                              uint8_t *dst, ptrdiff_t dst_linesize,
> +                              int line_dir, int w, int h)
> +{
> +    int x, y, i;
> +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> +        for (x = 0; x < w; x ++) {
> +            i = line_dir < 0 ? w-x-1 : x;
> +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src + 8*x));
> +        }
> +    }
> +}
> +
> +static void set_outlink_width_height(AVFilterLink *inlink, AVFilterLink
> *outlink, int transpose)
> +{
> +    if (transpose) {
> +        outlink->w = inlink->h;
> +        outlink->h = inlink->w;
> +
> +        if (inlink->sample_aspect_ratio.num)
> +            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
> +
> inlink->sample_aspect_ratio);
> +        else
> +            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +    } else {
> +        outlink->w = inlink->w;
> +        outlink->h = inlink->h;
> +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +    }
> +}
> +
>  static int config_props_output(AVFilterLink *outlink)
>  {
>      AVFilterContext *ctx = outlink->src;
> @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink *outlink)
>
>      av_assert0(desc_in->nb_components == desc_out->nb_components);
>
> -
>      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
> +
> +    if (s->orientation && s->orientation < 5) {
> +        outlink->h = inlink->h;
> +        outlink->w = inlink->w;
> +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +    } else {
> +        outlink->w = inlink->h;
> +        outlink->h = inlink->w;
>
> -    outlink->w = inlink->h;
> -    outlink->h = inlink->w;
> -
> -    if (inlink->sample_aspect_ratio.num)
> -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
> -
> inlink->sample_aspect_ratio);
> -    else
> -        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +        if (inlink->sample_aspect_ratio.num)
> +            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
> +
> inlink->sample_aspect_ratio);
> +        else
> +            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> +    }
>
>      for (int i = 0; i < 4; i++) {
>          TransVtable *v = &s->vtables[i];
>          switch (s->pixsteps[i]) {
>          case 1: v->transpose_block = transpose_block_8_c;
> -                v->transpose_8x8   = transpose_8x8_8_c;  break;
> +                v->transpose_8x8   = transpose_8x8_8_c;
> +                v->copyline_block  = copyline_block_8; break;
>          case 2: v->transpose_block = transpose_block_16_c;
> -                v->transpose_8x8   = transpose_8x8_16_c; break;
> +                v->transpose_8x8   = transpose_8x8_16_c;
> +                v->copyline_block  = copyline_block_16; break;
>          case 3: v->transpose_block = transpose_block_24_c;
> -                v->transpose_8x8   = transpose_8x8_24_c; break;
> +                v->transpose_8x8   = transpose_8x8_24_c;
> +                v->copyline_block  = copyline_block_24; break;
>          case 4: v->transpose_block = transpose_block_32_c;
> -                v->transpose_8x8   = transpose_8x8_32_c; break;
> +                v->transpose_8x8   = transpose_8x8_32_c;
> +                v->copyline_block  = copyline_block_32; break;
>          case 6: v->transpose_block = transpose_block_48_c;
> -                v->transpose_8x8   = transpose_8x8_48_c; break;
> +                v->transpose_8x8   = transpose_8x8_48_c;
> +                v->copyline_block  = copyline_block_48; break;
>          case 8: v->transpose_block = transpose_block_64_c;
> -                v->transpose_8x8   = transpose_8x8_64_c; break;
> +                v->transpose_8x8   = transpose_8x8_64_c;
> +                v->copyline_block  = copyline_block_64; break;
>          }
>      }
>
> @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext *ctx, void
> *arg, int jobnr,
>          uint8_t *dst, *src;
>          int dstlinesize, srclinesize;
>          int x, y;
> +        int dir = s->dir;
>          TransVtable *v = &s->vtables[plane];
> +
> +        if (s->orientation > 0 && s->orientation < 5) {
> +            int line_dir = 1;
> +            dstlinesize = out->linesize[plane];
> +            dst         = out->data[plane] + start * dstlinesize;
> +            srclinesize = in->linesize[plane];
> +            src         = in->data[plane] + start * srclinesize;
> +
> +            switch (s->orientation) {
> +                case 2: // mirror horizontal
> +                    line_dir     = -1;
> +                    break;
> +                case 3: // rotate 180
> +                    dst          = out->data[plane] + (outh - start - 1) *
> dstlinesize;
> +                    line_dir     = -1;
> +                    dstlinesize *= -1;
> +                    break;
> +                case 4: // mirror vertical
> +                    dst          = out->data[plane] + (outh - start - 1) *
> dstlinesize;
> +                    dstlinesize *= -1;
> +                    break;
> +                default:
> +                    break;
> +            }
> +            v->copyline_block(src, srclinesize, dst, dstlinesize, line_dir,
> outw, end - start);
> +        } else {
> +            dstlinesize = out->linesize[plane];
> +            dst         = out->data[plane] + start * dstlinesize;
> +            src         = in->data[plane];
> +            srclinesize = in->linesize[plane];
> +            switch (s->orientation) {
> +                case 5: // mirror horizontal and rotate 270 CW
> +                    dir = 0; break;
> +                case 6: // rotate 90 CW
> +                    dir = 1; break;
> +                case 7: // mirror horizontal and rotate 90 CW
> +                    dir = 3; break;
> +                case 8: // rotate 270 CW
> +                    dir = 2; break;
> +                default: break;
> +            }
>
> -        dstlinesize = out->linesize[plane];
> -        dst         = out->data[plane] + start * dstlinesize;
> -        src         = in->data[plane];
> -        srclinesize = in->linesize[plane];
> -
> -        if (s->dir & 1) {
> -            src         += in->linesize[plane] * (inh - 1);
> -            srclinesize *= -1;
> -        }
> +            if (dir & 1) {
> +                src         += in->linesize[plane] * (inh - 1);
> +                srclinesize *= -1;
> +            }
>
> -        if (s->dir & 2) {
> -            dst          = out->data[plane] + dstlinesize * (outh - start -
> 1);
> -            dstlinesize *= -1;
> -        }
> +            if (dir & 2) {
> +                dst          = out->data[plane] + dstlinesize * (outh -
> start - 1);
> +                dstlinesize *= -1;
> +            }
>
> -        for (y = start; y < end - 7; y += 8) {
> -            for (x = 0; x < outw - 7; x += 8) {
> -                v->transpose_8x8(src + x * srclinesize + y * pixstep,
> -                                 srclinesize,
> -                                 dst + (y - start) * dstlinesize + x *
> pixstep,
> -                                 dstlinesize);
> +            for (y = start; y < end - 7; y += 8) {
> +                for (x = 0; x < outw - 7; x += 8) {
> +                    v->transpose_8x8(src + x * srclinesize + y * pixstep,
> +                                    srclinesize,
> +                                    dst + (y - start) * dstlinesize + x *
> pixstep,
> +                                    dstlinesize);
> +                }
> +                if (outw - x > 0 && end - y > 0)
> +                    v->transpose_block(src + x * srclinesize + y * pixstep,
> +                                    srclinesize,
> +                                    dst + (y - start) * dstlinesize + x *
> pixstep,
> +                                    dstlinesize, outw - x, end - y);
>              }
> -            if (outw - x > 0 && end - y > 0)
> -                v->transpose_block(src + x * srclinesize + y * pixstep,
> -                                   srclinesize,
> -                                   dst + (y - start) * dstlinesize + x *
> pixstep,
> -                                   dstlinesize, outw - x, end - y);
> -        }
>
> -        if (end - y > 0)
> -            v->transpose_block(src + 0 * srclinesize + y * pixstep,
> -                               srclinesize,
> -                               dst + (y - start) * dstlinesize + 0 *
> pixstep,
> -                               dstlinesize, outw, end - y);
> +            if (end - y > 0)
> +                v->transpose_block(src + 0 * srclinesize + y * pixstep,
> +                                srclinesize,
> +                                dst + (y - start) * dstlinesize + 0 *
> pixstep,
> +                                dstlinesize, outw, end - y);
> +        }
>      }
>
>      return 0;
> @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>      ThreadData td;
>      AVFrame *out;
>
> -    if (s->passthrough)
> +    if (s->passthrough || s->orientation == 1)
>          return ff_filter_frame(outlink, in);
>
>      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> *in)
>      if (in->sample_aspect_ratio.num == 0) {
>          out->sample_aspect_ratio = in->sample_aspect_ratio;
>      } else {
> -        out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
> -        out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
> +        if (s->orientation > 0 && s->orientation < 5) {
> +            out->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
> +            out->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
> +        } else {
> +            out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
> +            out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
> +        }
>      }
>
>      td.in = in, td.out = out;
> @@ -366,13 +522,13 @@ static const AVOption transpose_options[] = {
>          { "clock",       "rotate clockwise",                            0,
> AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS, .unit =
> "dir" },
>          { "cclock",      "rotate counter-clockwise",                    0,
> AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS, .unit =
> "dir" },
>          { "clock_flip",  "rotate clockwise with vertical flip",         0,
> AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS, .unit =
> "dir" },
> -
> +
>      { "passthrough", "do not apply transposition if the input matches the
> specified geometry",
>        OFFSET(passthrough), AV_OPT_TYPE_INT, {.i64=TRANSPOSE_PT_TYPE_NONE},
> 0, INT_MAX, FLAGS, "passthrough" },
>          { "none",      "always apply transposition",   0,
> AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN, INT_MAX,
> FLAGS, "passthrough" },
>          { "portrait",  "preserve portrait geometry",   0,
> AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN, INT_MAX,
> FLAGS, "passthrough" },
>          { "landscape", "preserve landscape geometry",  0,
> AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN, INT_MAX,
> FLAGS, "passthrough" },
> -
> +    { "orientation", "set exif orientation", OFFSET(orientation),
> AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
>      { NULL }
>  };
>
> --
> 2.17.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jun Li May 26, 2019, 8:39 a.m. UTC | #4
On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com> wrote:

> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
> > Add exif orientation support and expose an option.
> > ---
> >  libavfilter/vf_transpose.c | 258 +++++++++++++++++++++++++++++--------
> >  1 file changed, 207 insertions(+), 51 deletions(-)
> >
> > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c
> > index dd54947bd9..4aebfb9ee4 100644
> > --- a/libavfilter/vf_transpose.c
> > +++ b/libavfilter/vf_transpose.c
> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
> >      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
> >                              uint8_t *dst, ptrdiff_t dst_linesize,
> >                              int w, int h);
> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
> > +                           uint8_t *dst, ptrdiff_t dst_linesize,
> > +                           int line_dir, int w, int h);
> >  } TransVtable;
> >
>
> This is slow, the operations should be in one loop.
>

Thanks Paul for review.
I see transpose is using " ctx->internal->execute(ctx, filter_slice, &td,
NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
which looks like leveraging multi-threading.
If we do that in one loop, I guess it would be hard to use multi-threading
to accelerate ? But correct me if I am wrong.

Best Regards,
-Jun


> >  typedef struct TransContext {
> > @@ -56,7 +59,7 @@ typedef struct TransContext {
> >
> >      int passthrough;    ///< PassthroughType, landscape passthrough mode
> > enabled
> >      int dir;            ///< TransposeDir
> > -
> > +    int orientation;    ///< Orientation
> >      TransVtable vtables[4];
> >  } TransContext;
> >
> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src,
> ptrdiff_t
> > src_linesize,
> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8, 8);
> >  }
> >
> > +
> > +static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
> > +                             uint8_t *dst, ptrdiff_t dst_linesize,
> > +                             int line_dir, int w, int h)
> > +{
> > +    int x, y, i;
> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> > +        for (x = 0; x < w; x ++) {
> > +            i = line_dir < 0 ? w-x-1 : x;
> > +            dst[i] = src[x];
> > +        }
> > +    }
> > +}
> > +
> > +static void copyline_block_16(uint8_t *src, ptrdiff_t src_linesize,
> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> > +                              int line_dir, int w, int h)
> > +{
> > +    int x, y, i;
> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> > +        for (x = 0; x < w; x ++) {
> > +            i = line_dir < 0 ? w-x-1 : x;
> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src + 2*x));
> > +        }
> > +    }
> > +}
> > +
> > +static void copyline_block_24(uint8_t *src, ptrdiff_t src_linesize,
> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> > +                              int line_dir, int w, int h)
> > +{
> > +    int x, y, i;
> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> > +        for (x = 0; x < w; x++) {
> > +            int32_t v = AV_RB24(src + 3*x);
> > +            i = line_dir < 0 ? w-x-1 : x;
> > +            AV_WB24(dst + 3*i, v);
> > +        }
> > +    }
> > +}
> > +
> > +static void copyline_block_32(uint8_t *src, ptrdiff_t src_linesize,
> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> > +                              int line_dir, int w, int h)
> > +{
> > +    int x, y, i;
> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> > +        for (x = 0; x < w; x ++) {
> > +            i = line_dir < 0 ? w-x-1 : x;
> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src + 4*x));
> > +        }
> > +    }
> > +}
> > +
> > +static void copyline_block_48(uint8_t *src, ptrdiff_t src_linesize,
> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> > +                              int line_dir, int w, int h)
> > +{
> > +    int x, y, i;
> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> > +        for (x = 0; x < w; x++) {
> > +            int64_t v = AV_RB48(src + 6*x);
> > +            i = line_dir < 0 ? w-x-1 : x;
> > +            AV_WB48(dst + 6*i, v);
> > +        }
> > +    }
> > +}
> > +
> > +static void copyline_block_64(uint8_t *src, ptrdiff_t src_linesize,
> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> > +                              int line_dir, int w, int h)
> > +{
> > +    int x, y, i;
> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
> > +        for (x = 0; x < w; x ++) {
> > +            i = line_dir < 0 ? w-x-1 : x;
> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src + 8*x));
> > +        }
> > +    }
> > +}
> > +
> > +static void set_outlink_width_height(AVFilterLink *inlink, AVFilterLink
> > *outlink, int transpose)
> > +{
> > +    if (transpose) {
> > +        outlink->w = inlink->h;
> > +        outlink->h = inlink->w;
> > +
> > +        if (inlink->sample_aspect_ratio.num)
> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1
> },
> > +
> > inlink->sample_aspect_ratio);
> > +        else
> > +            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> > +    } else {
> > +        outlink->w = inlink->w;
> > +        outlink->h = inlink->h;
> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> > +    }
> > +}
> > +
> >  static int config_props_output(AVFilterLink *outlink)
> >  {
> >      AVFilterContext *ctx = outlink->src;
> > @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink
> *outlink)
> >
> >      av_assert0(desc_in->nb_components == desc_out->nb_components);
> >
> > -
> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
> > +
> > +    if (s->orientation && s->orientation < 5) {
> > +        outlink->h = inlink->h;
> > +        outlink->w = inlink->w;
> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> > +    } else {
> > +        outlink->w = inlink->h;
> > +        outlink->h = inlink->w;
> >
> > -    outlink->w = inlink->h;
> > -    outlink->h = inlink->w;
> > -
> > -    if (inlink->sample_aspect_ratio.num)
> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
> > -
> > inlink->sample_aspect_ratio);
> > -    else
> > -        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> > +        if (inlink->sample_aspect_ratio.num)
> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1
> },
> > +
> > inlink->sample_aspect_ratio);
> > +        else
> > +            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> > +    }
> >
> >      for (int i = 0; i < 4; i++) {
> >          TransVtable *v = &s->vtables[i];
> >          switch (s->pixsteps[i]) {
> >          case 1: v->transpose_block = transpose_block_8_c;
> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
> > +                v->transpose_8x8   = transpose_8x8_8_c;
> > +                v->copyline_block  = copyline_block_8; break;
> >          case 2: v->transpose_block = transpose_block_16_c;
> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
> > +                v->transpose_8x8   = transpose_8x8_16_c;
> > +                v->copyline_block  = copyline_block_16; break;
> >          case 3: v->transpose_block = transpose_block_24_c;
> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
> > +                v->transpose_8x8   = transpose_8x8_24_c;
> > +                v->copyline_block  = copyline_block_24; break;
> >          case 4: v->transpose_block = transpose_block_32_c;
> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
> > +                v->transpose_8x8   = transpose_8x8_32_c;
> > +                v->copyline_block  = copyline_block_32; break;
> >          case 6: v->transpose_block = transpose_block_48_c;
> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
> > +                v->transpose_8x8   = transpose_8x8_48_c;
> > +                v->copyline_block  = copyline_block_48; break;
> >          case 8: v->transpose_block = transpose_block_64_c;
> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
> > +                v->transpose_8x8   = transpose_8x8_64_c;
> > +                v->copyline_block  = copyline_block_64; break;
> >          }
> >      }
> >
> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext *ctx, void
> > *arg, int jobnr,
> >          uint8_t *dst, *src;
> >          int dstlinesize, srclinesize;
> >          int x, y;
> > +        int dir = s->dir;
> >          TransVtable *v = &s->vtables[plane];
> > +
> > +        if (s->orientation > 0 && s->orientation < 5) {
> > +            int line_dir = 1;
> > +            dstlinesize = out->linesize[plane];
> > +            dst         = out->data[plane] + start * dstlinesize;
> > +            srclinesize = in->linesize[plane];
> > +            src         = in->data[plane] + start * srclinesize;
> > +
> > +            switch (s->orientation) {
> > +                case 2: // mirror horizontal
> > +                    line_dir     = -1;
> > +                    break;
> > +                case 3: // rotate 180
> > +                    dst          = out->data[plane] + (outh - start -
> 1) *
> > dstlinesize;
> > +                    line_dir     = -1;
> > +                    dstlinesize *= -1;
> > +                    break;
> > +                case 4: // mirror vertical
> > +                    dst          = out->data[plane] + (outh - start -
> 1) *
> > dstlinesize;
> > +                    dstlinesize *= -1;
> > +                    break;
> > +                default:
> > +                    break;
> > +            }
> > +            v->copyline_block(src, srclinesize, dst, dstlinesize,
> line_dir,
> > outw, end - start);
> > +        } else {
> > +            dstlinesize = out->linesize[plane];
> > +            dst         = out->data[plane] + start * dstlinesize;
> > +            src         = in->data[plane];
> > +            srclinesize = in->linesize[plane];
> > +            switch (s->orientation) {
> > +                case 5: // mirror horizontal and rotate 270 CW
> > +                    dir = 0; break;
> > +                case 6: // rotate 90 CW
> > +                    dir = 1; break;
> > +                case 7: // mirror horizontal and rotate 90 CW
> > +                    dir = 3; break;
> > +                case 8: // rotate 270 CW
> > +                    dir = 2; break;
> > +                default: break;
> > +            }
> >
> > -        dstlinesize = out->linesize[plane];
> > -        dst         = out->data[plane] + start * dstlinesize;
> > -        src         = in->data[plane];
> > -        srclinesize = in->linesize[plane];
> > -
> > -        if (s->dir & 1) {
> > -            src         += in->linesize[plane] * (inh - 1);
> > -            srclinesize *= -1;
> > -        }
> > +            if (dir & 1) {
> > +                src         += in->linesize[plane] * (inh - 1);
> > +                srclinesize *= -1;
> > +            }
> >
> > -        if (s->dir & 2) {
> > -            dst          = out->data[plane] + dstlinesize * (outh -
> start -
> > 1);
> > -            dstlinesize *= -1;
> > -        }
> > +            if (dir & 2) {
> > +                dst          = out->data[plane] + dstlinesize * (outh -
> > start - 1);
> > +                dstlinesize *= -1;
> > +            }
> >
> > -        for (y = start; y < end - 7; y += 8) {
> > -            for (x = 0; x < outw - 7; x += 8) {
> > -                v->transpose_8x8(src + x * srclinesize + y * pixstep,
> > -                                 srclinesize,
> > -                                 dst + (y - start) * dstlinesize + x *
> > pixstep,
> > -                                 dstlinesize);
> > +            for (y = start; y < end - 7; y += 8) {
> > +                for (x = 0; x < outw - 7; x += 8) {
> > +                    v->transpose_8x8(src + x * srclinesize + y *
> pixstep,
> > +                                    srclinesize,
> > +                                    dst + (y - start) * dstlinesize + x
> *
> > pixstep,
> > +                                    dstlinesize);
> > +                }
> > +                if (outw - x > 0 && end - y > 0)
> > +                    v->transpose_block(src + x * srclinesize + y *
> pixstep,
> > +                                    srclinesize,
> > +                                    dst + (y - start) * dstlinesize + x
> *
> > pixstep,
> > +                                    dstlinesize, outw - x, end - y);
> >              }
> > -            if (outw - x > 0 && end - y > 0)
> > -                v->transpose_block(src + x * srclinesize + y * pixstep,
> > -                                   srclinesize,
> > -                                   dst + (y - start) * dstlinesize + x *
> > pixstep,
> > -                                   dstlinesize, outw - x, end - y);
> > -        }
> >
> > -        if (end - y > 0)
> > -            v->transpose_block(src + 0 * srclinesize + y * pixstep,
> > -                               srclinesize,
> > -                               dst + (y - start) * dstlinesize + 0 *
> > pixstep,
> > -                               dstlinesize, outw, end - y);
> > +            if (end - y > 0)
> > +                v->transpose_block(src + 0 * srclinesize + y * pixstep,
> > +                                srclinesize,
> > +                                dst + (y - start) * dstlinesize + 0 *
> > pixstep,
> > +                                dstlinesize, outw, end - y);
> > +        }
> >      }
> >
> >      return 0;
> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
> > *in)
> >      ThreadData td;
> >      AVFrame *out;
> >
> > -    if (s->passthrough)
> > +    if (s->passthrough || s->orientation == 1)
> >          return ff_filter_frame(outlink, in);
> >
> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink,
> AVFrame
> > *in)
> >      if (in->sample_aspect_ratio.num == 0) {
> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
> >      } else {
> > -        out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
> > -        out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
> > +        if (s->orientation > 0 && s->orientation < 5) {
> > +            out->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
> > +            out->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
> > +        } else {
> > +            out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
> > +            out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
> > +        }
> >      }
> >
> >      td.in = in, td.out = out;
> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[] = {
> >          { "clock",       "rotate clockwise",
> 0,
> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS, .unit
> =
> > "dir" },
> >          { "cclock",      "rotate counter-clockwise",
> 0,
> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS, .unit
> =
> > "dir" },
> >          { "clock_flip",  "rotate clockwise with vertical flip",
>  0,
> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS, .unit
> =
> > "dir" },
> > -
> > +
> >      { "passthrough", "do not apply transposition if the input matches
> the
> > specified geometry",
> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
> {.i64=TRANSPOSE_PT_TYPE_NONE},
> > 0, INT_MAX, FLAGS, "passthrough" },
> >          { "none",      "always apply transposition",   0,
> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN, INT_MAX,
> > FLAGS, "passthrough" },
> >          { "portrait",  "preserve portrait geometry",   0,
> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN, INT_MAX,
> > FLAGS, "passthrough" },
> >          { "landscape", "preserve landscape geometry",  0,
> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN, INT_MAX,
> > FLAGS, "passthrough" },
> > -
> > +    { "orientation", "set exif orientation", OFFSET(orientation),
> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
> >      { NULL }
> >  };
> >
> > --
> > 2.17.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol May 26, 2019, 9:09 a.m. UTC | #5
On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
> On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com> wrote:
>
>> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
>> > Add exif orientation support and expose an option.
>> > ---
>> >  libavfilter/vf_transpose.c | 258 +++++++++++++++++++++++++++++--------
>> >  1 file changed, 207 insertions(+), 51 deletions(-)
>> >
>> > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c
>> > index dd54947bd9..4aebfb9ee4 100644
>> > --- a/libavfilter/vf_transpose.c
>> > +++ b/libavfilter/vf_transpose.c
>> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
>> >      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
>> >                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >                              int w, int h);
>> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
>> > +                           uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                           int line_dir, int w, int h);
>> >  } TransVtable;
>> >
>>
>> This is slow, the operations should be in one loop.
>>
>
> Thanks Paul for review.
> I see transpose is using " ctx->internal->execute(ctx, filter_slice, &td,
> NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
> which looks like leveraging multi-threading.
> If we do that in one loop, I guess it would be hard to use multi-threading
> to accelerate ? But correct me if I am wrong.

Transpose filter does one thing and that is transposing.

If I'm not mistaken you are adding vertical and horizontal flipping?

If you really need to do that, do it efficiently in one loop, by that I mean
you do not do cascading operations at all.

>
> Best Regards,
> -Jun
>
>
>> >  typedef struct TransContext {
>> > @@ -56,7 +59,7 @@ typedef struct TransContext {
>> >
>> >      int passthrough;    ///< PassthroughType, landscape passthrough
>> > mode
>> > enabled
>> >      int dir;            ///< TransposeDir
>> > -
>> > +    int orientation;    ///< Orientation
>> >      TransVtable vtables[4];
>> >  } TransContext;
>> >
>> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src,
>> ptrdiff_t
>> > src_linesize,
>> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8, 8);
>> >  }
>> >
>> > +
>> > +static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
>> > +                             uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                             int line_dir, int w, int h)
>> > +{
>> > +    int x, y, i;
>> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
>> > +        for (x = 0; x < w; x ++) {
>> > +            i = line_dir < 0 ? w-x-1 : x;
>> > +            dst[i] = src[x];
>> > +        }
>> > +    }
>> > +}
>> > +
>> > +static void copyline_block_16(uint8_t *src, ptrdiff_t src_linesize,
>> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                              int line_dir, int w, int h)
>> > +{
>> > +    int x, y, i;
>> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
>> > +        for (x = 0; x < w; x ++) {
>> > +            i = line_dir < 0 ? w-x-1 : x;
>> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src + 2*x));
>> > +        }
>> > +    }
>> > +}
>> > +
>> > +static void copyline_block_24(uint8_t *src, ptrdiff_t src_linesize,
>> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                              int line_dir, int w, int h)
>> > +{
>> > +    int x, y, i;
>> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
>> > +        for (x = 0; x < w; x++) {
>> > +            int32_t v = AV_RB24(src + 3*x);
>> > +            i = line_dir < 0 ? w-x-1 : x;
>> > +            AV_WB24(dst + 3*i, v);
>> > +        }
>> > +    }
>> > +}
>> > +
>> > +static void copyline_block_32(uint8_t *src, ptrdiff_t src_linesize,
>> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                              int line_dir, int w, int h)
>> > +{
>> > +    int x, y, i;
>> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
>> > +        for (x = 0; x < w; x ++) {
>> > +            i = line_dir < 0 ? w-x-1 : x;
>> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src + 4*x));
>> > +        }
>> > +    }
>> > +}
>> > +
>> > +static void copyline_block_48(uint8_t *src, ptrdiff_t src_linesize,
>> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                              int line_dir, int w, int h)
>> > +{
>> > +    int x, y, i;
>> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
>> > +        for (x = 0; x < w; x++) {
>> > +            int64_t v = AV_RB48(src + 6*x);
>> > +            i = line_dir < 0 ? w-x-1 : x;
>> > +            AV_WB48(dst + 6*i, v);
>> > +        }
>> > +    }
>> > +}
>> > +
>> > +static void copyline_block_64(uint8_t *src, ptrdiff_t src_linesize,
>> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> > +                              int line_dir, int w, int h)
>> > +{
>> > +    int x, y, i;
>> > +    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
>> > +        for (x = 0; x < w; x ++) {
>> > +            i = line_dir < 0 ? w-x-1 : x;
>> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src + 8*x));
>> > +        }
>> > +    }
>> > +}
>> > +
>> > +static void set_outlink_width_height(AVFilterLink *inlink, AVFilterLink
>> > *outlink, int transpose)
>> > +{
>> > +    if (transpose) {
>> > +        outlink->w = inlink->h;
>> > +        outlink->h = inlink->w;
>> > +
>> > +        if (inlink->sample_aspect_ratio.num)
>> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1
>> },
>> > +
>> > inlink->sample_aspect_ratio);
>> > +        else
>> > +            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> > +    } else {
>> > +        outlink->w = inlink->w;
>> > +        outlink->h = inlink->h;
>> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> > +    }
>> > +}
>> > +
>> >  static int config_props_output(AVFilterLink *outlink)
>> >  {
>> >      AVFilterContext *ctx = outlink->src;
>> > @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink
>> *outlink)
>> >
>> >      av_assert0(desc_in->nb_components == desc_out->nb_components);
>> >
>> > -
>> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
>> > +
>> > +    if (s->orientation && s->orientation < 5) {
>> > +        outlink->h = inlink->h;
>> > +        outlink->w = inlink->w;
>> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> > +    } else {
>> > +        outlink->w = inlink->h;
>> > +        outlink->h = inlink->w;
>> >
>> > -    outlink->w = inlink->h;
>> > -    outlink->h = inlink->w;
>> > -
>> > -    if (inlink->sample_aspect_ratio.num)
>> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
>> > -
>> > inlink->sample_aspect_ratio);
>> > -    else
>> > -        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> > +        if (inlink->sample_aspect_ratio.num)
>> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1
>> },
>> > +
>> > inlink->sample_aspect_ratio);
>> > +        else
>> > +            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> > +    }
>> >
>> >      for (int i = 0; i < 4; i++) {
>> >          TransVtable *v = &s->vtables[i];
>> >          switch (s->pixsteps[i]) {
>> >          case 1: v->transpose_block = transpose_block_8_c;
>> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
>> > +                v->transpose_8x8   = transpose_8x8_8_c;
>> > +                v->copyline_block  = copyline_block_8; break;
>> >          case 2: v->transpose_block = transpose_block_16_c;
>> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
>> > +                v->transpose_8x8   = transpose_8x8_16_c;
>> > +                v->copyline_block  = copyline_block_16; break;
>> >          case 3: v->transpose_block = transpose_block_24_c;
>> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
>> > +                v->transpose_8x8   = transpose_8x8_24_c;
>> > +                v->copyline_block  = copyline_block_24; break;
>> >          case 4: v->transpose_block = transpose_block_32_c;
>> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
>> > +                v->transpose_8x8   = transpose_8x8_32_c;
>> > +                v->copyline_block  = copyline_block_32; break;
>> >          case 6: v->transpose_block = transpose_block_48_c;
>> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
>> > +                v->transpose_8x8   = transpose_8x8_48_c;
>> > +                v->copyline_block  = copyline_block_48; break;
>> >          case 8: v->transpose_block = transpose_block_64_c;
>> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
>> > +                v->transpose_8x8   = transpose_8x8_64_c;
>> > +                v->copyline_block  = copyline_block_64; break;
>> >          }
>> >      }
>> >
>> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext *ctx, void
>> > *arg, int jobnr,
>> >          uint8_t *dst, *src;
>> >          int dstlinesize, srclinesize;
>> >          int x, y;
>> > +        int dir = s->dir;
>> >          TransVtable *v = &s->vtables[plane];
>> > +
>> > +        if (s->orientation > 0 && s->orientation < 5) {
>> > +            int line_dir = 1;
>> > +            dstlinesize = out->linesize[plane];
>> > +            dst         = out->data[plane] + start * dstlinesize;
>> > +            srclinesize = in->linesize[plane];
>> > +            src         = in->data[plane] + start * srclinesize;
>> > +
>> > +            switch (s->orientation) {
>> > +                case 2: // mirror horizontal
>> > +                    line_dir     = -1;
>> > +                    break;
>> > +                case 3: // rotate 180
>> > +                    dst          = out->data[plane] + (outh - start -
>> 1) *
>> > dstlinesize;
>> > +                    line_dir     = -1;
>> > +                    dstlinesize *= -1;
>> > +                    break;
>> > +                case 4: // mirror vertical
>> > +                    dst          = out->data[plane] + (outh - start -
>> 1) *
>> > dstlinesize;
>> > +                    dstlinesize *= -1;
>> > +                    break;
>> > +                default:
>> > +                    break;
>> > +            }
>> > +            v->copyline_block(src, srclinesize, dst, dstlinesize,
>> line_dir,
>> > outw, end - start);
>> > +        } else {
>> > +            dstlinesize = out->linesize[plane];
>> > +            dst         = out->data[plane] + start * dstlinesize;
>> > +            src         = in->data[plane];
>> > +            srclinesize = in->linesize[plane];
>> > +            switch (s->orientation) {
>> > +                case 5: // mirror horizontal and rotate 270 CW
>> > +                    dir = 0; break;
>> > +                case 6: // rotate 90 CW
>> > +                    dir = 1; break;
>> > +                case 7: // mirror horizontal and rotate 90 CW
>> > +                    dir = 3; break;
>> > +                case 8: // rotate 270 CW
>> > +                    dir = 2; break;
>> > +                default: break;
>> > +            }
>> >
>> > -        dstlinesize = out->linesize[plane];
>> > -        dst         = out->data[plane] + start * dstlinesize;
>> > -        src         = in->data[plane];
>> > -        srclinesize = in->linesize[plane];
>> > -
>> > -        if (s->dir & 1) {
>> > -            src         += in->linesize[plane] * (inh - 1);
>> > -            srclinesize *= -1;
>> > -        }
>> > +            if (dir & 1) {
>> > +                src         += in->linesize[plane] * (inh - 1);
>> > +                srclinesize *= -1;
>> > +            }
>> >
>> > -        if (s->dir & 2) {
>> > -            dst          = out->data[plane] + dstlinesize * (outh -
>> start -
>> > 1);
>> > -            dstlinesize *= -1;
>> > -        }
>> > +            if (dir & 2) {
>> > +                dst          = out->data[plane] + dstlinesize * (outh -
>> > start - 1);
>> > +                dstlinesize *= -1;
>> > +            }
>> >
>> > -        for (y = start; y < end - 7; y += 8) {
>> > -            for (x = 0; x < outw - 7; x += 8) {
>> > -                v->transpose_8x8(src + x * srclinesize + y * pixstep,
>> > -                                 srclinesize,
>> > -                                 dst + (y - start) * dstlinesize + x *
>> > pixstep,
>> > -                                 dstlinesize);
>> > +            for (y = start; y < end - 7; y += 8) {
>> > +                for (x = 0; x < outw - 7; x += 8) {
>> > +                    v->transpose_8x8(src + x * srclinesize + y *
>> pixstep,
>> > +                                    srclinesize,
>> > +                                    dst + (y - start) * dstlinesize + x
>> *
>> > pixstep,
>> > +                                    dstlinesize);
>> > +                }
>> > +                if (outw - x > 0 && end - y > 0)
>> > +                    v->transpose_block(src + x * srclinesize + y *
>> pixstep,
>> > +                                    srclinesize,
>> > +                                    dst + (y - start) * dstlinesize + x
>> *
>> > pixstep,
>> > +                                    dstlinesize, outw - x, end - y);
>> >              }
>> > -            if (outw - x > 0 && end - y > 0)
>> > -                v->transpose_block(src + x * srclinesize + y * pixstep,
>> > -                                   srclinesize,
>> > -                                   dst + (y - start) * dstlinesize + x
>> > *
>> > pixstep,
>> > -                                   dstlinesize, outw - x, end - y);
>> > -        }
>> >
>> > -        if (end - y > 0)
>> > -            v->transpose_block(src + 0 * srclinesize + y * pixstep,
>> > -                               srclinesize,
>> > -                               dst + (y - start) * dstlinesize + 0 *
>> > pixstep,
>> > -                               dstlinesize, outw, end - y);
>> > +            if (end - y > 0)
>> > +                v->transpose_block(src + 0 * srclinesize + y * pixstep,
>> > +                                srclinesize,
>> > +                                dst + (y - start) * dstlinesize + 0 *
>> > pixstep,
>> > +                                dstlinesize, outw, end - y);
>> > +        }
>> >      }
>> >
>> >      return 0;
>> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink,
>> > AVFrame
>> > *in)
>> >      ThreadData td;
>> >      AVFrame *out;
>> >
>> > -    if (s->passthrough)
>> > +    if (s->passthrough || s->orientation == 1)
>> >          return ff_filter_frame(outlink, in);
>> >
>> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink,
>> AVFrame
>> > *in)
>> >      if (in->sample_aspect_ratio.num == 0) {
>> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
>> >      } else {
>> > -        out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
>> > -        out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
>> > +        if (s->orientation > 0 && s->orientation < 5) {
>> > +            out->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
>> > +            out->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
>> > +        } else {
>> > +            out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
>> > +            out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
>> > +        }
>> >      }
>> >
>> >      td.in = in, td.out = out;
>> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[] = {
>> >          { "clock",       "rotate clockwise",
>> 0,
>> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS, .unit
>> =
>> > "dir" },
>> >          { "cclock",      "rotate counter-clockwise",
>> 0,
>> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS, .unit
>> =
>> > "dir" },
>> >          { "clock_flip",  "rotate clockwise with vertical flip",
>>  0,
>> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS, .unit
>> =
>> > "dir" },
>> > -
>> > +
>> >      { "passthrough", "do not apply transposition if the input matches
>> the
>> > specified geometry",
>> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
>> {.i64=TRANSPOSE_PT_TYPE_NONE},
>> > 0, INT_MAX, FLAGS, "passthrough" },
>> >          { "none",      "always apply transposition",   0,
>> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN, INT_MAX,
>> > FLAGS, "passthrough" },
>> >          { "portrait",  "preserve portrait geometry",   0,
>> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN, INT_MAX,
>> > FLAGS, "passthrough" },
>> >          { "landscape", "preserve landscape geometry",  0,
>> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN, INT_MAX,
>> > FLAGS, "passthrough" },
>> > -
>> > +    { "orientation", "set exif orientation", OFFSET(orientation),
>> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
>> >      { NULL }
>> >  };
>> >
>> > --
>> > 2.17.1
>> >
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> > To unsubscribe, visit link above, or email
>> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jun Li May 26, 2019, 11:06 p.m. UTC | #6
On Sun, May 26, 2019 at 2:09 AM Paul B Mahol <onemda@gmail.com> wrote:

> On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
> > On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com> wrote:
> >
> >> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
> >> > Add exif orientation support and expose an option.
> >> > ---
> >> >  libavfilter/vf_transpose.c | 258
> +++++++++++++++++++++++++++++--------
> >> >  1 file changed, 207 insertions(+), 51 deletions(-)
> >> >
> >> > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c
> >> > index dd54947bd9..4aebfb9ee4 100644
> >> > --- a/libavfilter/vf_transpose.c
> >> > +++ b/libavfilter/vf_transpose.c
> >> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
> >> >      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
> >> >                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> >                              int w, int h);
> >> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                           uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                           int line_dir, int w, int h);
> >> >  } TransVtable;
> >> >
> >>
> >> This is slow, the operations should be in one loop.
> >>
> >
> > Thanks Paul for review.
> > I see transpose is using " ctx->internal->execute(ctx, filter_slice, &td,
> > NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
> > which looks like leveraging multi-threading.
> > If we do that in one loop, I guess it would be hard to use
> multi-threading
> > to accelerate ? But correct me if I am wrong.
>
> Transpose filter does one thing and that is transposing.
>
> If I'm not mistaken you are adding vertical and horizontal flipping?
>

Thanks for review.
Yes, I am doing vflip, hflip and rotate180(vflip+hflip).


> If you really need to do that, do it efficiently in one loop, by that I
> mean
> you do not do cascading operations at all.
>

Are you suggesting not use function pointer, and call the function directly
like this:
            switch (s->pixsteps[plane]) {
                case 1:  copyline_block_8(src, srclinesize, dst,
dstlinesize, line_dir, outw, end - start); break;
                case 2:  copyline_block_16(src, srclinesize, dst,
dstlinesize, line_dir, outw, end - start); break;
                case 3:  copyline_block_24(src, srclinesize, dst,
dstlinesize, line_dir, outw, end - start); break;
                case 4:  copyline_block_32(src, srclinesize, dst,
dstlinesize, line_dir, outw, end - start); break;
                case 6:  copyline_block_48(src, srclinesize, dst,
dstlinesize, line_dir, outw, end - start); break;
                case 8:  copyline_block_64(src, srclinesize, dst,
dstlinesize, line_dir, outw, end - start); break;
            }

I did some perf comparsion, they are very close and no significant
difference, about 9ms for one frame.

Also I did some comparison between current hflip filter and this patch,
they take almost the same time, about 9ms too on my machine and I see 13
threads/jobs are used for filtering.
I just use the simple command "ffmpeg  -i test.bmp -vf
transpose=orientation=2  -y hello.jpg".
Current vflip wins a lot and I see it is not copying frame at all, just
like "orientation=1" or "passthrough" cases.

I hope I understand you correctly, especially about "cascading operations".
But correct me if I am wrong.
Thanks for your time Paul.

Best Regards,
-Jun

>
> > Best Regards,
> > -Jun
> >
> >
> >> >  typedef struct TransContext {
> >> > @@ -56,7 +59,7 @@ typedef struct TransContext {
> >> >
> >> >      int passthrough;    ///< PassthroughType, landscape passthrough
> >> > mode
> >> > enabled
> >> >      int dir;            ///< TransposeDir
> >> > -
> >> > +    int orientation;    ///< Orientation
> >> >      TransVtable vtables[4];
> >> >  } TransContext;
> >> >
> >> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src,
> >> ptrdiff_t
> >> > src_linesize,
> >> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8, 8);
> >> >  }
> >> >
> >> > +
> >> > +static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                             uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                             int line_dir, int w, int h)
> >> > +{
> >> > +    int x, y, i;
> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> src_linesize) {
> >> > +        for (x = 0; x < w; x ++) {
> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> > +            dst[i] = src[x];
> >> > +        }
> >> > +    }
> >> > +}
> >> > +
> >> > +static void copyline_block_16(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                              int line_dir, int w, int h)
> >> > +{
> >> > +    int x, y, i;
> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> src_linesize) {
> >> > +        for (x = 0; x < w; x ++) {
> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src + 2*x));
> >> > +        }
> >> > +    }
> >> > +}
> >> > +
> >> > +static void copyline_block_24(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                              int line_dir, int w, int h)
> >> > +{
> >> > +    int x, y, i;
> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> src_linesize) {
> >> > +        for (x = 0; x < w; x++) {
> >> > +            int32_t v = AV_RB24(src + 3*x);
> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> > +            AV_WB24(dst + 3*i, v);
> >> > +        }
> >> > +    }
> >> > +}
> >> > +
> >> > +static void copyline_block_32(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                              int line_dir, int w, int h)
> >> > +{
> >> > +    int x, y, i;
> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> src_linesize) {
> >> > +        for (x = 0; x < w; x ++) {
> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src + 4*x));
> >> > +        }
> >> > +    }
> >> > +}
> >> > +
> >> > +static void copyline_block_48(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                              int line_dir, int w, int h)
> >> > +{
> >> > +    int x, y, i;
> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> src_linesize) {
> >> > +        for (x = 0; x < w; x++) {
> >> > +            int64_t v = AV_RB48(src + 6*x);
> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> > +            AV_WB48(dst + 6*i, v);
> >> > +        }
> >> > +    }
> >> > +}
> >> > +
> >> > +static void copyline_block_64(uint8_t *src, ptrdiff_t src_linesize,
> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> > +                              int line_dir, int w, int h)
> >> > +{
> >> > +    int x, y, i;
> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> src_linesize) {
> >> > +        for (x = 0; x < w; x ++) {
> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src + 8*x));
> >> > +        }
> >> > +    }
> >> > +}
> >> > +
> >> > +static void set_outlink_width_height(AVFilterLink *inlink,
> AVFilterLink
> >> > *outlink, int transpose)
> >> > +{
> >> > +    if (transpose) {
> >> > +        outlink->w = inlink->h;
> >> > +        outlink->h = inlink->w;
> >> > +
> >> > +        if (inlink->sample_aspect_ratio.num)
> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) {
> 1, 1
> >> },
> >> > +
> >> > inlink->sample_aspect_ratio);
> >> > +        else
> >> > +            outlink->sample_aspect_ratio =
> inlink->sample_aspect_ratio;
> >> > +    } else {
> >> > +        outlink->w = inlink->w;
> >> > +        outlink->h = inlink->h;
> >> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> >> > +    }
> >> > +}
> >> > +
> >> >  static int config_props_output(AVFilterLink *outlink)
> >> >  {
> >> >      AVFilterContext *ctx = outlink->src;
> >> > @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink
> >> *outlink)
> >> >
> >> >      av_assert0(desc_in->nb_components == desc_out->nb_components);
> >> >
> >> > -
> >> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
> >> > +
> >> > +    if (s->orientation && s->orientation < 5) {
> >> > +        outlink->h = inlink->h;
> >> > +        outlink->w = inlink->w;
> >> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> >> > +    } else {
> >> > +        outlink->w = inlink->h;
> >> > +        outlink->h = inlink->w;
> >> >
> >> > -    outlink->w = inlink->h;
> >> > -    outlink->h = inlink->w;
> >> > -
> >> > -    if (inlink->sample_aspect_ratio.num)
> >> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1
> },
> >> > -
> >> > inlink->sample_aspect_ratio);
> >> > -    else
> >> > -        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> >> > +        if (inlink->sample_aspect_ratio.num)
> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) {
> 1, 1
> >> },
> >> > +
> >> > inlink->sample_aspect_ratio);
> >> > +        else
> >> > +            outlink->sample_aspect_ratio =
> inlink->sample_aspect_ratio;
> >> > +    }
> >> >
> >> >      for (int i = 0; i < 4; i++) {
> >> >          TransVtable *v = &s->vtables[i];
> >> >          switch (s->pixsteps[i]) {
> >> >          case 1: v->transpose_block = transpose_block_8_c;
> >> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
> >> > +                v->transpose_8x8   = transpose_8x8_8_c;
> >> > +                v->copyline_block  = copyline_block_8; break;
> >> >          case 2: v->transpose_block = transpose_block_16_c;
> >> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
> >> > +                v->transpose_8x8   = transpose_8x8_16_c;
> >> > +                v->copyline_block  = copyline_block_16; break;
> >> >          case 3: v->transpose_block = transpose_block_24_c;
> >> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
> >> > +                v->transpose_8x8   = transpose_8x8_24_c;
> >> > +                v->copyline_block  = copyline_block_24; break;
> >> >          case 4: v->transpose_block = transpose_block_32_c;
> >> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
> >> > +                v->transpose_8x8   = transpose_8x8_32_c;
> >> > +                v->copyline_block  = copyline_block_32; break;
> >> >          case 6: v->transpose_block = transpose_block_48_c;
> >> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
> >> > +                v->transpose_8x8   = transpose_8x8_48_c;
> >> > +                v->copyline_block  = copyline_block_48; break;
> >> >          case 8: v->transpose_block = transpose_block_64_c;
> >> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
> >> > +                v->transpose_8x8   = transpose_8x8_64_c;
> >> > +                v->copyline_block  = copyline_block_64; break;
> >> >          }
> >> >      }
> >> >
> >> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext *ctx,
> void
> >> > *arg, int jobnr,
> >> >          uint8_t *dst, *src;
> >> >          int dstlinesize, srclinesize;
> >> >          int x, y;
> >> > +        int dir = s->dir;
> >> >          TransVtable *v = &s->vtables[plane];
> >> > +
> >> > +        if (s->orientation > 0 && s->orientation < 5) {
> >> > +            int line_dir = 1;
> >> > +            dstlinesize = out->linesize[plane];
> >> > +            dst         = out->data[plane] + start * dstlinesize;
> >> > +            srclinesize = in->linesize[plane];
> >> > +            src         = in->data[plane] + start * srclinesize;
> >> > +
> >> > +            switch (s->orientation) {
> >> > +                case 2: // mirror horizontal
> >> > +                    line_dir     = -1;
> >> > +                    break;
> >> > +                case 3: // rotate 180
> >> > +                    dst          = out->data[plane] + (outh - start -
> >> 1) *
> >> > dstlinesize;
> >> > +                    line_dir     = -1;
> >> > +                    dstlinesize *= -1;
> >> > +                    break;
> >> > +                case 4: // mirror vertical
> >> > +                    dst          = out->data[plane] + (outh - start -
> >> 1) *
> >> > dstlinesize;
> >> > +                    dstlinesize *= -1;
> >> > +                    break;
> >> > +                default:
> >> > +                    break;
> >> > +            }
> >> > +            v->copyline_block(src, srclinesize, dst, dstlinesize,
> >> line_dir,
> >> > outw, end - start);
> >> > +        } else {
> >> > +            dstlinesize = out->linesize[plane];
> >> > +            dst         = out->data[plane] + start * dstlinesize;
> >> > +            src         = in->data[plane];
> >> > +            srclinesize = in->linesize[plane];
> >> > +            switch (s->orientation) {
> >> > +                case 5: // mirror horizontal and rotate 270 CW
> >> > +                    dir = 0; break;
> >> > +                case 6: // rotate 90 CW
> >> > +                    dir = 1; break;
> >> > +                case 7: // mirror horizontal and rotate 90 CW
> >> > +                    dir = 3; break;
> >> > +                case 8: // rotate 270 CW
> >> > +                    dir = 2; break;
> >> > +                default: break;
> >> > +            }
> >> >
> >> > -        dstlinesize = out->linesize[plane];
> >> > -        dst         = out->data[plane] + start * dstlinesize;
> >> > -        src         = in->data[plane];
> >> > -        srclinesize = in->linesize[plane];
> >> > -
> >> > -        if (s->dir & 1) {
> >> > -            src         += in->linesize[plane] * (inh - 1);
> >> > -            srclinesize *= -1;
> >> > -        }
> >> > +            if (dir & 1) {
> >> > +                src         += in->linesize[plane] * (inh - 1);
> >> > +                srclinesize *= -1;
> >> > +            }
> >> >
> >> > -        if (s->dir & 2) {
> >> > -            dst          = out->data[plane] + dstlinesize * (outh -
> >> start -
> >> > 1);
> >> > -            dstlinesize *= -1;
> >> > -        }
> >> > +            if (dir & 2) {
> >> > +                dst          = out->data[plane] + dstlinesize *
> (outh -
> >> > start - 1);
> >> > +                dstlinesize *= -1;
> >> > +            }
> >> >
> >> > -        for (y = start; y < end - 7; y += 8) {
> >> > -            for (x = 0; x < outw - 7; x += 8) {
> >> > -                v->transpose_8x8(src + x * srclinesize + y * pixstep,
> >> > -                                 srclinesize,
> >> > -                                 dst + (y - start) * dstlinesize + x
> *
> >> > pixstep,
> >> > -                                 dstlinesize);
> >> > +            for (y = start; y < end - 7; y += 8) {
> >> > +                for (x = 0; x < outw - 7; x += 8) {
> >> > +                    v->transpose_8x8(src + x * srclinesize + y *
> >> pixstep,
> >> > +                                    srclinesize,
> >> > +                                    dst + (y - start) * dstlinesize
> + x
> >> *
> >> > pixstep,
> >> > +                                    dstlinesize);
> >> > +                }
> >> > +                if (outw - x > 0 && end - y > 0)
> >> > +                    v->transpose_block(src + x * srclinesize + y *
> >> pixstep,
> >> > +                                    srclinesize,
> >> > +                                    dst + (y - start) * dstlinesize
> + x
> >> *
> >> > pixstep,
> >> > +                                    dstlinesize, outw - x, end - y);
> >> >              }
> >> > -            if (outw - x > 0 && end - y > 0)
> >> > -                v->transpose_block(src + x * srclinesize + y *
> pixstep,
> >> > -                                   srclinesize,
> >> > -                                   dst + (y - start) * dstlinesize +
> x
> >> > *
> >> > pixstep,
> >> > -                                   dstlinesize, outw - x, end - y);
> >> > -        }
> >> >
> >> > -        if (end - y > 0)
> >> > -            v->transpose_block(src + 0 * srclinesize + y * pixstep,
> >> > -                               srclinesize,
> >> > -                               dst + (y - start) * dstlinesize + 0 *
> >> > pixstep,
> >> > -                               dstlinesize, outw, end - y);
> >> > +            if (end - y > 0)
> >> > +                v->transpose_block(src + 0 * srclinesize + y *
> pixstep,
> >> > +                                srclinesize,
> >> > +                                dst + (y - start) * dstlinesize + 0 *
> >> > pixstep,
> >> > +                                dstlinesize, outw, end - y);
> >> > +        }
> >> >      }
> >> >
> >> >      return 0;
> >> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink,
> >> > AVFrame
> >> > *in)
> >> >      ThreadData td;
> >> >      AVFrame *out;
> >> >
> >> > -    if (s->passthrough)
> >> > +    if (s->passthrough || s->orientation == 1)
> >> >          return ff_filter_frame(outlink, in);
> >> >
> >> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> >> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink,
> >> AVFrame
> >> > *in)
> >> >      if (in->sample_aspect_ratio.num == 0) {
> >> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
> >> >      } else {
> >> > -        out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
> >> > -        out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
> >> > +        if (s->orientation > 0 && s->orientation < 5) {
> >> > +            out->sample_aspect_ratio.num =
> in->sample_aspect_ratio.num;
> >> > +            out->sample_aspect_ratio.den =
> in->sample_aspect_ratio.den;
> >> > +        } else {
> >> > +            out->sample_aspect_ratio.num =
> in->sample_aspect_ratio.den;
> >> > +            out->sample_aspect_ratio.den =
> in->sample_aspect_ratio.num;
> >> > +        }
> >> >      }
> >> >
> >> >      td.in = in, td.out = out;
> >> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[] = {
> >> >          { "clock",       "rotate clockwise",
> >> 0,
> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS,
> .unit
> >> =
> >> > "dir" },
> >> >          { "cclock",      "rotate counter-clockwise",
> >> 0,
> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS,
> .unit
> >> =
> >> > "dir" },
> >> >          { "clock_flip",  "rotate clockwise with vertical flip",
> >>  0,
> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS,
> .unit
> >> =
> >> > "dir" },
> >> > -
> >> > +
> >> >      { "passthrough", "do not apply transposition if the input matches
> >> the
> >> > specified geometry",
> >> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
> >> {.i64=TRANSPOSE_PT_TYPE_NONE},
> >> > 0, INT_MAX, FLAGS, "passthrough" },
> >> >          { "none",      "always apply transposition",   0,
> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN,
> INT_MAX,
> >> > FLAGS, "passthrough" },
> >> >          { "portrait",  "preserve portrait geometry",   0,
> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN,
> INT_MAX,
> >> > FLAGS, "passthrough" },
> >> >          { "landscape", "preserve landscape geometry",  0,
> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN,
> INT_MAX,
> >> > FLAGS, "passthrough" },
> >> > -
> >> > +    { "orientation", "set exif orientation", OFFSET(orientation),
> >> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
> >> >      { NULL }
> >> >  };
> >> >
> >> > --
> >> > 2.17.1
> >> >
> >> > _______________________________________________
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> > To unsubscribe, visit link above, or email
> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol May 27, 2019, 7:32 a.m. UTC | #7
On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
> On Sun, May 26, 2019 at 2:09 AM Paul B Mahol <onemda@gmail.com> wrote:
>
>> On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
>> > On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com> wrote:
>> >
>> >> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
>> >> > Add exif orientation support and expose an option.
>> >> > ---
>> >> >  libavfilter/vf_transpose.c | 258
>> +++++++++++++++++++++++++++++--------
>> >> >  1 file changed, 207 insertions(+), 51 deletions(-)
>> >> >
>> >> > diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c
>> >> > index dd54947bd9..4aebfb9ee4 100644
>> >> > --- a/libavfilter/vf_transpose.c
>> >> > +++ b/libavfilter/vf_transpose.c
>> >> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
>> >> >      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
>> >> >                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> >                              int w, int h);
>> >> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                           uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                           int line_dir, int w, int h);
>> >> >  } TransVtable;
>> >> >
>> >>
>> >> This is slow, the operations should be in one loop.
>> >>
>> >
>> > Thanks Paul for review.
>> > I see transpose is using " ctx->internal->execute(ctx, filter_slice,
>> > &td,
>> > NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
>> > which looks like leveraging multi-threading.
>> > If we do that in one loop, I guess it would be hard to use
>> multi-threading
>> > to accelerate ? But correct me if I am wrong.
>>
>> Transpose filter does one thing and that is transposing.
>>
>> If I'm not mistaken you are adding vertical and horizontal flipping?
>>
>
> Thanks for review.
> Yes, I am doing vflip, hflip and rotate180(vflip+hflip).
>
>
>> If you really need to do that, do it efficiently in one loop, by that I
>> mean
>> you do not do cascading operations at all.
>>
>
> Are you suggesting not use function pointer, and call the function directly
> like this:

Please do not invent things I never said.

I think I was very clear.

But to repeat it once more, it is unacceptable to transpose and do
vflip/hflip as
two separate operations. Instead everything should be done in single pass.

>             switch (s->pixsteps[plane]) {
>                 case 1:  copyline_block_8(src, srclinesize, dst,
> dstlinesize, line_dir, outw, end - start); break;
>                 case 2:  copyline_block_16(src, srclinesize, dst,
> dstlinesize, line_dir, outw, end - start); break;
>                 case 3:  copyline_block_24(src, srclinesize, dst,
> dstlinesize, line_dir, outw, end - start); break;
>                 case 4:  copyline_block_32(src, srclinesize, dst,
> dstlinesize, line_dir, outw, end - start); break;
>                 case 6:  copyline_block_48(src, srclinesize, dst,
> dstlinesize, line_dir, outw, end - start); break;
>                 case 8:  copyline_block_64(src, srclinesize, dst,
> dstlinesize, line_dir, outw, end - start); break;
>             }
>
> I did some perf comparsion, they are very close and no significant
> difference, about 9ms for one frame.
>
> Also I did some comparison between current hflip filter and this patch,
> they take almost the same time, about 9ms too on my machine and I see 13
> threads/jobs are used for filtering.
> I just use the simple command "ffmpeg  -i test.bmp -vf
> transpose=orientation=2  -y hello.jpg".
> Current vflip wins a lot and I see it is not copying frame at all, just
> like "orientation=1" or "passthrough" cases.
>
> I hope I understand you correctly, especially about "cascading operations".
> But correct me if I am wrong.
> Thanks for your time Paul.
>
> Best Regards,
> -Jun
>
>>
>> > Best Regards,
>> > -Jun
>> >
>> >
>> >> >  typedef struct TransContext {
>> >> > @@ -56,7 +59,7 @@ typedef struct TransContext {
>> >> >
>> >> >      int passthrough;    ///< PassthroughType, landscape passthrough
>> >> > mode
>> >> > enabled
>> >> >      int dir;            ///< TransposeDir
>> >> > -
>> >> > +    int orientation;    ///< Orientation
>> >> >      TransVtable vtables[4];
>> >> >  } TransContext;
>> >> >
>> >> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src,
>> >> ptrdiff_t
>> >> > src_linesize,
>> >> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8,
>> >> > 8);
>> >> >  }
>> >> >
>> >> > +
>> >> > +static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                             uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                             int line_dir, int w, int h)
>> >> > +{
>> >> > +    int x, y, i;
>> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> src_linesize) {
>> >> > +        for (x = 0; x < w; x ++) {
>> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> > +            dst[i] = src[x];
>> >> > +        }
>> >> > +    }
>> >> > +}
>> >> > +
>> >> > +static void copyline_block_16(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                              int line_dir, int w, int h)
>> >> > +{
>> >> > +    int x, y, i;
>> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> src_linesize) {
>> >> > +        for (x = 0; x < w; x ++) {
>> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src + 2*x));
>> >> > +        }
>> >> > +    }
>> >> > +}
>> >> > +
>> >> > +static void copyline_block_24(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                              int line_dir, int w, int h)
>> >> > +{
>> >> > +    int x, y, i;
>> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> src_linesize) {
>> >> > +        for (x = 0; x < w; x++) {
>> >> > +            int32_t v = AV_RB24(src + 3*x);
>> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> > +            AV_WB24(dst + 3*i, v);
>> >> > +        }
>> >> > +    }
>> >> > +}
>> >> > +
>> >> > +static void copyline_block_32(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                              int line_dir, int w, int h)
>> >> > +{
>> >> > +    int x, y, i;
>> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> src_linesize) {
>> >> > +        for (x = 0; x < w; x ++) {
>> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src + 4*x));
>> >> > +        }
>> >> > +    }
>> >> > +}
>> >> > +
>> >> > +static void copyline_block_48(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                              int line_dir, int w, int h)
>> >> > +{
>> >> > +    int x, y, i;
>> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> src_linesize) {
>> >> > +        for (x = 0; x < w; x++) {
>> >> > +            int64_t v = AV_RB48(src + 6*x);
>> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> > +            AV_WB48(dst + 6*i, v);
>> >> > +        }
>> >> > +    }
>> >> > +}
>> >> > +
>> >> > +static void copyline_block_64(uint8_t *src, ptrdiff_t src_linesize,
>> >> > +                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> > +                              int line_dir, int w, int h)
>> >> > +{
>> >> > +    int x, y, i;
>> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> src_linesize) {
>> >> > +        for (x = 0; x < w; x ++) {
>> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src + 8*x));
>> >> > +        }
>> >> > +    }
>> >> > +}
>> >> > +
>> >> > +static void set_outlink_width_height(AVFilterLink *inlink,
>> AVFilterLink
>> >> > *outlink, int transpose)
>> >> > +{
>> >> > +    if (transpose) {
>> >> > +        outlink->w = inlink->h;
>> >> > +        outlink->h = inlink->w;
>> >> > +
>> >> > +        if (inlink->sample_aspect_ratio.num)
>> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) {
>> 1, 1
>> >> },
>> >> > +
>> >> > inlink->sample_aspect_ratio);
>> >> > +        else
>> >> > +            outlink->sample_aspect_ratio =
>> inlink->sample_aspect_ratio;
>> >> > +    } else {
>> >> > +        outlink->w = inlink->w;
>> >> > +        outlink->h = inlink->h;
>> >> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> >> > +    }
>> >> > +}
>> >> > +
>> >> >  static int config_props_output(AVFilterLink *outlink)
>> >> >  {
>> >> >      AVFilterContext *ctx = outlink->src;
>> >> > @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink
>> >> *outlink)
>> >> >
>> >> >      av_assert0(desc_in->nb_components == desc_out->nb_components);
>> >> >
>> >> > -
>> >> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
>> >> > +
>> >> > +    if (s->orientation && s->orientation < 5) {
>> >> > +        outlink->h = inlink->h;
>> >> > +        outlink->w = inlink->w;
>> >> > +        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> >> > +    } else {
>> >> > +        outlink->w = inlink->h;
>> >> > +        outlink->h = inlink->w;
>> >> >
>> >> > -    outlink->w = inlink->h;
>> >> > -    outlink->h = inlink->w;
>> >> > -
>> >> > -    if (inlink->sample_aspect_ratio.num)
>> >> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1
>> },
>> >> > -
>> >> > inlink->sample_aspect_ratio);
>> >> > -    else
>> >> > -        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
>> >> > +        if (inlink->sample_aspect_ratio.num)
>> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) {
>> 1, 1
>> >> },
>> >> > +
>> >> > inlink->sample_aspect_ratio);
>> >> > +        else
>> >> > +            outlink->sample_aspect_ratio =
>> inlink->sample_aspect_ratio;
>> >> > +    }
>> >> >
>> >> >      for (int i = 0; i < 4; i++) {
>> >> >          TransVtable *v = &s->vtables[i];
>> >> >          switch (s->pixsteps[i]) {
>> >> >          case 1: v->transpose_block = transpose_block_8_c;
>> >> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
>> >> > +                v->transpose_8x8   = transpose_8x8_8_c;
>> >> > +                v->copyline_block  = copyline_block_8; break;
>> >> >          case 2: v->transpose_block = transpose_block_16_c;
>> >> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
>> >> > +                v->transpose_8x8   = transpose_8x8_16_c;
>> >> > +                v->copyline_block  = copyline_block_16; break;
>> >> >          case 3: v->transpose_block = transpose_block_24_c;
>> >> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
>> >> > +                v->transpose_8x8   = transpose_8x8_24_c;
>> >> > +                v->copyline_block  = copyline_block_24; break;
>> >> >          case 4: v->transpose_block = transpose_block_32_c;
>> >> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
>> >> > +                v->transpose_8x8   = transpose_8x8_32_c;
>> >> > +                v->copyline_block  = copyline_block_32; break;
>> >> >          case 6: v->transpose_block = transpose_block_48_c;
>> >> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
>> >> > +                v->transpose_8x8   = transpose_8x8_48_c;
>> >> > +                v->copyline_block  = copyline_block_48; break;
>> >> >          case 8: v->transpose_block = transpose_block_64_c;
>> >> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
>> >> > +                v->transpose_8x8   = transpose_8x8_64_c;
>> >> > +                v->copyline_block  = copyline_block_64; break;
>> >> >          }
>> >> >      }
>> >> >
>> >> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext *ctx,
>> void
>> >> > *arg, int jobnr,
>> >> >          uint8_t *dst, *src;
>> >> >          int dstlinesize, srclinesize;
>> >> >          int x, y;
>> >> > +        int dir = s->dir;
>> >> >          TransVtable *v = &s->vtables[plane];
>> >> > +
>> >> > +        if (s->orientation > 0 && s->orientation < 5) {
>> >> > +            int line_dir = 1;
>> >> > +            dstlinesize = out->linesize[plane];
>> >> > +            dst         = out->data[plane] + start * dstlinesize;
>> >> > +            srclinesize = in->linesize[plane];
>> >> > +            src         = in->data[plane] + start * srclinesize;
>> >> > +
>> >> > +            switch (s->orientation) {
>> >> > +                case 2: // mirror horizontal
>> >> > +                    line_dir     = -1;
>> >> > +                    break;
>> >> > +                case 3: // rotate 180
>> >> > +                    dst          = out->data[plane] + (outh - start
>> >> > -
>> >> 1) *
>> >> > dstlinesize;
>> >> > +                    line_dir     = -1;
>> >> > +                    dstlinesize *= -1;
>> >> > +                    break;
>> >> > +                case 4: // mirror vertical
>> >> > +                    dst          = out->data[plane] + (outh - start
>> >> > -
>> >> 1) *
>> >> > dstlinesize;
>> >> > +                    dstlinesize *= -1;
>> >> > +                    break;
>> >> > +                default:
>> >> > +                    break;
>> >> > +            }
>> >> > +            v->copyline_block(src, srclinesize, dst, dstlinesize,
>> >> line_dir,
>> >> > outw, end - start);
>> >> > +        } else {
>> >> > +            dstlinesize = out->linesize[plane];
>> >> > +            dst         = out->data[plane] + start * dstlinesize;
>> >> > +            src         = in->data[plane];
>> >> > +            srclinesize = in->linesize[plane];
>> >> > +            switch (s->orientation) {
>> >> > +                case 5: // mirror horizontal and rotate 270 CW
>> >> > +                    dir = 0; break;
>> >> > +                case 6: // rotate 90 CW
>> >> > +                    dir = 1; break;
>> >> > +                case 7: // mirror horizontal and rotate 90 CW
>> >> > +                    dir = 3; break;
>> >> > +                case 8: // rotate 270 CW
>> >> > +                    dir = 2; break;
>> >> > +                default: break;
>> >> > +            }
>> >> >
>> >> > -        dstlinesize = out->linesize[plane];
>> >> > -        dst         = out->data[plane] + start * dstlinesize;
>> >> > -        src         = in->data[plane];
>> >> > -        srclinesize = in->linesize[plane];
>> >> > -
>> >> > -        if (s->dir & 1) {
>> >> > -            src         += in->linesize[plane] * (inh - 1);
>> >> > -            srclinesize *= -1;
>> >> > -        }
>> >> > +            if (dir & 1) {
>> >> > +                src         += in->linesize[plane] * (inh - 1);
>> >> > +                srclinesize *= -1;
>> >> > +            }
>> >> >
>> >> > -        if (s->dir & 2) {
>> >> > -            dst          = out->data[plane] + dstlinesize * (outh -
>> >> start -
>> >> > 1);
>> >> > -            dstlinesize *= -1;
>> >> > -        }
>> >> > +            if (dir & 2) {
>> >> > +                dst          = out->data[plane] + dstlinesize *
>> (outh -
>> >> > start - 1);
>> >> > +                dstlinesize *= -1;
>> >> > +            }
>> >> >
>> >> > -        for (y = start; y < end - 7; y += 8) {
>> >> > -            for (x = 0; x < outw - 7; x += 8) {
>> >> > -                v->transpose_8x8(src + x * srclinesize + y *
>> >> > pixstep,
>> >> > -                                 srclinesize,
>> >> > -                                 dst + (y - start) * dstlinesize + x
>> *
>> >> > pixstep,
>> >> > -                                 dstlinesize);
>> >> > +            for (y = start; y < end - 7; y += 8) {
>> >> > +                for (x = 0; x < outw - 7; x += 8) {
>> >> > +                    v->transpose_8x8(src + x * srclinesize + y *
>> >> pixstep,
>> >> > +                                    srclinesize,
>> >> > +                                    dst + (y - start) * dstlinesize
>> + x
>> >> *
>> >> > pixstep,
>> >> > +                                    dstlinesize);
>> >> > +                }
>> >> > +                if (outw - x > 0 && end - y > 0)
>> >> > +                    v->transpose_block(src + x * srclinesize + y *
>> >> pixstep,
>> >> > +                                    srclinesize,
>> >> > +                                    dst + (y - start) * dstlinesize
>> + x
>> >> *
>> >> > pixstep,
>> >> > +                                    dstlinesize, outw - x, end - y);
>> >> >              }
>> >> > -            if (outw - x > 0 && end - y > 0)
>> >> > -                v->transpose_block(src + x * srclinesize + y *
>> pixstep,
>> >> > -                                   srclinesize,
>> >> > -                                   dst + (y - start) * dstlinesize +
>> x
>> >> > *
>> >> > pixstep,
>> >> > -                                   dstlinesize, outw - x, end - y);
>> >> > -        }
>> >> >
>> >> > -        if (end - y > 0)
>> >> > -            v->transpose_block(src + 0 * srclinesize + y * pixstep,
>> >> > -                               srclinesize,
>> >> > -                               dst + (y - start) * dstlinesize + 0 *
>> >> > pixstep,
>> >> > -                               dstlinesize, outw, end - y);
>> >> > +            if (end - y > 0)
>> >> > +                v->transpose_block(src + 0 * srclinesize + y *
>> pixstep,
>> >> > +                                srclinesize,
>> >> > +                                dst + (y - start) * dstlinesize + 0
>> >> > *
>> >> > pixstep,
>> >> > +                                dstlinesize, outw, end - y);
>> >> > +        }
>> >> >      }
>> >> >
>> >> >      return 0;
>> >> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink,
>> >> > AVFrame
>> >> > *in)
>> >> >      ThreadData td;
>> >> >      AVFrame *out;
>> >> >
>> >> > -    if (s->passthrough)
>> >> > +    if (s->passthrough || s->orientation == 1)
>> >> >          return ff_filter_frame(outlink, in);
>> >> >
>> >> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> >> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink,
>> >> AVFrame
>> >> > *in)
>> >> >      if (in->sample_aspect_ratio.num == 0) {
>> >> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
>> >> >      } else {
>> >> > -        out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
>> >> > -        out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
>> >> > +        if (s->orientation > 0 && s->orientation < 5) {
>> >> > +            out->sample_aspect_ratio.num =
>> in->sample_aspect_ratio.num;
>> >> > +            out->sample_aspect_ratio.den =
>> in->sample_aspect_ratio.den;
>> >> > +        } else {
>> >> > +            out->sample_aspect_ratio.num =
>> in->sample_aspect_ratio.den;
>> >> > +            out->sample_aspect_ratio.den =
>> in->sample_aspect_ratio.num;
>> >> > +        }
>> >> >      }
>> >> >
>> >> >      td.in = in, td.out = out;
>> >> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[] = {
>> >> >          { "clock",       "rotate clockwise",
>> >> 0,
>> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS,
>> .unit
>> >> =
>> >> > "dir" },
>> >> >          { "cclock",      "rotate counter-clockwise",
>> >> 0,
>> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS,
>> .unit
>> >> =
>> >> > "dir" },
>> >> >          { "clock_flip",  "rotate clockwise with vertical flip",
>> >>  0,
>> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS,
>> .unit
>> >> =
>> >> > "dir" },
>> >> > -
>> >> > +
>> >> >      { "passthrough", "do not apply transposition if the input
>> >> > matches
>> >> the
>> >> > specified geometry",
>> >> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
>> >> {.i64=TRANSPOSE_PT_TYPE_NONE},
>> >> > 0, INT_MAX, FLAGS, "passthrough" },
>> >> >          { "none",      "always apply transposition",   0,
>> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN,
>> INT_MAX,
>> >> > FLAGS, "passthrough" },
>> >> >          { "portrait",  "preserve portrait geometry",   0,
>> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN,
>> INT_MAX,
>> >> > FLAGS, "passthrough" },
>> >> >          { "landscape", "preserve landscape geometry",  0,
>> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN,
>> INT_MAX,
>> >> > FLAGS, "passthrough" },
>> >> > -
>> >> > +    { "orientation", "set exif orientation", OFFSET(orientation),
>> >> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
>> >> >      { NULL }
>> >> >  };
>> >> >
>> >> > --
>> >> > 2.17.1
>> >> >
>> >> > _______________________________________________
>> >> > ffmpeg-devel mailing list
>> >> > ffmpeg-devel@ffmpeg.org
>> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >
>> >> > To unsubscribe, visit link above, or email
>> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> _______________________________________________
>> >> ffmpeg-devel mailing list
>> >> ffmpeg-devel@ffmpeg.org
>> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >>
>> >> To unsubscribe, visit link above, or email
>> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> > To unsubscribe, visit link above, or email
>> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jun Li May 27, 2019, 8:12 a.m. UTC | #8
On Mon, May 27, 2019 at 12:32 AM Paul B Mahol <onemda@gmail.com> wrote:

> On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
> > On Sun, May 26, 2019 at 2:09 AM Paul B Mahol <onemda@gmail.com> wrote:
> >
> >> On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
> >> > On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com>
> wrote:
> >> >
> >> >> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
> >> >> > Add exif orientation support and expose an option.
> >> >> > ---
> >> >> >  libavfilter/vf_transpose.c | 258
> >> +++++++++++++++++++++++++++++--------
> >> >> >  1 file changed, 207 insertions(+), 51 deletions(-)
> >> >> >
> >> >> > diff --git a/libavfilter/vf_transpose.c
> b/libavfilter/vf_transpose.c
> >> >> > index dd54947bd9..4aebfb9ee4 100644
> >> >> > --- a/libavfilter/vf_transpose.c
> >> >> > +++ b/libavfilter/vf_transpose.c
> >> >> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
> >> >> >      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
> >> >> >                              uint8_t *dst, ptrdiff_t dst_linesize,
> >> >> >                              int w, int h);
> >> >> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
> >> >> > +                           uint8_t *dst, ptrdiff_t dst_linesize,
> >> >> > +                           int line_dir, int w, int h);
> >> >> >  } TransVtable;
> >> >> >
> >> >>
> >> >> This is slow, the operations should be in one loop.
> >> >>
> >> >
> >> > Thanks Paul for review.
> >> > I see transpose is using " ctx->internal->execute(ctx, filter_slice,
> >> > &td,
> >> > NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
> >> > which looks like leveraging multi-threading.
> >> > If we do that in one loop, I guess it would be hard to use
> >> multi-threading
> >> > to accelerate ? But correct me if I am wrong.
> >>
> >> Transpose filter does one thing and that is transposing.
> >>
> >> If I'm not mistaken you are adding vertical and horizontal flipping?
> >>
> >
> > Thanks for review.
> > Yes, I am doing vflip, hflip and rotate180(vflip+hflip).
> >
> >
> >> If you really need to do that, do it efficiently in one loop, by that I
> >> mean
> >> you do not do cascading operations at all.
> >>
> >
> > Are you suggesting not use function pointer, and call the function
> directly
> > like this:
>
> Please do not invent things I never said.

I am sorry that I wrongly understand your question. I am not a native
speaker, obviously here we have different understanding about the same
sentence.

I think I was very clear.

But to repeat it once more, it is unacceptable to transpose and do
> vflip/hflip as
> two separate operations. Instead everything should be done in single pass.


It may be stupid but I really have trouble understanding "single pass"
here. The new added function will be called only once per thread. And won't
be used together with current transpose method, they are "either-or" case.

Or are you suggesting moving the flip operation into existing function,
rather than create a new one ?

Best Regards,
Jun

>             switch (s->pixsteps[plane]) {
> >                 case 1:  copyline_block_8(src, srclinesize, dst,
> > dstlinesize, line_dir, outw, end - start); break;
> >                 case 2:  copyline_block_16(src, srclinesize, dst,
> > dstlinesize, line_dir, outw, end - start); break;
> >                 case 3:  copyline_block_24(src, srclinesize, dst,
> > dstlinesize, line_dir, outw, end - start); break;
> >                 case 4:  copyline_block_32(src, srclinesize, dst,
> > dstlinesize, line_dir, outw, end - start); break;
> >                 case 6:  copyline_block_48(src, srclinesize, dst,
> > dstlinesize, line_dir, outw, end - start); break;
> >                 case 8:  copyline_block_64(src, srclinesize, dst,
> > dstlinesize, line_dir, outw, end - start); break;
> >             }
> >
> > I did some perf comparsion, they are very close and no significant
> > difference, about 9ms for one frame.
> >
> > Also I did some comparison between current hflip filter and this patch,
> > they take almost the same time, about 9ms too on my machine and I see 13
> > threads/jobs are used for filtering.
> > I just use the simple command "ffmpeg  -i test.bmp -vf
> > transpose=orientation=2  -y hello.jpg".
> > Current vflip wins a lot and I see it is not copying frame at all, just
> > like "orientation=1" or "passthrough" cases.
> >
> > I hope I understand you correctly, especially about "cascading
> operations".
> > But correct me if I am wrong.
> > Thanks for your time Paul.
> >
> > Best Regards,
> > -Jun
> >
> >>
> >> > Best Regards,
> >> > -Jun
> >> >
> >> >
> >> >> >  typedef struct TransContext {
> >> >> > @@ -56,7 +59,7 @@ typedef struct TransContext {
> >> >> >
> >> >> >      int passthrough;    ///< PassthroughType, landscape
> passthrough
> >> >> > mode
> >> >> > enabled
> >> >> >      int dir;            ///< TransposeDir
> >> >> > -
> >> >> > +    int orientation;    ///< Orientation
> >> >> >      TransVtable vtables[4];
> >> >> >  } TransContext;
> >> >> >
> >> >> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src,
> >> >> ptrdiff_t
> >> >> > src_linesize,
> >> >> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8,
> >> >> > 8);
> >> >> >  }
> >> >> >
> >> >> > +
> >> >> > +static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
> >> >> > +                             uint8_t *dst, ptrdiff_t dst_linesize,
> >> >> > +                             int line_dir, int w, int h)
> >> >> > +{
> >> >> > +    int x, y, i;
> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> src_linesize) {
> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> > +            dst[i] = src[x];
> >> >> > +        }
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> > +static void copyline_block_16(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> > +                              uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> > +                              int line_dir, int w, int h)
> >> >> > +{
> >> >> > +    int x, y, i;
> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> src_linesize) {
> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src +
> 2*x));
> >> >> > +        }
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> > +static void copyline_block_24(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> > +                              uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> > +                              int line_dir, int w, int h)
> >> >> > +{
> >> >> > +    int x, y, i;
> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> src_linesize) {
> >> >> > +        for (x = 0; x < w; x++) {
> >> >> > +            int32_t v = AV_RB24(src + 3*x);
> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> > +            AV_WB24(dst + 3*i, v);
> >> >> > +        }
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> > +static void copyline_block_32(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> > +                              uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> > +                              int line_dir, int w, int h)
> >> >> > +{
> >> >> > +    int x, y, i;
> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> src_linesize) {
> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src +
> 4*x));
> >> >> > +        }
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> > +static void copyline_block_48(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> > +                              uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> > +                              int line_dir, int w, int h)
> >> >> > +{
> >> >> > +    int x, y, i;
> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> src_linesize) {
> >> >> > +        for (x = 0; x < w; x++) {
> >> >> > +            int64_t v = AV_RB48(src + 6*x);
> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> > +            AV_WB48(dst + 6*i, v);
> >> >> > +        }
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> > +static void copyline_block_64(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> > +                              uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> > +                              int line_dir, int w, int h)
> >> >> > +{
> >> >> > +    int x, y, i;
> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> src_linesize) {
> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src +
> 8*x));
> >> >> > +        }
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> > +static void set_outlink_width_height(AVFilterLink *inlink,
> >> AVFilterLink
> >> >> > *outlink, int transpose)
> >> >> > +{
> >> >> > +    if (transpose) {
> >> >> > +        outlink->w = inlink->h;
> >> >> > +        outlink->h = inlink->w;
> >> >> > +
> >> >> > +        if (inlink->sample_aspect_ratio.num)
> >> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) {
> >> 1, 1
> >> >> },
> >> >> > +
> >> >> > inlink->sample_aspect_ratio);
> >> >> > +        else
> >> >> > +            outlink->sample_aspect_ratio =
> >> inlink->sample_aspect_ratio;
> >> >> > +    } else {
> >> >> > +        outlink->w = inlink->w;
> >> >> > +        outlink->h = inlink->h;
> >> >> > +        outlink->sample_aspect_ratio =
> inlink->sample_aspect_ratio;
> >> >> > +    }
> >> >> > +}
> >> >> > +
> >> >> >  static int config_props_output(AVFilterLink *outlink)
> >> >> >  {
> >> >> >      AVFilterContext *ctx = outlink->src;
> >> >> > @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink
> >> >> *outlink)
> >> >> >
> >> >> >      av_assert0(desc_in->nb_components == desc_out->nb_components);
> >> >> >
> >> >> > -
> >> >> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
> >> >> > +
> >> >> > +    if (s->orientation && s->orientation < 5) {
> >> >> > +        outlink->h = inlink->h;
> >> >> > +        outlink->w = inlink->w;
> >> >> > +        outlink->sample_aspect_ratio =
> inlink->sample_aspect_ratio;
> >> >> > +    } else {
> >> >> > +        outlink->w = inlink->h;
> >> >> > +        outlink->h = inlink->w;
> >> >> >
> >> >> > -    outlink->w = inlink->h;
> >> >> > -    outlink->h = inlink->w;
> >> >> > -
> >> >> > -    if (inlink->sample_aspect_ratio.num)
> >> >> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1,
> 1
> >> },
> >> >> > -
> >> >> > inlink->sample_aspect_ratio);
> >> >> > -    else
> >> >> > -        outlink->sample_aspect_ratio =
> inlink->sample_aspect_ratio;
> >> >> > +        if (inlink->sample_aspect_ratio.num)
> >> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational) {
> >> 1, 1
> >> >> },
> >> >> > +
> >> >> > inlink->sample_aspect_ratio);
> >> >> > +        else
> >> >> > +            outlink->sample_aspect_ratio =
> >> inlink->sample_aspect_ratio;
> >> >> > +    }
> >> >> >
> >> >> >      for (int i = 0; i < 4; i++) {
> >> >> >          TransVtable *v = &s->vtables[i];
> >> >> >          switch (s->pixsteps[i]) {
> >> >> >          case 1: v->transpose_block = transpose_block_8_c;
> >> >> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
> >> >> > +                v->transpose_8x8   = transpose_8x8_8_c;
> >> >> > +                v->copyline_block  = copyline_block_8; break;
> >> >> >          case 2: v->transpose_block = transpose_block_16_c;
> >> >> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
> >> >> > +                v->transpose_8x8   = transpose_8x8_16_c;
> >> >> > +                v->copyline_block  = copyline_block_16; break;
> >> >> >          case 3: v->transpose_block = transpose_block_24_c;
> >> >> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
> >> >> > +                v->transpose_8x8   = transpose_8x8_24_c;
> >> >> > +                v->copyline_block  = copyline_block_24; break;
> >> >> >          case 4: v->transpose_block = transpose_block_32_c;
> >> >> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
> >> >> > +                v->transpose_8x8   = transpose_8x8_32_c;
> >> >> > +                v->copyline_block  = copyline_block_32; break;
> >> >> >          case 6: v->transpose_block = transpose_block_48_c;
> >> >> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
> >> >> > +                v->transpose_8x8   = transpose_8x8_48_c;
> >> >> > +                v->copyline_block  = copyline_block_48; break;
> >> >> >          case 8: v->transpose_block = transpose_block_64_c;
> >> >> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
> >> >> > +                v->transpose_8x8   = transpose_8x8_64_c;
> >> >> > +                v->copyline_block  = copyline_block_64; break;
> >> >> >          }
> >> >> >      }
> >> >> >
> >> >> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext *ctx,
> >> void
> >> >> > *arg, int jobnr,
> >> >> >          uint8_t *dst, *src;
> >> >> >          int dstlinesize, srclinesize;
> >> >> >          int x, y;
> >> >> > +        int dir = s->dir;
> >> >> >          TransVtable *v = &s->vtables[plane];
> >> >> > +
> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
> >> >> > +            int line_dir = 1;
> >> >> > +            dstlinesize = out->linesize[plane];
> >> >> > +            dst         = out->data[plane] + start * dstlinesize;
> >> >> > +            srclinesize = in->linesize[plane];
> >> >> > +            src         = in->data[plane] + start * srclinesize;
> >> >> > +
> >> >> > +            switch (s->orientation) {
> >> >> > +                case 2: // mirror horizontal
> >> >> > +                    line_dir     = -1;
> >> >> > +                    break;
> >> >> > +                case 3: // rotate 180
> >> >> > +                    dst          = out->data[plane] + (outh -
> start
> >> >> > -
> >> >> 1) *
> >> >> > dstlinesize;
> >> >> > +                    line_dir     = -1;
> >> >> > +                    dstlinesize *= -1;
> >> >> > +                    break;
> >> >> > +                case 4: // mirror vertical
> >> >> > +                    dst          = out->data[plane] + (outh -
> start
> >> >> > -
> >> >> 1) *
> >> >> > dstlinesize;
> >> >> > +                    dstlinesize *= -1;
> >> >> > +                    break;
> >> >> > +                default:
> >> >> > +                    break;
> >> >> > +            }
> >> >> > +            v->copyline_block(src, srclinesize, dst, dstlinesize,
> >> >> line_dir,
> >> >> > outw, end - start);
> >> >> > +        } else {
> >> >> > +            dstlinesize = out->linesize[plane];
> >> >> > +            dst         = out->data[plane] + start * dstlinesize;
> >> >> > +            src         = in->data[plane];
> >> >> > +            srclinesize = in->linesize[plane];
> >> >> > +            switch (s->orientation) {
> >> >> > +                case 5: // mirror horizontal and rotate 270 CW
> >> >> > +                    dir = 0; break;
> >> >> > +                case 6: // rotate 90 CW
> >> >> > +                    dir = 1; break;
> >> >> > +                case 7: // mirror horizontal and rotate 90 CW
> >> >> > +                    dir = 3; break;
> >> >> > +                case 8: // rotate 270 CW
> >> >> > +                    dir = 2; break;
> >> >> > +                default: break;
> >> >> > +            }
> >> >> >
> >> >> > -        dstlinesize = out->linesize[plane];
> >> >> > -        dst         = out->data[plane] + start * dstlinesize;
> >> >> > -        src         = in->data[plane];
> >> >> > -        srclinesize = in->linesize[plane];
> >> >> > -
> >> >> > -        if (s->dir & 1) {
> >> >> > -            src         += in->linesize[plane] * (inh - 1);
> >> >> > -            srclinesize *= -1;
> >> >> > -        }
> >> >> > +            if (dir & 1) {
> >> >> > +                src         += in->linesize[plane] * (inh - 1);
> >> >> > +                srclinesize *= -1;
> >> >> > +            }
> >> >> >
> >> >> > -        if (s->dir & 2) {
> >> >> > -            dst          = out->data[plane] + dstlinesize * (outh
> -
> >> >> start -
> >> >> > 1);
> >> >> > -            dstlinesize *= -1;
> >> >> > -        }
> >> >> > +            if (dir & 2) {
> >> >> > +                dst          = out->data[plane] + dstlinesize *
> >> (outh -
> >> >> > start - 1);
> >> >> > +                dstlinesize *= -1;
> >> >> > +            }
> >> >> >
> >> >> > -        for (y = start; y < end - 7; y += 8) {
> >> >> > -            for (x = 0; x < outw - 7; x += 8) {
> >> >> > -                v->transpose_8x8(src + x * srclinesize + y *
> >> >> > pixstep,
> >> >> > -                                 srclinesize,
> >> >> > -                                 dst + (y - start) * dstlinesize
> + x
> >> *
> >> >> > pixstep,
> >> >> > -                                 dstlinesize);
> >> >> > +            for (y = start; y < end - 7; y += 8) {
> >> >> > +                for (x = 0; x < outw - 7; x += 8) {
> >> >> > +                    v->transpose_8x8(src + x * srclinesize + y *
> >> >> pixstep,
> >> >> > +                                    srclinesize,
> >> >> > +                                    dst + (y - start) *
> dstlinesize
> >> + x
> >> >> *
> >> >> > pixstep,
> >> >> > +                                    dstlinesize);
> >> >> > +                }
> >> >> > +                if (outw - x > 0 && end - y > 0)
> >> >> > +                    v->transpose_block(src + x * srclinesize + y *
> >> >> pixstep,
> >> >> > +                                    srclinesize,
> >> >> > +                                    dst + (y - start) *
> dstlinesize
> >> + x
> >> >> *
> >> >> > pixstep,
> >> >> > +                                    dstlinesize, outw - x, end -
> y);
> >> >> >              }
> >> >> > -            if (outw - x > 0 && end - y > 0)
> >> >> > -                v->transpose_block(src + x * srclinesize + y *
> >> pixstep,
> >> >> > -                                   srclinesize,
> >> >> > -                                   dst + (y - start) *
> dstlinesize +
> >> x
> >> >> > *
> >> >> > pixstep,
> >> >> > -                                   dstlinesize, outw - x, end -
> y);
> >> >> > -        }
> >> >> >
> >> >> > -        if (end - y > 0)
> >> >> > -            v->transpose_block(src + 0 * srclinesize + y *
> pixstep,
> >> >> > -                               srclinesize,
> >> >> > -                               dst + (y - start) * dstlinesize +
> 0 *
> >> >> > pixstep,
> >> >> > -                               dstlinesize, outw, end - y);
> >> >> > +            if (end - y > 0)
> >> >> > +                v->transpose_block(src + 0 * srclinesize + y *
> >> pixstep,
> >> >> > +                                srclinesize,
> >> >> > +                                dst + (y - start) * dstlinesize +
> 0
> >> >> > *
> >> >> > pixstep,
> >> >> > +                                dstlinesize, outw, end - y);
> >> >> > +        }
> >> >> >      }
> >> >> >
> >> >> >      return 0;
> >> >> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink,
> >> >> > AVFrame
> >> >> > *in)
> >> >> >      ThreadData td;
> >> >> >      AVFrame *out;
> >> >> >
> >> >> > -    if (s->passthrough)
> >> >> > +    if (s->passthrough || s->orientation == 1)
> >> >> >          return ff_filter_frame(outlink, in);
> >> >> >
> >> >> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> >> >> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink,
> >> >> AVFrame
> >> >> > *in)
> >> >> >      if (in->sample_aspect_ratio.num == 0) {
> >> >> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
> >> >> >      } else {
> >> >> > -        out->sample_aspect_ratio.num =
> in->sample_aspect_ratio.den;
> >> >> > -        out->sample_aspect_ratio.den =
> in->sample_aspect_ratio.num;
> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
> >> >> > +            out->sample_aspect_ratio.num =
> >> in->sample_aspect_ratio.num;
> >> >> > +            out->sample_aspect_ratio.den =
> >> in->sample_aspect_ratio.den;
> >> >> > +        } else {
> >> >> > +            out->sample_aspect_ratio.num =
> >> in->sample_aspect_ratio.den;
> >> >> > +            out->sample_aspect_ratio.den =
> >> in->sample_aspect_ratio.num;
> >> >> > +        }
> >> >> >      }
> >> >> >
> >> >> >      td.in = in, td.out = out;
> >> >> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[] = {
> >> >> >          { "clock",       "rotate clockwise",
> >> >> 0,
> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS,
> >> .unit
> >> >> =
> >> >> > "dir" },
> >> >> >          { "cclock",      "rotate counter-clockwise",
> >> >> 0,
> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS,
> >> .unit
> >> >> =
> >> >> > "dir" },
> >> >> >          { "clock_flip",  "rotate clockwise with vertical flip",
> >> >>  0,
> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS,
> >> .unit
> >> >> =
> >> >> > "dir" },
> >> >> > -
> >> >> > +
> >> >> >      { "passthrough", "do not apply transposition if the input
> >> >> > matches
> >> >> the
> >> >> > specified geometry",
> >> >> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
> >> >> {.i64=TRANSPOSE_PT_TYPE_NONE},
> >> >> > 0, INT_MAX, FLAGS, "passthrough" },
> >> >> >          { "none",      "always apply transposition",   0,
> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN,
> >> INT_MAX,
> >> >> > FLAGS, "passthrough" },
> >> >> >          { "portrait",  "preserve portrait geometry",   0,
> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN,
> >> INT_MAX,
> >> >> > FLAGS, "passthrough" },
> >> >> >          { "landscape", "preserve landscape geometry",  0,
> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN,
> >> INT_MAX,
> >> >> > FLAGS, "passthrough" },
> >> >> > -
> >> >> > +    { "orientation", "set exif orientation", OFFSET(orientation),
> >> >> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
> >> >> >      { NULL }
> >> >> >  };
> >> >> >
> >> >> > --
> >> >> > 2.17.1
> >> >> >
> >> >> > _______________________________________________
> >> >> > ffmpeg-devel mailing list
> >> >> > ffmpeg-devel@ffmpeg.org
> >> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >> >
> >> >> > To unsubscribe, visit link above, or email
> >> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> >> _______________________________________________
> >> >> ffmpeg-devel mailing list
> >> >> ffmpeg-devel@ffmpeg.org
> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >>
> >> >> To unsubscribe, visit link above, or email
> >> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> > _______________________________________________
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> > To unsubscribe, visit link above, or email
> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol May 27, 2019, 8:39 a.m. UTC | #9
On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
> On Mon, May 27, 2019 at 12:32 AM Paul B Mahol <onemda@gmail.com> wrote:
>
>> On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
>> > On Sun, May 26, 2019 at 2:09 AM Paul B Mahol <onemda@gmail.com> wrote:
>> >
>> >> On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
>> >> > On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com>
>> wrote:
>> >> >
>> >> >> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
>> >> >> > Add exif orientation support and expose an option.
>> >> >> > ---
>> >> >> >  libavfilter/vf_transpose.c | 258
>> >> +++++++++++++++++++++++++++++--------
>> >> >> >  1 file changed, 207 insertions(+), 51 deletions(-)
>> >> >> >
>> >> >> > diff --git a/libavfilter/vf_transpose.c
>> b/libavfilter/vf_transpose.c
>> >> >> > index dd54947bd9..4aebfb9ee4 100644
>> >> >> > --- a/libavfilter/vf_transpose.c
>> >> >> > +++ b/libavfilter/vf_transpose.c
>> >> >> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
>> >> >> >      void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
>> >> >> >                              uint8_t *dst, ptrdiff_t dst_linesize,
>> >> >> >                              int w, int h);
>> >> >> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
>> >> >> > +                           uint8_t *dst, ptrdiff_t dst_linesize,
>> >> >> > +                           int line_dir, int w, int h);
>> >> >> >  } TransVtable;
>> >> >> >
>> >> >>
>> >> >> This is slow, the operations should be in one loop.
>> >> >>
>> >> >
>> >> > Thanks Paul for review.
>> >> > I see transpose is using " ctx->internal->execute(ctx, filter_slice,
>> >> > &td,
>> >> > NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
>> >> > which looks like leveraging multi-threading.
>> >> > If we do that in one loop, I guess it would be hard to use
>> >> multi-threading
>> >> > to accelerate ? But correct me if I am wrong.
>> >>
>> >> Transpose filter does one thing and that is transposing.
>> >>
>> >> If I'm not mistaken you are adding vertical and horizontal flipping?
>> >>
>> >
>> > Thanks for review.
>> > Yes, I am doing vflip, hflip and rotate180(vflip+hflip).
>> >
>> >
>> >> If you really need to do that, do it efficiently in one loop, by that I
>> >> mean
>> >> you do not do cascading operations at all.
>> >>
>> >
>> > Are you suggesting not use function pointer, and call the function
>> directly
>> > like this:
>>
>> Please do not invent things I never said.
>
> I am sorry that I wrongly understand your question. I am not a native
> speaker, obviously here we have different understanding about the same
> sentence.
>
> I think I was very clear.
>
> But to repeat it once more, it is unacceptable to transpose and do
>> vflip/hflip as
>> two separate operations. Instead everything should be done in single pass.
>
>
> It may be stupid but I really have trouble understanding "single pass"
> here. The new added function will be called only once per thread. And won't
> be used together with current transpose method, they are "either-or" case.

If they are not used in combination with transpose operation, then I fail to see
point to addition of this slow path when much faster already exist in way
like calling vflip/hflip filter directly.
So I really do not see point in re-implementing something in slower fashion.

>
> Or are you suggesting moving the flip operation into existing function,
> rather than create a new one ?


>
> Best Regards,
> Jun
>
>>             switch (s->pixsteps[plane]) {
>> >                 case 1:  copyline_block_8(src, srclinesize, dst,
>> > dstlinesize, line_dir, outw, end - start); break;
>> >                 case 2:  copyline_block_16(src, srclinesize, dst,
>> > dstlinesize, line_dir, outw, end - start); break;
>> >                 case 3:  copyline_block_24(src, srclinesize, dst,
>> > dstlinesize, line_dir, outw, end - start); break;
>> >                 case 4:  copyline_block_32(src, srclinesize, dst,
>> > dstlinesize, line_dir, outw, end - start); break;
>> >                 case 6:  copyline_block_48(src, srclinesize, dst,
>> > dstlinesize, line_dir, outw, end - start); break;
>> >                 case 8:  copyline_block_64(src, srclinesize, dst,
>> > dstlinesize, line_dir, outw, end - start); break;
>> >             }
>> >
>> > I did some perf comparsion, they are very close and no significant
>> > difference, about 9ms for one frame.
>> >
>> > Also I did some comparison between current hflip filter and this patch,
>> > they take almost the same time, about 9ms too on my machine and I see 13
>> > threads/jobs are used for filtering.
>> > I just use the simple command "ffmpeg  -i test.bmp -vf
>> > transpose=orientation=2  -y hello.jpg".
>> > Current vflip wins a lot and I see it is not copying frame at all, just
>> > like "orientation=1" or "passthrough" cases.
>> >
>> > I hope I understand you correctly, especially about "cascading
>> operations".
>> > But correct me if I am wrong.
>> > Thanks for your time Paul.
>> >
>> > Best Regards,
>> > -Jun
>> >
>> >>
>> >> > Best Regards,
>> >> > -Jun
>> >> >
>> >> >
>> >> >> >  typedef struct TransContext {
>> >> >> > @@ -56,7 +59,7 @@ typedef struct TransContext {
>> >> >> >
>> >> >> >      int passthrough;    ///< PassthroughType, landscape
>> passthrough
>> >> >> > mode
>> >> >> > enabled
>> >> >> >      int dir;            ///< TransposeDir
>> >> >> > -
>> >> >> > +    int orientation;    ///< Orientation
>> >> >> >      TransVtable vtables[4];
>> >> >> >  } TransContext;
>> >> >> >
>> >> >> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t *src,
>> >> >> ptrdiff_t
>> >> >> > src_linesize,
>> >> >> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8,
>> >> >> > 8);
>> >> >> >  }
>> >> >> >
>> >> >> > +
>> >> >> > +static void copyline_block_8(uint8_t *src, ptrdiff_t
>> >> >> > src_linesize,
>> >> >> > +                             uint8_t *dst, ptrdiff_t
>> >> >> > dst_linesize,
>> >> >> > +                             int line_dir, int w, int h)
>> >> >> > +{
>> >> >> > +    int x, y, i;
>> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> src_linesize) {
>> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> > +            dst[i] = src[x];
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> > +static void copyline_block_16(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> > +                              uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> > +                              int line_dir, int w, int h)
>> >> >> > +{
>> >> >> > +    int x, y, i;
>> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> src_linesize) {
>> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src +
>> 2*x));
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> > +static void copyline_block_24(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> > +                              uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> > +                              int line_dir, int w, int h)
>> >> >> > +{
>> >> >> > +    int x, y, i;
>> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> src_linesize) {
>> >> >> > +        for (x = 0; x < w; x++) {
>> >> >> > +            int32_t v = AV_RB24(src + 3*x);
>> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> > +            AV_WB24(dst + 3*i, v);
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> > +static void copyline_block_32(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> > +                              uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> > +                              int line_dir, int w, int h)
>> >> >> > +{
>> >> >> > +    int x, y, i;
>> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> src_linesize) {
>> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src +
>> 4*x));
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> > +static void copyline_block_48(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> > +                              uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> > +                              int line_dir, int w, int h)
>> >> >> > +{
>> >> >> > +    int x, y, i;
>> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> src_linesize) {
>> >> >> > +        for (x = 0; x < w; x++) {
>> >> >> > +            int64_t v = AV_RB48(src + 6*x);
>> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> > +            AV_WB48(dst + 6*i, v);
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> > +static void copyline_block_64(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> > +                              uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> > +                              int line_dir, int w, int h)
>> >> >> > +{
>> >> >> > +    int x, y, i;
>> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> src_linesize) {
>> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src +
>> 8*x));
>> >> >> > +        }
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> > +static void set_outlink_width_height(AVFilterLink *inlink,
>> >> AVFilterLink
>> >> >> > *outlink, int transpose)
>> >> >> > +{
>> >> >> > +    if (transpose) {
>> >> >> > +        outlink->w = inlink->h;
>> >> >> > +        outlink->h = inlink->w;
>> >> >> > +
>> >> >> > +        if (inlink->sample_aspect_ratio.num)
>> >> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational)
>> >> >> > {
>> >> 1, 1
>> >> >> },
>> >> >> > +
>> >> >> > inlink->sample_aspect_ratio);
>> >> >> > +        else
>> >> >> > +            outlink->sample_aspect_ratio =
>> >> inlink->sample_aspect_ratio;
>> >> >> > +    } else {
>> >> >> > +        outlink->w = inlink->w;
>> >> >> > +        outlink->h = inlink->h;
>> >> >> > +        outlink->sample_aspect_ratio =
>> inlink->sample_aspect_ratio;
>> >> >> > +    }
>> >> >> > +}
>> >> >> > +
>> >> >> >  static int config_props_output(AVFilterLink *outlink)
>> >> >> >  {
>> >> >> >      AVFilterContext *ctx = outlink->src;
>> >> >> > @@ -213,33 +315,44 @@ static int config_props_output(AVFilterLink
>> >> >> *outlink)
>> >> >> >
>> >> >> >      av_assert0(desc_in->nb_components ==
>> >> >> > desc_out->nb_components);
>> >> >> >
>> >> >> > -
>> >> >> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
>> >> >> > +
>> >> >> > +    if (s->orientation && s->orientation < 5) {
>> >> >> > +        outlink->h = inlink->h;
>> >> >> > +        outlink->w = inlink->w;
>> >> >> > +        outlink->sample_aspect_ratio =
>> inlink->sample_aspect_ratio;
>> >> >> > +    } else {
>> >> >> > +        outlink->w = inlink->h;
>> >> >> > +        outlink->h = inlink->w;
>> >> >> >
>> >> >> > -    outlink->w = inlink->h;
>> >> >> > -    outlink->h = inlink->w;
>> >> >> > -
>> >> >> > -    if (inlink->sample_aspect_ratio.num)
>> >> >> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1,
>> 1
>> >> },
>> >> >> > -
>> >> >> > inlink->sample_aspect_ratio);
>> >> >> > -    else
>> >> >> > -        outlink->sample_aspect_ratio =
>> inlink->sample_aspect_ratio;
>> >> >> > +        if (inlink->sample_aspect_ratio.num)
>> >> >> > +            outlink->sample_aspect_ratio = av_div_q((AVRational)
>> >> >> > {
>> >> 1, 1
>> >> >> },
>> >> >> > +
>> >> >> > inlink->sample_aspect_ratio);
>> >> >> > +        else
>> >> >> > +            outlink->sample_aspect_ratio =
>> >> inlink->sample_aspect_ratio;
>> >> >> > +    }
>> >> >> >
>> >> >> >      for (int i = 0; i < 4; i++) {
>> >> >> >          TransVtable *v = &s->vtables[i];
>> >> >> >          switch (s->pixsteps[i]) {
>> >> >> >          case 1: v->transpose_block = transpose_block_8_c;
>> >> >> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
>> >> >> > +                v->transpose_8x8   = transpose_8x8_8_c;
>> >> >> > +                v->copyline_block  = copyline_block_8; break;
>> >> >> >          case 2: v->transpose_block = transpose_block_16_c;
>> >> >> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
>> >> >> > +                v->transpose_8x8   = transpose_8x8_16_c;
>> >> >> > +                v->copyline_block  = copyline_block_16; break;
>> >> >> >          case 3: v->transpose_block = transpose_block_24_c;
>> >> >> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
>> >> >> > +                v->transpose_8x8   = transpose_8x8_24_c;
>> >> >> > +                v->copyline_block  = copyline_block_24; break;
>> >> >> >          case 4: v->transpose_block = transpose_block_32_c;
>> >> >> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
>> >> >> > +                v->transpose_8x8   = transpose_8x8_32_c;
>> >> >> > +                v->copyline_block  = copyline_block_32; break;
>> >> >> >          case 6: v->transpose_block = transpose_block_48_c;
>> >> >> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
>> >> >> > +                v->transpose_8x8   = transpose_8x8_48_c;
>> >> >> > +                v->copyline_block  = copyline_block_48; break;
>> >> >> >          case 8: v->transpose_block = transpose_block_64_c;
>> >> >> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
>> >> >> > +                v->transpose_8x8   = transpose_8x8_64_c;
>> >> >> > +                v->copyline_block  = copyline_block_64; break;
>> >> >> >          }
>> >> >> >      }
>> >> >> >
>> >> >> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext
>> >> >> > *ctx,
>> >> void
>> >> >> > *arg, int jobnr,
>> >> >> >          uint8_t *dst, *src;
>> >> >> >          int dstlinesize, srclinesize;
>> >> >> >          int x, y;
>> >> >> > +        int dir = s->dir;
>> >> >> >          TransVtable *v = &s->vtables[plane];
>> >> >> > +
>> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
>> >> >> > +            int line_dir = 1;
>> >> >> > +            dstlinesize = out->linesize[plane];
>> >> >> > +            dst         = out->data[plane] + start * dstlinesize;
>> >> >> > +            srclinesize = in->linesize[plane];
>> >> >> > +            src         = in->data[plane] + start * srclinesize;
>> >> >> > +
>> >> >> > +            switch (s->orientation) {
>> >> >> > +                case 2: // mirror horizontal
>> >> >> > +                    line_dir     = -1;
>> >> >> > +                    break;
>> >> >> > +                case 3: // rotate 180
>> >> >> > +                    dst          = out->data[plane] + (outh -
>> start
>> >> >> > -
>> >> >> 1) *
>> >> >> > dstlinesize;
>> >> >> > +                    line_dir     = -1;
>> >> >> > +                    dstlinesize *= -1;
>> >> >> > +                    break;
>> >> >> > +                case 4: // mirror vertical
>> >> >> > +                    dst          = out->data[plane] + (outh -
>> start
>> >> >> > -
>> >> >> 1) *
>> >> >> > dstlinesize;
>> >> >> > +                    dstlinesize *= -1;
>> >> >> > +                    break;
>> >> >> > +                default:
>> >> >> > +                    break;
>> >> >> > +            }
>> >> >> > +            v->copyline_block(src, srclinesize, dst, dstlinesize,
>> >> >> line_dir,
>> >> >> > outw, end - start);
>> >> >> > +        } else {
>> >> >> > +            dstlinesize = out->linesize[plane];
>> >> >> > +            dst         = out->data[plane] + start * dstlinesize;
>> >> >> > +            src         = in->data[plane];
>> >> >> > +            srclinesize = in->linesize[plane];
>> >> >> > +            switch (s->orientation) {
>> >> >> > +                case 5: // mirror horizontal and rotate 270 CW
>> >> >> > +                    dir = 0; break;
>> >> >> > +                case 6: // rotate 90 CW
>> >> >> > +                    dir = 1; break;
>> >> >> > +                case 7: // mirror horizontal and rotate 90 CW
>> >> >> > +                    dir = 3; break;
>> >> >> > +                case 8: // rotate 270 CW
>> >> >> > +                    dir = 2; break;
>> >> >> > +                default: break;
>> >> >> > +            }
>> >> >> >
>> >> >> > -        dstlinesize = out->linesize[plane];
>> >> >> > -        dst         = out->data[plane] + start * dstlinesize;
>> >> >> > -        src         = in->data[plane];
>> >> >> > -        srclinesize = in->linesize[plane];
>> >> >> > -
>> >> >> > -        if (s->dir & 1) {
>> >> >> > -            src         += in->linesize[plane] * (inh - 1);
>> >> >> > -            srclinesize *= -1;
>> >> >> > -        }
>> >> >> > +            if (dir & 1) {
>> >> >> > +                src         += in->linesize[plane] * (inh - 1);
>> >> >> > +                srclinesize *= -1;
>> >> >> > +            }
>> >> >> >
>> >> >> > -        if (s->dir & 2) {
>> >> >> > -            dst          = out->data[plane] + dstlinesize * (outh
>> -
>> >> >> start -
>> >> >> > 1);
>> >> >> > -            dstlinesize *= -1;
>> >> >> > -        }
>> >> >> > +            if (dir & 2) {
>> >> >> > +                dst          = out->data[plane] + dstlinesize *
>> >> (outh -
>> >> >> > start - 1);
>> >> >> > +                dstlinesize *= -1;
>> >> >> > +            }
>> >> >> >
>> >> >> > -        for (y = start; y < end - 7; y += 8) {
>> >> >> > -            for (x = 0; x < outw - 7; x += 8) {
>> >> >> > -                v->transpose_8x8(src + x * srclinesize + y *
>> >> >> > pixstep,
>> >> >> > -                                 srclinesize,
>> >> >> > -                                 dst + (y - start) * dstlinesize
>> + x
>> >> *
>> >> >> > pixstep,
>> >> >> > -                                 dstlinesize);
>> >> >> > +            for (y = start; y < end - 7; y += 8) {
>> >> >> > +                for (x = 0; x < outw - 7; x += 8) {
>> >> >> > +                    v->transpose_8x8(src + x * srclinesize + y *
>> >> >> pixstep,
>> >> >> > +                                    srclinesize,
>> >> >> > +                                    dst + (y - start) *
>> dstlinesize
>> >> + x
>> >> >> *
>> >> >> > pixstep,
>> >> >> > +                                    dstlinesize);
>> >> >> > +                }
>> >> >> > +                if (outw - x > 0 && end - y > 0)
>> >> >> > +                    v->transpose_block(src + x * srclinesize + y
>> >> >> > *
>> >> >> pixstep,
>> >> >> > +                                    srclinesize,
>> >> >> > +                                    dst + (y - start) *
>> dstlinesize
>> >> + x
>> >> >> *
>> >> >> > pixstep,
>> >> >> > +                                    dstlinesize, outw - x, end -
>> y);
>> >> >> >              }
>> >> >> > -            if (outw - x > 0 && end - y > 0)
>> >> >> > -                v->transpose_block(src + x * srclinesize + y *
>> >> pixstep,
>> >> >> > -                                   srclinesize,
>> >> >> > -                                   dst + (y - start) *
>> dstlinesize +
>> >> x
>> >> >> > *
>> >> >> > pixstep,
>> >> >> > -                                   dstlinesize, outw - x, end -
>> y);
>> >> >> > -        }
>> >> >> >
>> >> >> > -        if (end - y > 0)
>> >> >> > -            v->transpose_block(src + 0 * srclinesize + y *
>> pixstep,
>> >> >> > -                               srclinesize,
>> >> >> > -                               dst + (y - start) * dstlinesize +
>> 0 *
>> >> >> > pixstep,
>> >> >> > -                               dstlinesize, outw, end - y);
>> >> >> > +            if (end - y > 0)
>> >> >> > +                v->transpose_block(src + 0 * srclinesize + y *
>> >> pixstep,
>> >> >> > +                                srclinesize,
>> >> >> > +                                dst + (y - start) * dstlinesize +
>> 0
>> >> >> > *
>> >> >> > pixstep,
>> >> >> > +                                dstlinesize, outw, end - y);
>> >> >> > +        }
>> >> >> >      }
>> >> >> >
>> >> >> >      return 0;
>> >> >> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink *inlink,
>> >> >> > AVFrame
>> >> >> > *in)
>> >> >> >      ThreadData td;
>> >> >> >      AVFrame *out;
>> >> >> >
>> >> >> > -    if (s->passthrough)
>> >> >> > +    if (s->passthrough || s->orientation == 1)
>> >> >> >          return ff_filter_frame(outlink, in);
>> >> >> >
>> >> >> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
>> >> >> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink *inlink,
>> >> >> AVFrame
>> >> >> > *in)
>> >> >> >      if (in->sample_aspect_ratio.num == 0) {
>> >> >> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
>> >> >> >      } else {
>> >> >> > -        out->sample_aspect_ratio.num =
>> in->sample_aspect_ratio.den;
>> >> >> > -        out->sample_aspect_ratio.den =
>> in->sample_aspect_ratio.num;
>> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
>> >> >> > +            out->sample_aspect_ratio.num =
>> >> in->sample_aspect_ratio.num;
>> >> >> > +            out->sample_aspect_ratio.den =
>> >> in->sample_aspect_ratio.den;
>> >> >> > +        } else {
>> >> >> > +            out->sample_aspect_ratio.num =
>> >> in->sample_aspect_ratio.den;
>> >> >> > +            out->sample_aspect_ratio.den =
>> >> in->sample_aspect_ratio.num;
>> >> >> > +        }
>> >> >> >      }
>> >> >> >
>> >> >> >      td.in = in, td.out = out;
>> >> >> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[] =
>> >> >> > {
>> >> >> >          { "clock",       "rotate clockwise",
>> >> >> 0,
>> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS,
>> >> .unit
>> >> >> =
>> >> >> > "dir" },
>> >> >> >          { "cclock",      "rotate counter-clockwise",
>> >> >> 0,
>> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS,
>> >> .unit
>> >> >> =
>> >> >> > "dir" },
>> >> >> >          { "clock_flip",  "rotate clockwise with vertical flip",
>> >> >>  0,
>> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS,
>> >> .unit
>> >> >> =
>> >> >> > "dir" },
>> >> >> > -
>> >> >> > +
>> >> >> >      { "passthrough", "do not apply transposition if the input
>> >> >> > matches
>> >> >> the
>> >> >> > specified geometry",
>> >> >> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
>> >> >> {.i64=TRANSPOSE_PT_TYPE_NONE},
>> >> >> > 0, INT_MAX, FLAGS, "passthrough" },
>> >> >> >          { "none",      "always apply transposition",   0,
>> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN,
>> >> INT_MAX,
>> >> >> > FLAGS, "passthrough" },
>> >> >> >          { "portrait",  "preserve portrait geometry",   0,
>> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN,
>> >> INT_MAX,
>> >> >> > FLAGS, "passthrough" },
>> >> >> >          { "landscape", "preserve landscape geometry",  0,
>> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN,
>> >> INT_MAX,
>> >> >> > FLAGS, "passthrough" },
>> >> >> > -
>> >> >> > +    { "orientation", "set exif orientation", OFFSET(orientation),
>> >> >> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
>> >> >> >      { NULL }
>> >> >> >  };
>> >> >> >
>> >> >> > --
>> >> >> > 2.17.1
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > ffmpeg-devel mailing list
>> >> >> > ffmpeg-devel@ffmpeg.org
>> >> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >> >
>> >> >> > To unsubscribe, visit link above, or email
>> >> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> >> _______________________________________________
>> >> >> ffmpeg-devel mailing list
>> >> >> ffmpeg-devel@ffmpeg.org
>> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >>
>> >> >> To unsubscribe, visit link above, or email
>> >> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> > _______________________________________________
>> >> > ffmpeg-devel mailing list
>> >> > ffmpeg-devel@ffmpeg.org
>> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >
>> >> > To unsubscribe, visit link above, or email
>> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> _______________________________________________
>> >> ffmpeg-devel mailing list
>> >> ffmpeg-devel@ffmpeg.org
>> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >>
>> >> To unsubscribe, visit link above, or email
>> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> > To unsubscribe, visit link above, or email
>> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Jun Li May 27, 2019, 8:59 a.m. UTC | #10
On Mon, May 27, 2019 at 1:39 AM Paul B Mahol <onemda@gmail.com> wrote:

> On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
> > On Mon, May 27, 2019 at 12:32 AM Paul B Mahol <onemda@gmail.com> wrote:
> >
> >> On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
> >> > On Sun, May 26, 2019 at 2:09 AM Paul B Mahol <onemda@gmail.com>
> wrote:
> >> >
> >> >> On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
> >> >> > On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com>
> >> wrote:
> >> >> >
> >> >> >> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
> >> >> >> > Add exif orientation support and expose an option.
> >> >> >> > ---
> >> >> >> >  libavfilter/vf_transpose.c | 258
> >> >> +++++++++++++++++++++++++++++--------
> >> >> >> >  1 file changed, 207 insertions(+), 51 deletions(-)
> >> >> >> >
> >> >> >> > diff --git a/libavfilter/vf_transpose.c
> >> b/libavfilter/vf_transpose.c
> >> >> >> > index dd54947bd9..4aebfb9ee4 100644
> >> >> >> > --- a/libavfilter/vf_transpose.c
> >> >> >> > +++ b/libavfilter/vf_transpose.c
> >> >> >> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
> >> >> >> >      void (*transpose_block)(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> >> >                              uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> >> >                              int w, int h);
> >> >> >> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t
> src_linesize,
> >> >> >> > +                           uint8_t *dst, ptrdiff_t
> dst_linesize,
> >> >> >> > +                           int line_dir, int w, int h);
> >> >> >> >  } TransVtable;
> >> >> >> >
> >> >> >>
> >> >> >> This is slow, the operations should be in one loop.
> >> >> >>
> >> >> >
> >> >> > Thanks Paul for review.
> >> >> > I see transpose is using " ctx->internal->execute(ctx,
> filter_slice,
> >> >> > &td,
> >> >> > NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
> >> >> > which looks like leveraging multi-threading.
> >> >> > If we do that in one loop, I guess it would be hard to use
> >> >> multi-threading
> >> >> > to accelerate ? But correct me if I am wrong.
> >> >>
> >> >> Transpose filter does one thing and that is transposing.
> >> >>
> >> >> If I'm not mistaken you are adding vertical and horizontal flipping?
> >> >>
> >> >
> >> > Thanks for review.
> >> > Yes, I am doing vflip, hflip and rotate180(vflip+hflip).
> >> >
> >> >
> >> >> If you really need to do that, do it efficiently in one loop, by
> that I
> >> >> mean
> >> >> you do not do cascading operations at all.
> >> >>
> >> >
> >> > Are you suggesting not use function pointer, and call the function
> >> directly
> >> > like this:
> >>
> >> Please do not invent things I never said.
> >
> > I am sorry that I wrongly understand your question. I am not a native
> > speaker, obviously here we have different understanding about the same
> > sentence.
> >
> > I think I was very clear.
> >
> > But to repeat it once more, it is unacceptable to transpose and do
> >> vflip/hflip as
> >> two separate operations. Instead everything should be done in single
> pass.
> >
> >
> > It may be stupid but I really have trouble understanding "single pass"
> > here. The new added function will be called only once per thread. And
> won't
> > be used together with current transpose method, they are "either-or"
> case.
>
> If they are not used in combination with transpose operation, then I fail
> to see
> point to addition of this slow path when much faster already exist in way
> like calling vflip/hflip filter directly.
> So I really do not see point in re-implementing something in slower
> fashion.
>

Hi Paul,
I compared the perf between vflip and this patch before, they take almost
the same time, around 9ms for a frame.
Hflip is a little special, but surely I can update the patch to avoid frame
copy, and it will be as fast as hflip.

But still this is duplicated feature with vflip/hflip, the only benefit I
can think of is rotate180.
Previously we need chain two filter (vflip + hflip) to do rotate180, here
we only need one.

Best Regards,
Jun

>
> > Or are you suggesting moving the flip operation into existing function,
> > rather than create a new one ?
>
>
> >
> > Best Regards,
> > Jun
> >
> >>             switch (s->pixsteps[plane]) {
> >> >                 case 1:  copyline_block_8(src, srclinesize, dst,
> >> > dstlinesize, line_dir, outw, end - start); break;
> >> >                 case 2:  copyline_block_16(src, srclinesize, dst,
> >> > dstlinesize, line_dir, outw, end - start); break;
> >> >                 case 3:  copyline_block_24(src, srclinesize, dst,
> >> > dstlinesize, line_dir, outw, end - start); break;
> >> >                 case 4:  copyline_block_32(src, srclinesize, dst,
> >> > dstlinesize, line_dir, outw, end - start); break;
> >> >                 case 6:  copyline_block_48(src, srclinesize, dst,
> >> > dstlinesize, line_dir, outw, end - start); break;
> >> >                 case 8:  copyline_block_64(src, srclinesize, dst,
> >> > dstlinesize, line_dir, outw, end - start); break;
> >> >             }
> >> >
> >> > I did some perf comparsion, they are very close and no significant
> >> > difference, about 9ms for one frame.
> >> >
> >> > Also I did some comparison between current hflip filter and this
> patch,
> >> > they take almost the same time, about 9ms too on my machine and I see
> 13
> >> > threads/jobs are used for filtering.
> >> > I just use the simple command "ffmpeg  -i test.bmp -vf
> >> > transpose=orientation=2  -y hello.jpg".
> >> > Current vflip wins a lot and I see it is not copying frame at all,
> just
> >> > like "orientation=1" or "passthrough" cases.
> >> >
> >> > I hope I understand you correctly, especially about "cascading
> >> operations".
> >> > But correct me if I am wrong.
> >> > Thanks for your time Paul.
> >> >
> >> > Best Regards,
> >> > -Jun
> >> >
> >> >>
> >> >> > Best Regards,
> >> >> > -Jun
> >> >> >
> >> >> >
> >> >> >> >  typedef struct TransContext {
> >> >> >> > @@ -56,7 +59,7 @@ typedef struct TransContext {
> >> >> >> >
> >> >> >> >      int passthrough;    ///< PassthroughType, landscape
> >> passthrough
> >> >> >> > mode
> >> >> >> > enabled
> >> >> >> >      int dir;            ///< TransposeDir
> >> >> >> > -
> >> >> >> > +    int orientation;    ///< Orientation
> >> >> >> >      TransVtable vtables[4];
> >> >> >> >  } TransContext;
> >> >> >> >
> >> >> >> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t
> *src,
> >> >> >> ptrdiff_t
> >> >> >> > src_linesize,
> >> >> >> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize,
> 8,
> >> >> >> > 8);
> >> >> >> >  }
> >> >> >> >
> >> >> >> > +
> >> >> >> > +static void copyline_block_8(uint8_t *src, ptrdiff_t
> >> >> >> > src_linesize,
> >> >> >> > +                             uint8_t *dst, ptrdiff_t
> >> >> >> > dst_linesize,
> >> >> >> > +                             int line_dir, int w, int h)
> >> >> >> > +{
> >> >> >> > +    int x, y, i;
> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> >> src_linesize) {
> >> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> >> > +            dst[i] = src[x];
> >> >> >> > +        }
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> > +static void copyline_block_16(uint8_t *src, ptrdiff_t
> >> src_linesize,
> >> >> >> > +                              uint8_t *dst, ptrdiff_t
> >> dst_linesize,
> >> >> >> > +                              int line_dir, int w, int h)
> >> >> >> > +{
> >> >> >> > +    int x, y, i;
> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> >> src_linesize) {
> >> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> >> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src +
> >> 2*x));
> >> >> >> > +        }
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> > +static void copyline_block_24(uint8_t *src, ptrdiff_t
> >> src_linesize,
> >> >> >> > +                              uint8_t *dst, ptrdiff_t
> >> dst_linesize,
> >> >> >> > +                              int line_dir, int w, int h)
> >> >> >> > +{
> >> >> >> > +    int x, y, i;
> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> >> src_linesize) {
> >> >> >> > +        for (x = 0; x < w; x++) {
> >> >> >> > +            int32_t v = AV_RB24(src + 3*x);
> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> >> > +            AV_WB24(dst + 3*i, v);
> >> >> >> > +        }
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> > +static void copyline_block_32(uint8_t *src, ptrdiff_t
> >> src_linesize,
> >> >> >> > +                              uint8_t *dst, ptrdiff_t
> >> dst_linesize,
> >> >> >> > +                              int line_dir, int w, int h)
> >> >> >> > +{
> >> >> >> > +    int x, y, i;
> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> >> src_linesize) {
> >> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> >> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src +
> >> 4*x));
> >> >> >> > +        }
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> > +static void copyline_block_48(uint8_t *src, ptrdiff_t
> >> src_linesize,
> >> >> >> > +                              uint8_t *dst, ptrdiff_t
> >> dst_linesize,
> >> >> >> > +                              int line_dir, int w, int h)
> >> >> >> > +{
> >> >> >> > +    int x, y, i;
> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> >> src_linesize) {
> >> >> >> > +        for (x = 0; x < w; x++) {
> >> >> >> > +            int64_t v = AV_RB48(src + 6*x);
> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> >> > +            AV_WB48(dst + 6*i, v);
> >> >> >> > +        }
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> > +static void copyline_block_64(uint8_t *src, ptrdiff_t
> >> src_linesize,
> >> >> >> > +                              uint8_t *dst, ptrdiff_t
> >> dst_linesize,
> >> >> >> > +                              int line_dir, int w, int h)
> >> >> >> > +{
> >> >> >> > +    int x, y, i;
> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
> >> >> src_linesize) {
> >> >> >> > +        for (x = 0; x < w; x ++) {
> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
> >> >> >> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src +
> >> 8*x));
> >> >> >> > +        }
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> > +static void set_outlink_width_height(AVFilterLink *inlink,
> >> >> AVFilterLink
> >> >> >> > *outlink, int transpose)
> >> >> >> > +{
> >> >> >> > +    if (transpose) {
> >> >> >> > +        outlink->w = inlink->h;
> >> >> >> > +        outlink->h = inlink->w;
> >> >> >> > +
> >> >> >> > +        if (inlink->sample_aspect_ratio.num)
> >> >> >> > +            outlink->sample_aspect_ratio =
> av_div_q((AVRational)
> >> >> >> > {
> >> >> 1, 1
> >> >> >> },
> >> >> >> > +
> >> >> >> > inlink->sample_aspect_ratio);
> >> >> >> > +        else
> >> >> >> > +            outlink->sample_aspect_ratio =
> >> >> inlink->sample_aspect_ratio;
> >> >> >> > +    } else {
> >> >> >> > +        outlink->w = inlink->w;
> >> >> >> > +        outlink->h = inlink->h;
> >> >> >> > +        outlink->sample_aspect_ratio =
> >> inlink->sample_aspect_ratio;
> >> >> >> > +    }
> >> >> >> > +}
> >> >> >> > +
> >> >> >> >  static int config_props_output(AVFilterLink *outlink)
> >> >> >> >  {
> >> >> >> >      AVFilterContext *ctx = outlink->src;
> >> >> >> > @@ -213,33 +315,44 @@ static int
> config_props_output(AVFilterLink
> >> >> >> *outlink)
> >> >> >> >
> >> >> >> >      av_assert0(desc_in->nb_components ==
> >> >> >> > desc_out->nb_components);
> >> >> >> >
> >> >> >> > -
> >> >> >> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
> >> >> >> > +
> >> >> >> > +    if (s->orientation && s->orientation < 5) {
> >> >> >> > +        outlink->h = inlink->h;
> >> >> >> > +        outlink->w = inlink->w;
> >> >> >> > +        outlink->sample_aspect_ratio =
> >> inlink->sample_aspect_ratio;
> >> >> >> > +    } else {
> >> >> >> > +        outlink->w = inlink->h;
> >> >> >> > +        outlink->h = inlink->w;
> >> >> >> >
> >> >> >> > -    outlink->w = inlink->h;
> >> >> >> > -    outlink->h = inlink->w;
> >> >> >> > -
> >> >> >> > -    if (inlink->sample_aspect_ratio.num)
> >> >> >> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) {
> 1,
> >> 1
> >> >> },
> >> >> >> > -
> >> >> >> > inlink->sample_aspect_ratio);
> >> >> >> > -    else
> >> >> >> > -        outlink->sample_aspect_ratio =
> >> inlink->sample_aspect_ratio;
> >> >> >> > +        if (inlink->sample_aspect_ratio.num)
> >> >> >> > +            outlink->sample_aspect_ratio =
> av_div_q((AVRational)
> >> >> >> > {
> >> >> 1, 1
> >> >> >> },
> >> >> >> > +
> >> >> >> > inlink->sample_aspect_ratio);
> >> >> >> > +        else
> >> >> >> > +            outlink->sample_aspect_ratio =
> >> >> inlink->sample_aspect_ratio;
> >> >> >> > +    }
> >> >> >> >
> >> >> >> >      for (int i = 0; i < 4; i++) {
> >> >> >> >          TransVtable *v = &s->vtables[i];
> >> >> >> >          switch (s->pixsteps[i]) {
> >> >> >> >          case 1: v->transpose_block = transpose_block_8_c;
> >> >> >> > -                v->transpose_8x8   = transpose_8x8_8_c;  break;
> >> >> >> > +                v->transpose_8x8   = transpose_8x8_8_c;
> >> >> >> > +                v->copyline_block  = copyline_block_8; break;
> >> >> >> >          case 2: v->transpose_block = transpose_block_16_c;
> >> >> >> > -                v->transpose_8x8   = transpose_8x8_16_c; break;
> >> >> >> > +                v->transpose_8x8   = transpose_8x8_16_c;
> >> >> >> > +                v->copyline_block  = copyline_block_16; break;
> >> >> >> >          case 3: v->transpose_block = transpose_block_24_c;
> >> >> >> > -                v->transpose_8x8   = transpose_8x8_24_c; break;
> >> >> >> > +                v->transpose_8x8   = transpose_8x8_24_c;
> >> >> >> > +                v->copyline_block  = copyline_block_24; break;
> >> >> >> >          case 4: v->transpose_block = transpose_block_32_c;
> >> >> >> > -                v->transpose_8x8   = transpose_8x8_32_c; break;
> >> >> >> > +                v->transpose_8x8   = transpose_8x8_32_c;
> >> >> >> > +                v->copyline_block  = copyline_block_32; break;
> >> >> >> >          case 6: v->transpose_block = transpose_block_48_c;
> >> >> >> > -                v->transpose_8x8   = transpose_8x8_48_c; break;
> >> >> >> > +                v->transpose_8x8   = transpose_8x8_48_c;
> >> >> >> > +                v->copyline_block  = copyline_block_48; break;
> >> >> >> >          case 8: v->transpose_block = transpose_block_64_c;
> >> >> >> > -                v->transpose_8x8   = transpose_8x8_64_c; break;
> >> >> >> > +                v->transpose_8x8   = transpose_8x8_64_c;
> >> >> >> > +                v->copyline_block  = copyline_block_64; break;
> >> >> >> >          }
> >> >> >> >      }
> >> >> >> >
> >> >> >> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext
> >> >> >> > *ctx,
> >> >> void
> >> >> >> > *arg, int jobnr,
> >> >> >> >          uint8_t *dst, *src;
> >> >> >> >          int dstlinesize, srclinesize;
> >> >> >> >          int x, y;
> >> >> >> > +        int dir = s->dir;
> >> >> >> >          TransVtable *v = &s->vtables[plane];
> >> >> >> > +
> >> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
> >> >> >> > +            int line_dir = 1;
> >> >> >> > +            dstlinesize = out->linesize[plane];
> >> >> >> > +            dst         = out->data[plane] + start *
> dstlinesize;
> >> >> >> > +            srclinesize = in->linesize[plane];
> >> >> >> > +            src         = in->data[plane] + start *
> srclinesize;
> >> >> >> > +
> >> >> >> > +            switch (s->orientation) {
> >> >> >> > +                case 2: // mirror horizontal
> >> >> >> > +                    line_dir     = -1;
> >> >> >> > +                    break;
> >> >> >> > +                case 3: // rotate 180
> >> >> >> > +                    dst          = out->data[plane] + (outh -
> >> start
> >> >> >> > -
> >> >> >> 1) *
> >> >> >> > dstlinesize;
> >> >> >> > +                    line_dir     = -1;
> >> >> >> > +                    dstlinesize *= -1;
> >> >> >> > +                    break;
> >> >> >> > +                case 4: // mirror vertical
> >> >> >> > +                    dst          = out->data[plane] + (outh -
> >> start
> >> >> >> > -
> >> >> >> 1) *
> >> >> >> > dstlinesize;
> >> >> >> > +                    dstlinesize *= -1;
> >> >> >> > +                    break;
> >> >> >> > +                default:
> >> >> >> > +                    break;
> >> >> >> > +            }
> >> >> >> > +            v->copyline_block(src, srclinesize, dst,
> dstlinesize,
> >> >> >> line_dir,
> >> >> >> > outw, end - start);
> >> >> >> > +        } else {
> >> >> >> > +            dstlinesize = out->linesize[plane];
> >> >> >> > +            dst         = out->data[plane] + start *
> dstlinesize;
> >> >> >> > +            src         = in->data[plane];
> >> >> >> > +            srclinesize = in->linesize[plane];
> >> >> >> > +            switch (s->orientation) {
> >> >> >> > +                case 5: // mirror horizontal and rotate 270 CW
> >> >> >> > +                    dir = 0; break;
> >> >> >> > +                case 6: // rotate 90 CW
> >> >> >> > +                    dir = 1; break;
> >> >> >> > +                case 7: // mirror horizontal and rotate 90 CW
> >> >> >> > +                    dir = 3; break;
> >> >> >> > +                case 8: // rotate 270 CW
> >> >> >> > +                    dir = 2; break;
> >> >> >> > +                default: break;
> >> >> >> > +            }
> >> >> >> >
> >> >> >> > -        dstlinesize = out->linesize[plane];
> >> >> >> > -        dst         = out->data[plane] + start * dstlinesize;
> >> >> >> > -        src         = in->data[plane];
> >> >> >> > -        srclinesize = in->linesize[plane];
> >> >> >> > -
> >> >> >> > -        if (s->dir & 1) {
> >> >> >> > -            src         += in->linesize[plane] * (inh - 1);
> >> >> >> > -            srclinesize *= -1;
> >> >> >> > -        }
> >> >> >> > +            if (dir & 1) {
> >> >> >> > +                src         += in->linesize[plane] * (inh - 1);
> >> >> >> > +                srclinesize *= -1;
> >> >> >> > +            }
> >> >> >> >
> >> >> >> > -        if (s->dir & 2) {
> >> >> >> > -            dst          = out->data[plane] + dstlinesize *
> (outh
> >> -
> >> >> >> start -
> >> >> >> > 1);
> >> >> >> > -            dstlinesize *= -1;
> >> >> >> > -        }
> >> >> >> > +            if (dir & 2) {
> >> >> >> > +                dst          = out->data[plane] + dstlinesize *
> >> >> (outh -
> >> >> >> > start - 1);
> >> >> >> > +                dstlinesize *= -1;
> >> >> >> > +            }
> >> >> >> >
> >> >> >> > -        for (y = start; y < end - 7; y += 8) {
> >> >> >> > -            for (x = 0; x < outw - 7; x += 8) {
> >> >> >> > -                v->transpose_8x8(src + x * srclinesize + y *
> >> >> >> > pixstep,
> >> >> >> > -                                 srclinesize,
> >> >> >> > -                                 dst + (y - start) *
> dstlinesize
> >> + x
> >> >> *
> >> >> >> > pixstep,
> >> >> >> > -                                 dstlinesize);
> >> >> >> > +            for (y = start; y < end - 7; y += 8) {
> >> >> >> > +                for (x = 0; x < outw - 7; x += 8) {
> >> >> >> > +                    v->transpose_8x8(src + x * srclinesize + y
> *
> >> >> >> pixstep,
> >> >> >> > +                                    srclinesize,
> >> >> >> > +                                    dst + (y - start) *
> >> dstlinesize
> >> >> + x
> >> >> >> *
> >> >> >> > pixstep,
> >> >> >> > +                                    dstlinesize);
> >> >> >> > +                }
> >> >> >> > +                if (outw - x > 0 && end - y > 0)
> >> >> >> > +                    v->transpose_block(src + x * srclinesize +
> y
> >> >> >> > *
> >> >> >> pixstep,
> >> >> >> > +                                    srclinesize,
> >> >> >> > +                                    dst + (y - start) *
> >> dstlinesize
> >> >> + x
> >> >> >> *
> >> >> >> > pixstep,
> >> >> >> > +                                    dstlinesize, outw - x, end
> -
> >> y);
> >> >> >> >              }
> >> >> >> > -            if (outw - x > 0 && end - y > 0)
> >> >> >> > -                v->transpose_block(src + x * srclinesize + y *
> >> >> pixstep,
> >> >> >> > -                                   srclinesize,
> >> >> >> > -                                   dst + (y - start) *
> >> dstlinesize +
> >> >> x
> >> >> >> > *
> >> >> >> > pixstep,
> >> >> >> > -                                   dstlinesize, outw - x, end -
> >> y);
> >> >> >> > -        }
> >> >> >> >
> >> >> >> > -        if (end - y > 0)
> >> >> >> > -            v->transpose_block(src + 0 * srclinesize + y *
> >> pixstep,
> >> >> >> > -                               srclinesize,
> >> >> >> > -                               dst + (y - start) * dstlinesize
> +
> >> 0 *
> >> >> >> > pixstep,
> >> >> >> > -                               dstlinesize, outw, end - y);
> >> >> >> > +            if (end - y > 0)
> >> >> >> > +                v->transpose_block(src + 0 * srclinesize + y *
> >> >> pixstep,
> >> >> >> > +                                srclinesize,
> >> >> >> > +                                dst + (y - start) *
> dstlinesize +
> >> 0
> >> >> >> > *
> >> >> >> > pixstep,
> >> >> >> > +                                dstlinesize, outw, end - y);
> >> >> >> > +        }
> >> >> >> >      }
> >> >> >> >
> >> >> >> >      return 0;
> >> >> >> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink
> *inlink,
> >> >> >> > AVFrame
> >> >> >> > *in)
> >> >> >> >      ThreadData td;
> >> >> >> >      AVFrame *out;
> >> >> >> >
> >> >> >> > -    if (s->passthrough)
> >> >> >> > +    if (s->passthrough || s->orientation == 1)
> >> >> >> >          return ff_filter_frame(outlink, in);
> >> >> >> >
> >> >> >> >      out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
> >> >> >> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink
> *inlink,
> >> >> >> AVFrame
> >> >> >> > *in)
> >> >> >> >      if (in->sample_aspect_ratio.num == 0) {
> >> >> >> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
> >> >> >> >      } else {
> >> >> >> > -        out->sample_aspect_ratio.num =
> >> in->sample_aspect_ratio.den;
> >> >> >> > -        out->sample_aspect_ratio.den =
> >> in->sample_aspect_ratio.num;
> >> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
> >> >> >> > +            out->sample_aspect_ratio.num =
> >> >> in->sample_aspect_ratio.num;
> >> >> >> > +            out->sample_aspect_ratio.den =
> >> >> in->sample_aspect_ratio.den;
> >> >> >> > +        } else {
> >> >> >> > +            out->sample_aspect_ratio.num =
> >> >> in->sample_aspect_ratio.den;
> >> >> >> > +            out->sample_aspect_ratio.den =
> >> >> in->sample_aspect_ratio.num;
> >> >> >> > +        }
> >> >> >> >      }
> >> >> >> >
> >> >> >> >      td.in = in, td.out = out;
> >> >> >> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[]
> =
> >> >> >> > {
> >> >> >> >          { "clock",       "rotate clockwise",
> >> >> >> 0,
> >> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       },
> .flags=FLAGS,
> >> >> .unit
> >> >> >> =
> >> >> >> > "dir" },
> >> >> >> >          { "cclock",      "rotate counter-clockwise",
> >> >> >> 0,
> >> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      },
> .flags=FLAGS,
> >> >> .unit
> >> >> >> =
> >> >> >> > "dir" },
> >> >> >> >          { "clock_flip",  "rotate clockwise with vertical flip",
> >> >> >>  0,
> >> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  },
> .flags=FLAGS,
> >> >> .unit
> >> >> >> =
> >> >> >> > "dir" },
> >> >> >> > -
> >> >> >> > +
> >> >> >> >      { "passthrough", "do not apply transposition if the input
> >> >> >> > matches
> >> >> >> the
> >> >> >> > specified geometry",
> >> >> >> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
> >> >> >> {.i64=TRANSPOSE_PT_TYPE_NONE},
> >> >> >> > 0, INT_MAX, FLAGS, "passthrough" },
> >> >> >> >          { "none",      "always apply transposition",   0,
> >> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN,
> >> >> INT_MAX,
> >> >> >> > FLAGS, "passthrough" },
> >> >> >> >          { "portrait",  "preserve portrait geometry",   0,
> >> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN,
> >> >> INT_MAX,
> >> >> >> > FLAGS, "passthrough" },
> >> >> >> >          { "landscape", "preserve landscape geometry",  0,
> >> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN,
> >> >> INT_MAX,
> >> >> >> > FLAGS, "passthrough" },
> >> >> >> > -
> >> >> >> > +    { "orientation", "set exif orientation",
> OFFSET(orientation),
> >> >> >> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
> >> >> >> >      { NULL }
> >> >> >> >  };
> >> >> >> >
> >> >> >> > --
> >> >> >> > 2.17.1
> >> >> >> >
> >> >> >> > _______________________________________________
> >> >> >> > ffmpeg-devel mailing list
> >> >> >> > ffmpeg-devel@ffmpeg.org
> >> >> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >> >> >
> >> >> >> > To unsubscribe, visit link above, or email
> >> >> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> >> >> _______________________________________________
> >> >> >> ffmpeg-devel mailing list
> >> >> >> ffmpeg-devel@ffmpeg.org
> >> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >> >>
> >> >> >> To unsubscribe, visit link above, or email
> >> >> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> >> > _______________________________________________
> >> >> > ffmpeg-devel mailing list
> >> >> > ffmpeg-devel@ffmpeg.org
> >> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >> >
> >> >> > To unsubscribe, visit link above, or email
> >> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> >> _______________________________________________
> >> >> ffmpeg-devel mailing list
> >> >> ffmpeg-devel@ffmpeg.org
> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >>
> >> >> To unsubscribe, visit link above, or email
> >> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> > _______________________________________________
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> > To unsubscribe, visit link above, or email
> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol May 27, 2019, 2:45 p.m. UTC | #11
On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
> On Mon, May 27, 2019 at 1:39 AM Paul B Mahol <onemda@gmail.com> wrote:
>
>> On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
>> > On Mon, May 27, 2019 at 12:32 AM Paul B Mahol <onemda@gmail.com> wrote:
>> >
>> >> On 5/27/19, Jun Li <junli1026@gmail.com> wrote:
>> >> > On Sun, May 26, 2019 at 2:09 AM Paul B Mahol <onemda@gmail.com>
>> wrote:
>> >> >
>> >> >> On 5/26/19, Jun Li <junli1026@gmail.com> wrote:
>> >> >> > On Sun, May 26, 2019 at 1:16 AM Paul B Mahol <onemda@gmail.com>
>> >> wrote:
>> >> >> >
>> >> >> >> On 5/25/19, Jun Li <junli1026@gmail.com> wrote:
>> >> >> >> > Add exif orientation support and expose an option.
>> >> >> >> > ---
>> >> >> >> >  libavfilter/vf_transpose.c | 258
>> >> >> +++++++++++++++++++++++++++++--------
>> >> >> >> >  1 file changed, 207 insertions(+), 51 deletions(-)
>> >> >> >> >
>> >> >> >> > diff --git a/libavfilter/vf_transpose.c
>> >> b/libavfilter/vf_transpose.c
>> >> >> >> > index dd54947bd9..4aebfb9ee4 100644
>> >> >> >> > --- a/libavfilter/vf_transpose.c
>> >> >> >> > +++ b/libavfilter/vf_transpose.c
>> >> >> >> > @@ -46,6 +46,9 @@ typedef struct TransVtable {
>> >> >> >> >      void (*transpose_block)(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> >> >                              uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> >> >                              int w, int h);
>> >> >> >> > +    void (*copyline_block)(uint8_t *src, ptrdiff_t
>> src_linesize,
>> >> >> >> > +                           uint8_t *dst, ptrdiff_t
>> dst_linesize,
>> >> >> >> > +                           int line_dir, int w, int h);
>> >> >> >> >  } TransVtable;
>> >> >> >> >
>> >> >> >>
>> >> >> >> This is slow, the operations should be in one loop.
>> >> >> >>
>> >> >> >
>> >> >> > Thanks Paul for review.
>> >> >> > I see transpose is using " ctx->internal->execute(ctx,
>> filter_slice,
>> >> >> > &td,
>> >> >> > NULL, FFMIN(outlink->h, ff_filter_get_nb_threads(ctx)));" .
>> >> >> > which looks like leveraging multi-threading.
>> >> >> > If we do that in one loop, I guess it would be hard to use
>> >> >> multi-threading
>> >> >> > to accelerate ? But correct me if I am wrong.
>> >> >>
>> >> >> Transpose filter does one thing and that is transposing.
>> >> >>
>> >> >> If I'm not mistaken you are adding vertical and horizontal flipping?
>> >> >>
>> >> >
>> >> > Thanks for review.
>> >> > Yes, I am doing vflip, hflip and rotate180(vflip+hflip).
>> >> >
>> >> >
>> >> >> If you really need to do that, do it efficiently in one loop, by
>> that I
>> >> >> mean
>> >> >> you do not do cascading operations at all.
>> >> >>
>> >> >
>> >> > Are you suggesting not use function pointer, and call the function
>> >> directly
>> >> > like this:
>> >>
>> >> Please do not invent things I never said.
>> >
>> > I am sorry that I wrongly understand your question. I am not a native
>> > speaker, obviously here we have different understanding about the same
>> > sentence.
>> >
>> > I think I was very clear.
>> >
>> > But to repeat it once more, it is unacceptable to transpose and do
>> >> vflip/hflip as
>> >> two separate operations. Instead everything should be done in single
>> pass.
>> >
>> >
>> > It may be stupid but I really have trouble understanding "single pass"
>> > here. The new added function will be called only once per thread. And
>> won't
>> > be used together with current transpose method, they are "either-or"
>> case.
>>
>> If they are not used in combination with transpose operation, then I fail
>> to see
>> point to addition of this slow path when much faster already exist in way
>> like calling vflip/hflip filter directly.
>> So I really do not see point in re-implementing something in slower
>> fashion.
>>
>
> Hi Paul,
> I compared the perf between vflip and this patch before, they take almost
> the same time, around 9ms for a frame.
> Hflip is a little special, but surely I can update the patch to avoid frame
> copy, and it will be as fast as hflip.
>
> But still this is duplicated feature with vflip/hflip, the only benefit I
> can think of is rotate180.
> Previously we need chain two filter (vflip + hflip) to do rotate180, here
> we only need one.

vflip is free, hflip have SIMD, your code does not have SIMD.

>
> Best Regards,
> Jun
>
>>
>> > Or are you suggesting moving the flip operation into existing function,
>> > rather than create a new one ?
>>
>>
>> >
>> > Best Regards,
>> > Jun
>> >
>> >>             switch (s->pixsteps[plane]) {
>> >> >                 case 1:  copyline_block_8(src, srclinesize, dst,
>> >> > dstlinesize, line_dir, outw, end - start); break;
>> >> >                 case 2:  copyline_block_16(src, srclinesize, dst,
>> >> > dstlinesize, line_dir, outw, end - start); break;
>> >> >                 case 3:  copyline_block_24(src, srclinesize, dst,
>> >> > dstlinesize, line_dir, outw, end - start); break;
>> >> >                 case 4:  copyline_block_32(src, srclinesize, dst,
>> >> > dstlinesize, line_dir, outw, end - start); break;
>> >> >                 case 6:  copyline_block_48(src, srclinesize, dst,
>> >> > dstlinesize, line_dir, outw, end - start); break;
>> >> >                 case 8:  copyline_block_64(src, srclinesize, dst,
>> >> > dstlinesize, line_dir, outw, end - start); break;
>> >> >             }
>> >> >
>> >> > I did some perf comparsion, they are very close and no significant
>> >> > difference, about 9ms for one frame.
>> >> >
>> >> > Also I did some comparison between current hflip filter and this
>> patch,
>> >> > they take almost the same time, about 9ms too on my machine and I see
>> 13
>> >> > threads/jobs are used for filtering.
>> >> > I just use the simple command "ffmpeg  -i test.bmp -vf
>> >> > transpose=orientation=2  -y hello.jpg".
>> >> > Current vflip wins a lot and I see it is not copying frame at all,
>> just
>> >> > like "orientation=1" or "passthrough" cases.
>> >> >
>> >> > I hope I understand you correctly, especially about "cascading
>> >> operations".
>> >> > But correct me if I am wrong.
>> >> > Thanks for your time Paul.
>> >> >
>> >> > Best Regards,
>> >> > -Jun
>> >> >
>> >> >>
>> >> >> > Best Regards,
>> >> >> > -Jun
>> >> >> >
>> >> >> >
>> >> >> >> >  typedef struct TransContext {
>> >> >> >> > @@ -56,7 +59,7 @@ typedef struct TransContext {
>> >> >> >> >
>> >> >> >> >      int passthrough;    ///< PassthroughType, landscape
>> >> passthrough
>> >> >> >> > mode
>> >> >> >> > enabled
>> >> >> >> >      int dir;            ///< TransposeDir
>> >> >> >> > -
>> >> >> >> > +    int orientation;    ///< Orientation
>> >> >> >> >      TransVtable vtables[4];
>> >> >> >> >  } TransContext;
>> >> >> >> >
>> >> >> >> > @@ -182,6 +185,105 @@ static void transpose_8x8_64_c(uint8_t
>> *src,
>> >> >> >> ptrdiff_t
>> >> >> >> > src_linesize,
>> >> >> >> >      transpose_block_64_c(src, src_linesize, dst, dst_linesize,
>> 8,
>> >> >> >> > 8);
>> >> >> >> >  }
>> >> >> >> >
>> >> >> >> > +
>> >> >> >> > +static void copyline_block_8(uint8_t *src, ptrdiff_t
>> >> >> >> > src_linesize,
>> >> >> >> > +                             uint8_t *dst, ptrdiff_t
>> >> >> >> > dst_linesize,
>> >> >> >> > +                             int line_dir, int w, int h)
>> >> >> >> > +{
>> >> >> >> > +    int x, y, i;
>> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> >> src_linesize) {
>> >> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> >> > +            dst[i] = src[x];
>> >> >> >> > +        }
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> > +static void copyline_block_16(uint8_t *src, ptrdiff_t
>> >> src_linesize,
>> >> >> >> > +                              uint8_t *dst, ptrdiff_t
>> >> dst_linesize,
>> >> >> >> > +                              int line_dir, int w, int h)
>> >> >> >> > +{
>> >> >> >> > +    int x, y, i;
>> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> >> src_linesize) {
>> >> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> >> > +            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src +
>> >> 2*x));
>> >> >> >> > +        }
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> > +static void copyline_block_24(uint8_t *src, ptrdiff_t
>> >> src_linesize,
>> >> >> >> > +                              uint8_t *dst, ptrdiff_t
>> >> dst_linesize,
>> >> >> >> > +                              int line_dir, int w, int h)
>> >> >> >> > +{
>> >> >> >> > +    int x, y, i;
>> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> >> src_linesize) {
>> >> >> >> > +        for (x = 0; x < w; x++) {
>> >> >> >> > +            int32_t v = AV_RB24(src + 3*x);
>> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> >> > +            AV_WB24(dst + 3*i, v);
>> >> >> >> > +        }
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> > +static void copyline_block_32(uint8_t *src, ptrdiff_t
>> >> src_linesize,
>> >> >> >> > +                              uint8_t *dst, ptrdiff_t
>> >> dst_linesize,
>> >> >> >> > +                              int line_dir, int w, int h)
>> >> >> >> > +{
>> >> >> >> > +    int x, y, i;
>> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> >> src_linesize) {
>> >> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> >> > +            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src +
>> >> 4*x));
>> >> >> >> > +        }
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> > +static void copyline_block_48(uint8_t *src, ptrdiff_t
>> >> src_linesize,
>> >> >> >> > +                              uint8_t *dst, ptrdiff_t
>> >> dst_linesize,
>> >> >> >> > +                              int line_dir, int w, int h)
>> >> >> >> > +{
>> >> >> >> > +    int x, y, i;
>> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> >> src_linesize) {
>> >> >> >> > +        for (x = 0; x < w; x++) {
>> >> >> >> > +            int64_t v = AV_RB48(src + 6*x);
>> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> >> > +            AV_WB48(dst + 6*i, v);
>> >> >> >> > +        }
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> > +static void copyline_block_64(uint8_t *src, ptrdiff_t
>> >> src_linesize,
>> >> >> >> > +                              uint8_t *dst, ptrdiff_t
>> >> dst_linesize,
>> >> >> >> > +                              int line_dir, int w, int h)
>> >> >> >> > +{
>> >> >> >> > +    int x, y, i;
>> >> >> >> > +    for (y = 0; y < h; y++, dst += dst_linesize, src +=
>> >> >> src_linesize) {
>> >> >> >> > +        for (x = 0; x < w; x ++) {
>> >> >> >> > +            i = line_dir < 0 ? w-x-1 : x;
>> >> >> >> > +            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src +
>> >> 8*x));
>> >> >> >> > +        }
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> > +static void set_outlink_width_height(AVFilterLink *inlink,
>> >> >> AVFilterLink
>> >> >> >> > *outlink, int transpose)
>> >> >> >> > +{
>> >> >> >> > +    if (transpose) {
>> >> >> >> > +        outlink->w = inlink->h;
>> >> >> >> > +        outlink->h = inlink->w;
>> >> >> >> > +
>> >> >> >> > +        if (inlink->sample_aspect_ratio.num)
>> >> >> >> > +            outlink->sample_aspect_ratio =
>> av_div_q((AVRational)
>> >> >> >> > {
>> >> >> 1, 1
>> >> >> >> },
>> >> >> >> > +
>> >> >> >> > inlink->sample_aspect_ratio);
>> >> >> >> > +        else
>> >> >> >> > +            outlink->sample_aspect_ratio =
>> >> >> inlink->sample_aspect_ratio;
>> >> >> >> > +    } else {
>> >> >> >> > +        outlink->w = inlink->w;
>> >> >> >> > +        outlink->h = inlink->h;
>> >> >> >> > +        outlink->sample_aspect_ratio =
>> >> inlink->sample_aspect_ratio;
>> >> >> >> > +    }
>> >> >> >> > +}
>> >> >> >> > +
>> >> >> >> >  static int config_props_output(AVFilterLink *outlink)
>> >> >> >> >  {
>> >> >> >> >      AVFilterContext *ctx = outlink->src;
>> >> >> >> > @@ -213,33 +315,44 @@ static int
>> config_props_output(AVFilterLink
>> >> >> >> *outlink)
>> >> >> >> >
>> >> >> >> >      av_assert0(desc_in->nb_components ==
>> >> >> >> > desc_out->nb_components);
>> >> >> >> >
>> >> >> >> > -
>> >> >> >> >      av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
>> >> >> >> > +
>> >> >> >> > +    if (s->orientation && s->orientation < 5) {
>> >> >> >> > +        outlink->h = inlink->h;
>> >> >> >> > +        outlink->w = inlink->w;
>> >> >> >> > +        outlink->sample_aspect_ratio =
>> >> inlink->sample_aspect_ratio;
>> >> >> >> > +    } else {
>> >> >> >> > +        outlink->w = inlink->h;
>> >> >> >> > +        outlink->h = inlink->w;
>> >> >> >> >
>> >> >> >> > -    outlink->w = inlink->h;
>> >> >> >> > -    outlink->h = inlink->w;
>> >> >> >> > -
>> >> >> >> > -    if (inlink->sample_aspect_ratio.num)
>> >> >> >> > -        outlink->sample_aspect_ratio = av_div_q((AVRational) {
>> 1,
>> >> 1
>> >> >> },
>> >> >> >> > -
>> >> >> >> > inlink->sample_aspect_ratio);
>> >> >> >> > -    else
>> >> >> >> > -        outlink->sample_aspect_ratio =
>> >> inlink->sample_aspect_ratio;
>> >> >> >> > +        if (inlink->sample_aspect_ratio.num)
>> >> >> >> > +            outlink->sample_aspect_ratio =
>> av_div_q((AVRational)
>> >> >> >> > {
>> >> >> 1, 1
>> >> >> >> },
>> >> >> >> > +
>> >> >> >> > inlink->sample_aspect_ratio);
>> >> >> >> > +        else
>> >> >> >> > +            outlink->sample_aspect_ratio =
>> >> >> inlink->sample_aspect_ratio;
>> >> >> >> > +    }
>> >> >> >> >
>> >> >> >> >      for (int i = 0; i < 4; i++) {
>> >> >> >> >          TransVtable *v = &s->vtables[i];
>> >> >> >> >          switch (s->pixsteps[i]) {
>> >> >> >> >          case 1: v->transpose_block = transpose_block_8_c;
>> >> >> >> > -                v->transpose_8x8   = transpose_8x8_8_c;
>> >> >> >> > break;
>> >> >> >> > +                v->transpose_8x8   = transpose_8x8_8_c;
>> >> >> >> > +                v->copyline_block  = copyline_block_8; break;
>> >> >> >> >          case 2: v->transpose_block = transpose_block_16_c;
>> >> >> >> > -                v->transpose_8x8   = transpose_8x8_16_c;
>> >> >> >> > break;
>> >> >> >> > +                v->transpose_8x8   = transpose_8x8_16_c;
>> >> >> >> > +                v->copyline_block  = copyline_block_16; break;
>> >> >> >> >          case 3: v->transpose_block = transpose_block_24_c;
>> >> >> >> > -                v->transpose_8x8   = transpose_8x8_24_c;
>> >> >> >> > break;
>> >> >> >> > +                v->transpose_8x8   = transpose_8x8_24_c;
>> >> >> >> > +                v->copyline_block  = copyline_block_24; break;
>> >> >> >> >          case 4: v->transpose_block = transpose_block_32_c;
>> >> >> >> > -                v->transpose_8x8   = transpose_8x8_32_c;
>> >> >> >> > break;
>> >> >> >> > +                v->transpose_8x8   = transpose_8x8_32_c;
>> >> >> >> > +                v->copyline_block  = copyline_block_32; break;
>> >> >> >> >          case 6: v->transpose_block = transpose_block_48_c;
>> >> >> >> > -                v->transpose_8x8   = transpose_8x8_48_c;
>> >> >> >> > break;
>> >> >> >> > +                v->transpose_8x8   = transpose_8x8_48_c;
>> >> >> >> > +                v->copyline_block  = copyline_block_48; break;
>> >> >> >> >          case 8: v->transpose_block = transpose_block_64_c;
>> >> >> >> > -                v->transpose_8x8   = transpose_8x8_64_c;
>> >> >> >> > break;
>> >> >> >> > +                v->transpose_8x8   = transpose_8x8_64_c;
>> >> >> >> > +                v->copyline_block  = copyline_block_64; break;
>> >> >> >> >          }
>> >> >> >> >      }
>> >> >> >> >
>> >> >> >> > @@ -285,42 +398,80 @@ static int filter_slice(AVFilterContext
>> >> >> >> > *ctx,
>> >> >> void
>> >> >> >> > *arg, int jobnr,
>> >> >> >> >          uint8_t *dst, *src;
>> >> >> >> >          int dstlinesize, srclinesize;
>> >> >> >> >          int x, y;
>> >> >> >> > +        int dir = s->dir;
>> >> >> >> >          TransVtable *v = &s->vtables[plane];
>> >> >> >> > +
>> >> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
>> >> >> >> > +            int line_dir = 1;
>> >> >> >> > +            dstlinesize = out->linesize[plane];
>> >> >> >> > +            dst         = out->data[plane] + start *
>> dstlinesize;
>> >> >> >> > +            srclinesize = in->linesize[plane];
>> >> >> >> > +            src         = in->data[plane] + start *
>> srclinesize;
>> >> >> >> > +
>> >> >> >> > +            switch (s->orientation) {
>> >> >> >> > +                case 2: // mirror horizontal
>> >> >> >> > +                    line_dir     = -1;
>> >> >> >> > +                    break;
>> >> >> >> > +                case 3: // rotate 180
>> >> >> >> > +                    dst          = out->data[plane] + (outh -
>> >> start
>> >> >> >> > -
>> >> >> >> 1) *
>> >> >> >> > dstlinesize;
>> >> >> >> > +                    line_dir     = -1;
>> >> >> >> > +                    dstlinesize *= -1;
>> >> >> >> > +                    break;
>> >> >> >> > +                case 4: // mirror vertical
>> >> >> >> > +                    dst          = out->data[plane] + (outh -
>> >> start
>> >> >> >> > -
>> >> >> >> 1) *
>> >> >> >> > dstlinesize;
>> >> >> >> > +                    dstlinesize *= -1;
>> >> >> >> > +                    break;
>> >> >> >> > +                default:
>> >> >> >> > +                    break;
>> >> >> >> > +            }
>> >> >> >> > +            v->copyline_block(src, srclinesize, dst,
>> dstlinesize,
>> >> >> >> line_dir,
>> >> >> >> > outw, end - start);
>> >> >> >> > +        } else {
>> >> >> >> > +            dstlinesize = out->linesize[plane];
>> >> >> >> > +            dst         = out->data[plane] + start *
>> dstlinesize;
>> >> >> >> > +            src         = in->data[plane];
>> >> >> >> > +            srclinesize = in->linesize[plane];
>> >> >> >> > +            switch (s->orientation) {
>> >> >> >> > +                case 5: // mirror horizontal and rotate 270 CW
>> >> >> >> > +                    dir = 0; break;
>> >> >> >> > +                case 6: // rotate 90 CW
>> >> >> >> > +                    dir = 1; break;
>> >> >> >> > +                case 7: // mirror horizontal and rotate 90 CW
>> >> >> >> > +                    dir = 3; break;
>> >> >> >> > +                case 8: // rotate 270 CW
>> >> >> >> > +                    dir = 2; break;
>> >> >> >> > +                default: break;
>> >> >> >> > +            }
>> >> >> >> >
>> >> >> >> > -        dstlinesize = out->linesize[plane];
>> >> >> >> > -        dst         = out->data[plane] + start * dstlinesize;
>> >> >> >> > -        src         = in->data[plane];
>> >> >> >> > -        srclinesize = in->linesize[plane];
>> >> >> >> > -
>> >> >> >> > -        if (s->dir & 1) {
>> >> >> >> > -            src         += in->linesize[plane] * (inh - 1);
>> >> >> >> > -            srclinesize *= -1;
>> >> >> >> > -        }
>> >> >> >> > +            if (dir & 1) {
>> >> >> >> > +                src         += in->linesize[plane] * (inh -
>> >> >> >> > 1);
>> >> >> >> > +                srclinesize *= -1;
>> >> >> >> > +            }
>> >> >> >> >
>> >> >> >> > -        if (s->dir & 2) {
>> >> >> >> > -            dst          = out->data[plane] + dstlinesize *
>> (outh
>> >> -
>> >> >> >> start -
>> >> >> >> > 1);
>> >> >> >> > -            dstlinesize *= -1;
>> >> >> >> > -        }
>> >> >> >> > +            if (dir & 2) {
>> >> >> >> > +                dst          = out->data[plane] + dstlinesize
>> >> >> >> > *
>> >> >> (outh -
>> >> >> >> > start - 1);
>> >> >> >> > +                dstlinesize *= -1;
>> >> >> >> > +            }
>> >> >> >> >
>> >> >> >> > -        for (y = start; y < end - 7; y += 8) {
>> >> >> >> > -            for (x = 0; x < outw - 7; x += 8) {
>> >> >> >> > -                v->transpose_8x8(src + x * srclinesize + y *
>> >> >> >> > pixstep,
>> >> >> >> > -                                 srclinesize,
>> >> >> >> > -                                 dst + (y - start) *
>> dstlinesize
>> >> + x
>> >> >> *
>> >> >> >> > pixstep,
>> >> >> >> > -                                 dstlinesize);
>> >> >> >> > +            for (y = start; y < end - 7; y += 8) {
>> >> >> >> > +                for (x = 0; x < outw - 7; x += 8) {
>> >> >> >> > +                    v->transpose_8x8(src + x * srclinesize + y
>> *
>> >> >> >> pixstep,
>> >> >> >> > +                                    srclinesize,
>> >> >> >> > +                                    dst + (y - start) *
>> >> dstlinesize
>> >> >> + x
>> >> >> >> *
>> >> >> >> > pixstep,
>> >> >> >> > +                                    dstlinesize);
>> >> >> >> > +                }
>> >> >> >> > +                if (outw - x > 0 && end - y > 0)
>> >> >> >> > +                    v->transpose_block(src + x * srclinesize +
>> y
>> >> >> >> > *
>> >> >> >> pixstep,
>> >> >> >> > +                                    srclinesize,
>> >> >> >> > +                                    dst + (y - start) *
>> >> dstlinesize
>> >> >> + x
>> >> >> >> *
>> >> >> >> > pixstep,
>> >> >> >> > +                                    dstlinesize, outw - x, end
>> -
>> >> y);
>> >> >> >> >              }
>> >> >> >> > -            if (outw - x > 0 && end - y > 0)
>> >> >> >> > -                v->transpose_block(src + x * srclinesize + y *
>> >> >> pixstep,
>> >> >> >> > -                                   srclinesize,
>> >> >> >> > -                                   dst + (y - start) *
>> >> dstlinesize +
>> >> >> x
>> >> >> >> > *
>> >> >> >> > pixstep,
>> >> >> >> > -                                   dstlinesize, outw - x, end
>> >> >> >> > -
>> >> y);
>> >> >> >> > -        }
>> >> >> >> >
>> >> >> >> > -        if (end - y > 0)
>> >> >> >> > -            v->transpose_block(src + 0 * srclinesize + y *
>> >> pixstep,
>> >> >> >> > -                               srclinesize,
>> >> >> >> > -                               dst + (y - start) * dstlinesize
>> +
>> >> 0 *
>> >> >> >> > pixstep,
>> >> >> >> > -                               dstlinesize, outw, end - y);
>> >> >> >> > +            if (end - y > 0)
>> >> >> >> > +                v->transpose_block(src + 0 * srclinesize + y *
>> >> >> pixstep,
>> >> >> >> > +                                srclinesize,
>> >> >> >> > +                                dst + (y - start) *
>> dstlinesize +
>> >> 0
>> >> >> >> > *
>> >> >> >> > pixstep,
>> >> >> >> > +                                dstlinesize, outw, end - y);
>> >> >> >> > +        }
>> >> >> >> >      }
>> >> >> >> >
>> >> >> >> >      return 0;
>> >> >> >> > @@ -334,7 +485,7 @@ static int filter_frame(AVFilterLink
>> *inlink,
>> >> >> >> > AVFrame
>> >> >> >> > *in)
>> >> >> >> >      ThreadData td;
>> >> >> >> >      AVFrame *out;
>> >> >> >> >
>> >> >> >> > -    if (s->passthrough)
>> >> >> >> > +    if (s->passthrough || s->orientation == 1)
>> >> >> >> >          return ff_filter_frame(outlink, in);
>> >> >> >> >
>> >> >> >> >      out = ff_get_video_buffer(outlink, outlink->w,
>> >> >> >> > outlink->h);
>> >> >> >> > @@ -347,8 +498,13 @@ static int filter_frame(AVFilterLink
>> *inlink,
>> >> >> >> AVFrame
>> >> >> >> > *in)
>> >> >> >> >      if (in->sample_aspect_ratio.num == 0) {
>> >> >> >> >          out->sample_aspect_ratio = in->sample_aspect_ratio;
>> >> >> >> >      } else {
>> >> >> >> > -        out->sample_aspect_ratio.num =
>> >> in->sample_aspect_ratio.den;
>> >> >> >> > -        out->sample_aspect_ratio.den =
>> >> in->sample_aspect_ratio.num;
>> >> >> >> > +        if (s->orientation > 0 && s->orientation < 5) {
>> >> >> >> > +            out->sample_aspect_ratio.num =
>> >> >> in->sample_aspect_ratio.num;
>> >> >> >> > +            out->sample_aspect_ratio.den =
>> >> >> in->sample_aspect_ratio.den;
>> >> >> >> > +        } else {
>> >> >> >> > +            out->sample_aspect_ratio.num =
>> >> >> in->sample_aspect_ratio.den;
>> >> >> >> > +            out->sample_aspect_ratio.den =
>> >> >> in->sample_aspect_ratio.num;
>> >> >> >> > +        }
>> >> >> >> >      }
>> >> >> >> >
>> >> >> >> >      td.in = in, td.out = out;
>> >> >> >> > @@ -366,13 +522,13 @@ static const AVOption transpose_options[]
>> =
>> >> >> >> > {
>> >> >> >> >          { "clock",       "rotate clockwise",
>> >> >> >> 0,
>> >> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       },
>> .flags=FLAGS,
>> >> >> .unit
>> >> >> >> =
>> >> >> >> > "dir" },
>> >> >> >> >          { "cclock",      "rotate counter-clockwise",
>> >> >> >> 0,
>> >> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      },
>> .flags=FLAGS,
>> >> >> .unit
>> >> >> >> =
>> >> >> >> > "dir" },
>> >> >> >> >          { "clock_flip",  "rotate clockwise with vertical
>> >> >> >> > flip",
>> >> >> >>  0,
>> >> >> >> > AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  },
>> .flags=FLAGS,
>> >> >> .unit
>> >> >> >> =
>> >> >> >> > "dir" },
>> >> >> >> > -
>> >> >> >> > +
>> >> >> >> >      { "passthrough", "do not apply transposition if the input
>> >> >> >> > matches
>> >> >> >> the
>> >> >> >> > specified geometry",
>> >> >> >> >        OFFSET(passthrough), AV_OPT_TYPE_INT,
>> >> >> >> {.i64=TRANSPOSE_PT_TYPE_NONE},
>> >> >> >> > 0, INT_MAX, FLAGS, "passthrough" },
>> >> >> >> >          { "none",      "always apply transposition",   0,
>> >> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN,
>> >> >> INT_MAX,
>> >> >> >> > FLAGS, "passthrough" },
>> >> >> >> >          { "portrait",  "preserve portrait geometry",   0,
>> >> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN,
>> >> >> INT_MAX,
>> >> >> >> > FLAGS, "passthrough" },
>> >> >> >> >          { "landscape", "preserve landscape geometry",  0,
>> >> >> >> > AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN,
>> >> >> INT_MAX,
>> >> >> >> > FLAGS, "passthrough" },
>> >> >> >> > -
>> >> >> >> > +    { "orientation", "set exif orientation",
>> OFFSET(orientation),
>> >> >> >> > AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
>> >> >> >> >      { NULL }
>> >> >> >> >  };
>> >> >> >> >
>> >> >> >> > --
>> >> >> >> > 2.17.1
>> >> >> >> >
>> >> >> >> > _______________________________________________
>> >> >> >> > ffmpeg-devel mailing list
>> >> >> >> > ffmpeg-devel@ffmpeg.org
>> >> >> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >> >> >
>> >> >> >> > To unsubscribe, visit link above, or email
>> >> >> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> >> >> _______________________________________________
>> >> >> >> ffmpeg-devel mailing list
>> >> >> >> ffmpeg-devel@ffmpeg.org
>> >> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >> >>
>> >> >> >> To unsubscribe, visit link above, or email
>> >> >> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> >> > _______________________________________________
>> >> >> > ffmpeg-devel mailing list
>> >> >> > ffmpeg-devel@ffmpeg.org
>> >> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >> >
>> >> >> > To unsubscribe, visit link above, or email
>> >> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> >> _______________________________________________
>> >> >> ffmpeg-devel mailing list
>> >> >> ffmpeg-devel@ffmpeg.org
>> >> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >>
>> >> >> To unsubscribe, visit link above, or email
>> >> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> > _______________________________________________
>> >> > ffmpeg-devel mailing list
>> >> > ffmpeg-devel@ffmpeg.org
>> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> >
>> >> > To unsubscribe, visit link above, or email
>> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> >> _______________________________________________
>> >> ffmpeg-devel mailing list
>> >> ffmpeg-devel@ffmpeg.org
>> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >>
>> >> To unsubscribe, visit link above, or email
>> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> > To unsubscribe, visit link above, or email
>> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Nicolas George May 27, 2019, 2:52 p.m. UTC | #12
Jun Li (12019-05-27):
> I compared the perf between vflip and this patch before, they take almost
> the same time, around 9ms for a frame.
> Hflip is a little special, but surely I can update the patch to avoid frame
> copy, and it will be as fast as hflip.
> 
> But still this is duplicated feature with vflip/hflip, the only benefit I
> can think of is rotate180.
> Previously we need chain two filter (vflip + hflip) to do rotate180, here
> we only need one.

Please note that there are two separate issues interacting here: having
separate filters, having separate code.

Merging the features of a filter into a more generic one is, IMHO, a
good thing. In this instance, it makes the rest of the code simpler.

But that does not mean you have to duplicate the code. You should almost
never duplicate the code. What you should do is make sure the two
filters are using the same code, i.e. calling the same functions to do
the actual work.

Regards,
Jun Li May 28, 2019, 6:25 a.m. UTC | #13
On Mon, May 27, 2019 at 7:52 AM Nicolas George <george@nsup.org> wrote:

> Jun Li (12019-05-27):
> > I compared the perf between vflip and this patch before, they take almost
> > the same time, around 9ms for a frame.
> > Hflip is a little special, but surely I can update the patch to avoid
> frame
> > copy, and it will be as fast as hflip.
> >
> > But still this is duplicated feature with vflip/hflip, the only benefit I
> > can think of is rotate180.
> > Previously we need chain two filter (vflip + hflip) to do rotate180, here
> > we only need one.
>
> Please note that there are two separate issues interacting here: having
> separate filters, having separate code.
>
> Merging the features of a filter into a more generic one is, IMHO, a
> good thing. In this instance, it makes the rest of the code simpler.
>
> But that does not mean you have to duplicate the code. You should almost
> never duplicate the code. What you should do is make sure the two
> filters are using the same code, i.e. calling the same functions to do
> the actual work.
>
>
Thanks for pointing that out. I updated the version and merged the existing
flip operation into transpose, removed my previous implementation.
https://patchwork.ffmpeg.org/patch/13311/
https://patchwork.ffmpeg.org/patch/13312/

Thanks a lot for your code review.

Best Regard,
Jun

Regards,
>
> --
>   Nicolas George
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox

Patch

diff --git a/libavfilter/vf_transpose.c b/libavfilter/vf_transpose.c
index dd54947bd9..4aebfb9ee4 100644
--- a/libavfilter/vf_transpose.c
+++ b/libavfilter/vf_transpose.c
@@ -46,6 +46,9 @@  typedef struct TransVtable {
     void (*transpose_block)(uint8_t *src, ptrdiff_t src_linesize,
                             uint8_t *dst, ptrdiff_t dst_linesize,
                             int w, int h);
+    void (*copyline_block)(uint8_t *src, ptrdiff_t src_linesize,
+                           uint8_t *dst, ptrdiff_t dst_linesize,
+                           int line_dir, int w, int h);
 } TransVtable;
 
 typedef struct TransContext {
@@ -56,7 +59,7 @@  typedef struct TransContext {
 
     int passthrough;    ///< PassthroughType, landscape passthrough mode enabled
     int dir;            ///< TransposeDir
-
+    int orientation;    ///< Orientation
     TransVtable vtables[4];
 } TransContext;
 
@@ -182,6 +185,105 @@  static void transpose_8x8_64_c(uint8_t *src, ptrdiff_t src_linesize,
     transpose_block_64_c(src, src_linesize, dst, dst_linesize, 8, 8);
 }
 
+
+static void copyline_block_8(uint8_t *src, ptrdiff_t src_linesize,
+                             uint8_t *dst, ptrdiff_t dst_linesize,
+                             int line_dir, int w, int h)
+{
+    int x, y, i;
+    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
+        for (x = 0; x < w; x ++) {
+            i = line_dir < 0 ? w-x-1 : x;
+            dst[i] = src[x];
+        }
+    }
+}
+
+static void copyline_block_16(uint8_t *src, ptrdiff_t src_linesize,
+                              uint8_t *dst, ptrdiff_t dst_linesize,
+                              int line_dir, int w, int h)
+{
+    int x, y, i;
+    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
+        for (x = 0; x < w; x ++) {
+            i = line_dir < 0 ? w-x-1 : x;
+            *((uint16_t *)(dst + 2*i)) = *((uint16_t *)(src + 2*x));
+        }
+    }
+}
+
+static void copyline_block_24(uint8_t *src, ptrdiff_t src_linesize,
+                              uint8_t *dst, ptrdiff_t dst_linesize,
+                              int line_dir, int w, int h)
+{
+    int x, y, i;
+    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
+        for (x = 0; x < w; x++) {
+            int32_t v = AV_RB24(src + 3*x);
+            i = line_dir < 0 ? w-x-1 : x;
+            AV_WB24(dst + 3*i, v);
+        }
+    }
+}
+
+static void copyline_block_32(uint8_t *src, ptrdiff_t src_linesize,
+                              uint8_t *dst, ptrdiff_t dst_linesize,
+                              int line_dir, int w, int h)
+{
+    int x, y, i;
+    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
+        for (x = 0; x < w; x ++) {
+            i = line_dir < 0 ? w-x-1 : x;
+            *((uint32_t *)(dst + 4*i)) = *((uint32_t *)(src + 4*x));
+        }
+    }
+}
+
+static void copyline_block_48(uint8_t *src, ptrdiff_t src_linesize,
+                              uint8_t *dst, ptrdiff_t dst_linesize,
+                              int line_dir, int w, int h)
+{
+    int x, y, i;
+    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
+        for (x = 0; x < w; x++) {
+            int64_t v = AV_RB48(src + 6*x);
+            i = line_dir < 0 ? w-x-1 : x;
+            AV_WB48(dst + 6*i, v);
+        }
+    }
+}
+
+static void copyline_block_64(uint8_t *src, ptrdiff_t src_linesize,
+                              uint8_t *dst, ptrdiff_t dst_linesize,
+                              int line_dir, int w, int h)
+{
+    int x, y, i;
+    for (y = 0; y < h; y++, dst += dst_linesize, src += src_linesize) {
+        for (x = 0; x < w; x ++) {
+            i = line_dir < 0 ? w-x-1 : x;
+            *((uint64_t *)(dst + 8*i)) = *((uint64_t *)(src + 8*x));
+        }
+    }
+}
+
+static void set_outlink_width_height(AVFilterLink *inlink, AVFilterLink *outlink, int transpose)
+{
+    if (transpose) {
+        outlink->w = inlink->h;
+        outlink->h = inlink->w;
+
+        if (inlink->sample_aspect_ratio.num)
+            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
+                                                    inlink->sample_aspect_ratio);
+        else
+            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
+    } else {
+        outlink->w = inlink->w;
+        outlink->h = inlink->h;
+        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
+    }
+}
+
 static int config_props_output(AVFilterLink *outlink)
 {
     AVFilterContext *ctx = outlink->src;
@@ -213,33 +315,44 @@  static int config_props_output(AVFilterLink *outlink)
 
     av_assert0(desc_in->nb_components == desc_out->nb_components);
 
-
     av_image_fill_max_pixsteps(s->pixsteps, NULL, desc_out);
+    
+    if (s->orientation && s->orientation < 5) {
+        outlink->h = inlink->h;
+        outlink->w = inlink->w;
+        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio; 
+    } else {
+        outlink->w = inlink->h;
+        outlink->h = inlink->w;
 
-    outlink->w = inlink->h;
-    outlink->h = inlink->w;
-
-    if (inlink->sample_aspect_ratio.num)
-        outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
-                                                inlink->sample_aspect_ratio);
-    else
-        outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
+        if (inlink->sample_aspect_ratio.num)
+            outlink->sample_aspect_ratio = av_div_q((AVRational) { 1, 1 },
+                                                    inlink->sample_aspect_ratio);
+        else
+            outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
+    } 
 
     for (int i = 0; i < 4; i++) {
         TransVtable *v = &s->vtables[i];
         switch (s->pixsteps[i]) {
         case 1: v->transpose_block = transpose_block_8_c;
-                v->transpose_8x8   = transpose_8x8_8_c;  break;
+                v->transpose_8x8   = transpose_8x8_8_c;
+                v->copyline_block  = copyline_block_8; break;
         case 2: v->transpose_block = transpose_block_16_c;
-                v->transpose_8x8   = transpose_8x8_16_c; break;
+                v->transpose_8x8   = transpose_8x8_16_c;
+                v->copyline_block  = copyline_block_16; break;
         case 3: v->transpose_block = transpose_block_24_c;
-                v->transpose_8x8   = transpose_8x8_24_c; break;
+                v->transpose_8x8   = transpose_8x8_24_c;
+                v->copyline_block  = copyline_block_24; break;
         case 4: v->transpose_block = transpose_block_32_c;
-                v->transpose_8x8   = transpose_8x8_32_c; break;
+                v->transpose_8x8   = transpose_8x8_32_c;
+                v->copyline_block  = copyline_block_32; break;
         case 6: v->transpose_block = transpose_block_48_c;
-                v->transpose_8x8   = transpose_8x8_48_c; break;
+                v->transpose_8x8   = transpose_8x8_48_c;
+                v->copyline_block  = copyline_block_48; break;
         case 8: v->transpose_block = transpose_block_64_c;
-                v->transpose_8x8   = transpose_8x8_64_c; break;
+                v->transpose_8x8   = transpose_8x8_64_c;
+                v->copyline_block  = copyline_block_64; break;
         }
     }
 
@@ -285,42 +398,80 @@  static int filter_slice(AVFilterContext *ctx, void *arg, int jobnr,
         uint8_t *dst, *src;
         int dstlinesize, srclinesize;
         int x, y;
+        int dir = s->dir;
         TransVtable *v = &s->vtables[plane];
+        
+        if (s->orientation > 0 && s->orientation < 5) {
+            int line_dir = 1;
+            dstlinesize = out->linesize[plane];
+            dst         = out->data[plane] + start * dstlinesize;
+            srclinesize = in->linesize[plane];
+            src         = in->data[plane] + start * srclinesize;
+
+            switch (s->orientation) {
+                case 2: // mirror horizontal
+                    line_dir     = -1;
+                    break;
+                case 3: // rotate 180
+                    dst          = out->data[plane] + (outh - start - 1) * dstlinesize;
+                    line_dir     = -1;
+                    dstlinesize *= -1;
+                    break;
+                case 4: // mirror vertical
+                    dst          = out->data[plane] + (outh - start - 1) * dstlinesize;
+                    dstlinesize *= -1;
+                    break;
+                default:
+                    break;
+            }
+            v->copyline_block(src, srclinesize, dst, dstlinesize, line_dir, outw, end - start);
+        } else {
+            dstlinesize = out->linesize[plane];
+            dst         = out->data[plane] + start * dstlinesize;
+            src         = in->data[plane];
+            srclinesize = in->linesize[plane];
+            switch (s->orientation) {
+                case 5: // mirror horizontal and rotate 270 CW
+                    dir = 0; break;
+                case 6: // rotate 90 CW
+                    dir = 1; break;
+                case 7: // mirror horizontal and rotate 90 CW
+                    dir = 3; break;
+                case 8: // rotate 270 CW
+                    dir = 2; break;
+                default: break;
+            }
 
-        dstlinesize = out->linesize[plane];
-        dst         = out->data[plane] + start * dstlinesize;
-        src         = in->data[plane];
-        srclinesize = in->linesize[plane];
-
-        if (s->dir & 1) {
-            src         += in->linesize[plane] * (inh - 1);
-            srclinesize *= -1;
-        }
+            if (dir & 1) {
+                src         += in->linesize[plane] * (inh - 1);
+                srclinesize *= -1;
+            }
 
-        if (s->dir & 2) {
-            dst          = out->data[plane] + dstlinesize * (outh - start - 1);
-            dstlinesize *= -1;
-        }
+            if (dir & 2) {
+                dst          = out->data[plane] + dstlinesize * (outh - start - 1);
+                dstlinesize *= -1;
+            }
 
-        for (y = start; y < end - 7; y += 8) {
-            for (x = 0; x < outw - 7; x += 8) {
-                v->transpose_8x8(src + x * srclinesize + y * pixstep,
-                                 srclinesize,
-                                 dst + (y - start) * dstlinesize + x * pixstep,
-                                 dstlinesize);
+            for (y = start; y < end - 7; y += 8) {
+                for (x = 0; x < outw - 7; x += 8) {
+                    v->transpose_8x8(src + x * srclinesize + y * pixstep,
+                                    srclinesize,
+                                    dst + (y - start) * dstlinesize + x * pixstep,
+                                    dstlinesize);
+                }
+                if (outw - x > 0 && end - y > 0)
+                    v->transpose_block(src + x * srclinesize + y * pixstep,
+                                    srclinesize,
+                                    dst + (y - start) * dstlinesize + x * pixstep,
+                                    dstlinesize, outw - x, end - y);
             }
-            if (outw - x > 0 && end - y > 0)
-                v->transpose_block(src + x * srclinesize + y * pixstep,
-                                   srclinesize,
-                                   dst + (y - start) * dstlinesize + x * pixstep,
-                                   dstlinesize, outw - x, end - y);
-        }
 
-        if (end - y > 0)
-            v->transpose_block(src + 0 * srclinesize + y * pixstep,
-                               srclinesize,
-                               dst + (y - start) * dstlinesize + 0 * pixstep,
-                               dstlinesize, outw, end - y);
+            if (end - y > 0)
+                v->transpose_block(src + 0 * srclinesize + y * pixstep,
+                                srclinesize,
+                                dst + (y - start) * dstlinesize + 0 * pixstep,
+                                dstlinesize, outw, end - y);
+        }
     }
 
     return 0;
@@ -334,7 +485,7 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     ThreadData td;
     AVFrame *out;
 
-    if (s->passthrough)
+    if (s->passthrough || s->orientation == 1)
         return ff_filter_frame(outlink, in);
 
     out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
@@ -347,8 +498,13 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
     if (in->sample_aspect_ratio.num == 0) {
         out->sample_aspect_ratio = in->sample_aspect_ratio;
     } else {
-        out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
-        out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
+        if (s->orientation > 0 && s->orientation < 5) {
+            out->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
+            out->sample_aspect_ratio.den = in->sample_aspect_ratio.den;   
+        } else {
+            out->sample_aspect_ratio.num = in->sample_aspect_ratio.den;
+            out->sample_aspect_ratio.den = in->sample_aspect_ratio.num;
+        }
     }
 
     td.in = in, td.out = out;
@@ -366,13 +522,13 @@  static const AVOption transpose_options[] = {
         { "clock",       "rotate clockwise",                            0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK       }, .flags=FLAGS, .unit = "dir" },
         { "cclock",      "rotate counter-clockwise",                    0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CCLOCK      }, .flags=FLAGS, .unit = "dir" },
         { "clock_flip",  "rotate clockwise with vertical flip",         0, AV_OPT_TYPE_CONST, { .i64 = TRANSPOSE_CLOCK_FLIP  }, .flags=FLAGS, .unit = "dir" },
-
+        
     { "passthrough", "do not apply transposition if the input matches the specified geometry",
       OFFSET(passthrough), AV_OPT_TYPE_INT, {.i64=TRANSPOSE_PT_TYPE_NONE},  0, INT_MAX, FLAGS, "passthrough" },
         { "none",      "always apply transposition",   0, AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_NONE},      INT_MIN, INT_MAX, FLAGS, "passthrough" },
         { "portrait",  "preserve portrait geometry",   0, AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_PORTRAIT},  INT_MIN, INT_MAX, FLAGS, "passthrough" },
         { "landscape", "preserve landscape geometry",  0, AV_OPT_TYPE_CONST, {.i64=TRANSPOSE_PT_TYPE_LANDSCAPE}, INT_MIN, INT_MAX, FLAGS, "passthrough" },
-
+    { "orientation", "set exif orientation", OFFSET(orientation), AV_OPT_TYPE_INT, {.i64 = 0 },  0, 8, FLAGS, "orientation" },
     { NULL }
 };