After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 679522 - isomp4: Export transformation matrix
isomp4: Export transformation matrix
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 596326 687078 706649 728534 (view as bug list)
Depends on: 738914
Blocks:
 
 
Reported: 2012-07-06 15:43 UTC by Bastien Nocera
Modified: 2018-11-03 14:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Parse transformation matrix in TKHD atom (1.61 KB, patch)
2013-01-11 17:24 UTC, Arnaud Vrac
reviewed Details | Review
qtdemux: parse tkhd transformation matrix and add tags if appropriate (5.85 KB, patch)
2014-05-24 14:23 UTC, Thiago Sousa Santos
committed Details | Review
patch to write the rotation transformation matrix (3.91 KB, patch)
2015-03-02 23:05 UTC, Mohammed Sameer
none Details | Review
patch to write transformation matrix for rotation cases V2 (3.89 KB, text/plain)
2015-03-06 12:05 UTC, Mohammed Sameer
  Details
patch to write transformation matrix for rotation cases V3 (3.86 KB, patch)
2015-03-23 08:23 UTC, Mohammed Sameer
none Details | Review

Description Bastien Nocera 2012-07-06 15:43:56 UTC
http://people.gnome.org/~hadess/IMG_0205.MOV

QuickTime can export a tranformation matrix in the mvhd "Movie Header" atom.

See:
http://developer.apple.com/library/mac/#documentation/QuickTime/QTFF/QTFFChap2/qtff2.html#//apple_ref/doc/uid/TP40000939-CH204-BBCBEAIF

This could be exported, and naively interpreted by movie players to rotate the video.
Comment 1 Bastien Nocera 2012-10-29 09:29:31 UTC
*** Bug 687078 has been marked as a duplicate of this bug. ***
Comment 3 Arnaud Vrac 2013-01-11 17:24:25 UTC
Created attachment 233245 [details] [review]
Parse transformation matrix in TKHD atom

Here is a patch to parse the transformation matrix in the TKHD atom (not movie header).

I have another patch to send the transformation matrix downstream as a custom sticky event but it probably isn't the right way to do it, a buffer meta is probably preferable.
Comment 4 Sebastian Dröge (slomo) 2013-08-21 18:18:39 UTC
*** Bug 596326 has been marked as a duplicate of this bug. ***
Comment 5 Bastien Nocera 2013-08-23 15:49:06 UTC
*** Bug 706649 has been marked as a duplicate of this bug. ***
Comment 6 Michael Doube 2013-08-26 10:05:33 UTC
Four more test videos in downstream bug report:

https://bugs.launchpad.net/gstreamer/+bug/697756

They show all 4 video orientations from an iPod.
Comment 7 Tim-Philipp Müller 2013-08-26 10:24:28 UTC
Thanks. Amusingly enough I've seen this being an issue on Mac OS X as well recently when playing videos recorded with an ipod, we had to rotate the laptop to watch it in the right orientation (with quicktime).
Comment 8 Sebastian Dröge (slomo) 2013-11-03 09:53:50 UTC
For some of the rotations at least we could export that as the image-orientation tag. For generic support for all transformation matrizes we need some new API.
Comment 9 Mohammed Sameer 2013-11-07 11:55:15 UTC
The more I think about the matrix the more I realize that we should not try to hide it.

I think we should export the matrix as it is (like we do with codec_data for example).

Each sink can then act on the matrix as it sees fit.

Adding a buffer meta would do also but the only field it will carry is going to be the matrix itself since the transformation might be scaling, rotation and translation and we should not drop the rest and export the rotation only.
Comment 10 Sebastian Dröge (slomo) 2013-11-11 14:07:19 UTC
It should be added as a buffer meta to be negotiable, and also to be able to carry this over easily from encoded to raw video. The buffer meta should contain the raw matrix, but also hints about what the matrix is doing to make it easier to negotiate this. I.e. a flags field that has flags for rotation, scaling, translation, and others.
Comment 11 Andoni Morales 2013-11-11 16:46:04 UTC
This transformation matrix is also present in Android's SurfaceTexture, so I'd say it's a common use case that should be supported.
Comment 12 Sebastian Dröge (slomo) 2014-05-22 07:04:00 UTC
*** Bug 728534 has been marked as a duplicate of this bug. ***
Comment 13 Nicolas Dufresne (ndufresne) 2014-05-22 13:47:58 UTC
I'd like to see for well known matrix, the orientation tag being set (unless there is a problem doing so).
Comment 14 Bastien Nocera 2014-05-22 14:00:12 UTC
(In reply to comment #13)
> I'd like to see for well known matrix, the orientation tag being set (unless
> there is a problem doing so).

That's what Sebastian said in comment 8, and what I would need to implement automatic rotation in Totem too.
Comment 15 Thiago Sousa Santos 2014-05-24 14:23:05 UTC
Created attachment 277113 [details] [review]
qtdemux: parse tkhd transformation matrix and add tags if appropriate

This patch handles the transformation matrix using tags for the 90, 180 and 280
rotation cases. Those are very common with mobile devices recording. It is more
complete and has safety fixes on top of the current patch attached here.

I'm leaving this here as a partial solution to handle the most common cases, but
we should think on a way to organize metas to handle rotation/transformation/positioning
(or the transformation matrix as a single meta?). From my previous discussions on IRC one
issue was how to define the order of metas to be applied to avoid unexpected results like
subtitles being flipped together with the video.
Comment 16 Thiago Sousa Santos 2014-05-24 14:24:36 UTC
For testing just use gst-launch-1.0 playbin uri="" video-sink="videoflip method=automatic ! autovideosink"
Comment 17 Nicolas Dufresne (ndufresne) 2014-05-24 19:02:00 UTC
Just tried, great work, btw, in 1.3 you can also use the new video-filter property:

gst-launch-1.0 playbin uri=.. video-filter="videoflip method=automatic"
Comment 18 Nicolas Dufresne (ndufresne) 2014-05-25 00:08:31 UTC
Comment on attachment 277113 [details] [review]
qtdemux: parse tkhd transformation matrix and add tags if appropriate

Accidental commit, but I didn't revert as this all look good. It's also cleaner then the first proposed parsing.

commit d423b9f63ebdb0988af9b975d419b354175cf215
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Sat May 24 11:16:35 2014 -0300

    qtdemux: parse tkhd transformation matrix and add tags if appropriate
    
    Handle the transformation matrix cases where there are only simple rotations
    (90, 180 or 270 degrees) and use a tag for those cases. This is a common scenario
    when recording with mobile devices
    
    https://bugzilla.gnome.org/show_bug.cgi?id=679522
Comment 19 Nicolas Dufresne (ndufresne) 2014-05-25 00:11:33 UTC
Comment on attachment 233245 [details] [review]
Parse transformation matrix in TKHD atom

Thanks for your patch, the other was taken, since it added required feature to couple with videoflip, but I'm not marking obsolete, as I see you have conversion code to float, which might be needed.
Comment 20 Bastien Nocera 2014-05-30 14:27:46 UTC
And I've implemented automatic rotation of videos based on video orientation in totem. Works great, thanks.
Comment 21 Tim-Philipp Müller 2014-05-30 14:29:38 UTC
> And I've implemented automatic rotation of videos based on video orientation in
> totem. Works great, thanks.

So when cluttersink also gets support for that, will it get rotated twice? :)
Comment 22 Bastien Nocera 2014-05-30 14:31:40 UTC
(In reply to comment #21)
> > And I've implemented automatic rotation of videos based on video orientation in
> > totem. Works great, thanks.
> 
> So when cluttersink also gets support for that, will it get rotated twice? :)

I don't think it's cluttersink's business to be doing that. If the user has rotated the video to their liking, I wouldn't expect the video to rotate without user interaction.
Comment 23 Nicolas Dufresne (ndufresne) 2014-05-30 15:37:17 UTC
I think I forgot in videoflip, but if I change the rotation, I should update the rotation tag accordingly. Cluttersink/Coglsink would have to do that same if they want to handle it auto-magically. This way we don't mess with application that wants to handle it (and don't break Bastiaen work, just disable it). In the future, elements that handle transformation matrix, would have to reset the matrix too. The only risk here is that we might do-undo a transformation if an app want to display in some other orientation.
Comment 24 Nicolas Dufresne (ndufresne) 2014-05-30 15:37:56 UTC
(I mean the orientation tag, sorry)
Comment 25 Nicolas Dufresne (ndufresne) 2014-05-30 15:43:48 UTC
Quick question, what do we do about overlay composition. Shall we always try to render then straight ? Right now we seem to do nothing, so if text is overlayed in soft on the video stream, they'll be presented upside-down in Totem.

I suppose you rotate the actor, so even if cluttersink was doing this composition, these would be childs that also get rotated right ?
Comment 26 Sebastian Dröge (slomo) 2014-05-31 08:55:23 UTC
(In reply to comment #25)
> Quick question, what do we do about overlay composition. Shall we always try to
> render then straight ? Right now we seem to do nothing, so if text is overlayed
> in soft on the video stream, they'll be presented upside-down in Totem.
> 
> I suppose you rotate the actor, so even if cluttersink was doing this
> composition, these would be childs that also get rotated right ?

That's IMHO a problem of cluttersink. If the overlay composition needs to be rotated or not should be independent of the transformation matrix of the video (it should have its own metadata for the orientation).
Comment 27 Nicolas Dufresne (ndufresne) 2014-05-31 12:43:02 UTC
(In reply to comment #26)
> That's IMHO a problem of cluttersink. If the overlay composition needs to be
> rotated or not should be independent of the transformation matrix of the video
> (it should have its own metadata for the orientation).

I'm not convinced by this. I think sub-titles should render in the same direction the video is. I think there is a relation to make. If the image is recorded 90 degree, hence has a transformation (right now an orientation tag), the renderer of the overlay should take this into account. Otherwise you'll never get these straight and readable, and most likely not at the centre and slightly outside the screen. Though, I agree that if offloading though composition meta, it might be more flexible to make it so it have its own transformation (99% of the time will be a copy of the original). Though, right now we don't have a transformation matrix meta, we have a tag, which apply to all buffers, including the buffer in the composition meta. That would be my interpretation.
Comment 28 Sebastian Dröge (slomo) 2014-05-31 13:05:04 UTC
However that would mean that the subtitle renderers would need to rotate the subtitles if this tag is on the video stream. They would need to do that anyway if they render directly on the video instead of the overlay meta though.

I think this is all a bit awkward.
Comment 29 Andoni Morales 2014-06-16 10:45:17 UTC
(In reply to comment #11)
> This transformation matrix is also present in Android's SurfaceTexture, so I'd
> say it's a common use case that should be supported.

As discussed in #731204, a new meta for affine transformations could be used to handle all the scenarios in which a transformation is needed, whether it's a simple rotation or a more complex one.
Comment 30 Mohammed Sameer 2015-03-02 23:05:19 UTC
Created attachment 298355 [details] [review]
patch to write the rotation transformation matrix

Here is a patch that I have cooked that allows writing the rotation matrix into the TKHD atom. It handles only the simple cases (0, 90, 180 and 270).
Comment 31 Mohammed Sameer 2015-03-06 12:05:41 UTC
Created attachment 298700 [details]
patch to write transformation matrix for rotation cases V2

The previous patch is broken. No idea how it worked for me initially.
This updated patch has proper matrix values for the rotation.
Comment 32 Mohammed Sameer 2015-03-23 08:23:01 UTC
Created attachment 300116 [details] [review]
patch to write transformation matrix for rotation cases V3

I screwed up the matrix values apparently. The attached patch works fine for me now.
Comment 33 GStreamer system administrator 2018-11-03 14:46:57 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/66.