For commit title please try using the XXX: YYY
eg. mov: support parting the rotation element

On Tue, Dec 3, 2013 at 3:56 PM, André Cruz <[email protected]> wrote:
> Some devices include metadata in the video that indicate the orientation of 
> the
> video. Apple devices are an example of that.
>
> This code was lifted from ffmpeg and includes the "rotate" element in the 
> video
> metadata.
>
> There were several commits that touched the relevant code, but as this touched
> other areas as well I could not use those commits as is:
>
> d3cef0a85
> e6ba3d428
> 62d2a75b0
>
> ---

This message should not go in the commit log, but are fine for
discussion in an email.
Since the code comes from another project the hashes will not work
here; it's better to write something like
"Patch based on code by author1 <email> and author3 <email>" (the
second commit you quote doesn't really apply for this patch).

>  libavformat/mov.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 9b019e1..6dae12d 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -2256,6 +2256,18 @@ static int mov_read_tkhd(MOVContext *c, AVIOContext 
> *pb, MOVAtom atom)
>      sc->width = width >> 16;
>      sc->height = height >> 16;
>
> +    //  Assign clockwise rotate values based on transform matrix so that
> +    //  we can compensate for iPhone orientation during capture.

extra space between comment

> +
> +    if (display_matrix[1][0] == -65536 && display_matrix[0][1] == 65536)
> +         av_dict_set(&st->metadata, "rotate", "90", 0);
> +
> +    if (display_matrix[0][0] == -65536 && display_matrix[1][1] == -65536)
> +         av_dict_set(&st->metadata, "rotate", "180", 0);
> +
> +    if (display_matrix[1][0] == 65536 && display_matrix[0][1] == -65536)
> +         av_dict_set(&st->metadata, "rotate", "270", 0);
> +
>      // transform the display width/height according to the matrix
>      // skip this if the display matrix is the default identity matrix
>      // or if it is rotating the picture, ex iPhone 3GS

The values here correspond to the standard (and quickly checked from
here too 
http://stackoverflow.com/questions/7025186/how-to-remove-or-edit-exif-from-mp4-video)
so this part is ok. The only thing I do not like is that the metadata
name is a verb instead of a noun (like all other metadata) so I would
recommend switching to a simple "rotation" name. Any opinion on this?

Cheers,
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to