| Message ID | CAELrAsqXNotjJzdK=3Z6ULF-0eiShoJeVzKvZMw+Toy35t_gnw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 16, 2019 at 4:34 PM Ted Meyer < tmathmeyer-at-google.com@ffmpeg.org> wrote: > Right now ffmpeg doesn't export a mirroring status when checking the > display matrix for rotation. > Here is an example video: https://files.tedm.io/flip.mp4 > -Ted > _______________________________________________ > 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". There is a patch but not merged into master, quality not guaranteed :) Hope this helps. https://patchwork.ffmpeg.org/patch/13130/
On Thu, May 16, 2019 at 9:32 PM Jun Li <junli1026@gmail.com> wrote: > On Thu, May 16, 2019 at 4:34 PM Ted Meyer < > tmathmeyer-at-google.com@ffmpeg.org> wrote: > > > Right now ffmpeg doesn't export a mirroring status when checking the > > display matrix for rotation. > > Here is an example video: https://files.tedm.io/flip.mp4 > > -Ted > > _______________________________________________ > > 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". > > > There is a patch but not merged into master, quality not guaranteed :) > Hope this helps. > https://patchwork.ffmpeg.org/patch/13130/ > hey i just noticed this patch + if (CONV_FP(m[0]) * CONV_FP(m[4]) < CONV_FP(m[1]) * CONV_FP(m[3])) {+ *hflip = 1;+ av_display_matrix_flip(m, 1, 0);+ } the long if is basically computing the determinant of the matrix, but you only need the fact whether it's positive or negative, you can discard the result so you can avoid converting to CONV_FP, and just cast to int64_t + return av_display_rotation_get(m); don't you need to set vertical flip only if det < 0 and rot = 180? beside that small point, this patch introduces an api that basically supersedes the normal av_display_rotation_get(), and does many more things, I'd be tempted to deprecate any other use, and what do you think? in that case you could just call it av_display_rotation_get2() like is tradition I can't find the other patches from the set to review, would you be able to send an updated version? thanks
On Thu, May 16, 2019 at 7:38 PM Vittorio Giovara <vittorio.giovara@gmail.com> wrote: > On Thu, May 16, 2019 at 9:32 PM Jun Li <junli1026@gmail.com> wrote: > > > On Thu, May 16, 2019 at 4:34 PM Ted Meyer < > > tmathmeyer-at-google.com@ffmpeg.org> wrote: > > > > > Right now ffmpeg doesn't export a mirroring status when checking the > > > display matrix for rotation. > > > Here is an example video: https://files.tedm.io/flip.mp4 > > > -Ted > > > _______________________________________________ > > > 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". > > > > > > There is a patch but not merged into master, quality not guaranteed :) > > Hope this helps. > > https://patchwork.ffmpeg.org/patch/13130/ > > > > hey i just noticed this patch > > + if (CONV_FP(m[0]) * CONV_FP(m[4]) < CONV_FP(m[1]) * > CONV_FP(m[3])) {+ *hflip = 1;+ av_display_matrix_flip(m, > 1, 0);+ } > > the long if is basically computing the determinant of the matrix, but you > only need the fact whether it's positive or negative, you can discard the > result so you can avoid converting to CONV_FP, and just cast to int64_t > + return av_display_rotation_get(m); > > don't you need to set vertical flip only if det < 0 and rot = 180? > I think it should be fine and not need to check. Because vlip could be represented as hflip+rotation, the cross product (Michael's brilliant idea) m[0] * m[4] - m[1]*[3] cannot reflect it is horizontal or vertical flip. It all depends how we want to represents the transform, it could be "rotation+hflip" or "rotation+vlip". Any combination of rotation+flip can be represented by either one. > beside that small point, this patch introduces an api that basically > supersedes the normal av_display_rotation_get(), and does many more things, > I'd be tempted to deprecate any other use, and what do you think? in that > case you could just call it av_display_rotation_get2() like is tradition I can't find the other patches from the set to review, would you be able to > send an updated version? > Thanks Vittorio, I updated the version as suggested: https://patchwork.ffmpeg.org/patch/13181/ The original patch includes two part, but I am going to abandon it after discussing with Nicolas. The patches were for per frame flip/mirror and rotation. Basically it checks every frame's meta-data and address flip or rotation or both. I think it also covers stream level flip, addressed in ffmpeg_filter.c (correct me if I am wrong). https://patchwork.ffmpeg.org/patch/13131/ https://patchwork.ffmpeg.org/patch/13130/ Best Regards, Jun > thanks > -- > Vittorio > _______________________________________________ > 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".
From 7121cecdd54f403cb857dbfd056ca22253d6948a Mon Sep 17 00:00:00 2001 From: Ted Meyer <tmathmeyer@chromium.org> Date: Thu, 16 May 2019 14:17:51 -0700 Subject: [PATCH 1/1] Export display matrix mirroring information MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit normally a display matrix will look like: [ cos(Θ), -sin(Θ), ...] [ sin(Θ), cos(Θ), ...] [ ..., ..., ...] but switching the signs can be used to imply a mirroring effect. If the cosine values are of a different sign, then there is a mirroring about the y-axis, and if the sine values are the same sign, but non-zero, then there is a mirroring about the x-axis. Signed-off-by: Ted Meyer <tmathmeyer@chromium.org> --- libavformat/mov.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libavformat/mov.c b/libavformat/mov.c index e40bcf3b86..b8748ee374 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -4560,6 +4560,16 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext *pb, MOVAtom atom) snprintf(rotate_buf, sizeof(rotate_buf), "%g", rotate); av_dict_set(&st->metadata, "rotate", rotate_buf, 0); } + + if (sc->display_matrix[0] != sc->display_matrix[4]) { + av_dict_set(&st->metadata, "mirror-y", "yes", 0); + } + + // non-mirrored matricies have differing signs - except 0 + if (sc->display_matrix[1] == sc->display_matrix[3] && + sc->display_matrix[1] != 0) { + av_dict_set(&st->metadata, "mirror-x", "yes", 0); + } #endif } -- 2.21.0.1020.gf2820cf01a-goog