[vlc-devel] [PATCH 1/6] MP4 rotation, 3rd attempt (bug #2882)

[Rémi Denis-Courmont]

Le jeudi 27 février 2014, 23:30:13 Matthias Keiser a écrit :
> This patch is the third attempt to support rotated MP4 movies. The patches
> of this series supersede all previous patches.
(...)

While it is always nice to have some context for a patch series, I don't think 
this belongs in the comment logs of the first patch.

> 
> Some previous discussion was here:
> 
> https://mailman.videolan.org/pipermail/vlc-devel/2014-February/096975.html
> 
> Some notes:
> 
> - the transform filter is now inserted into the vd->owner.sys.filter filter
> chain in the function VoutDisplayCreateRender - in my test no swscale
> filter was added to the chain except if chroma conversion are necessary -
> all vout modules that wish to correctly support rotation need to be updated
> (see [PATCH 6/6] for details)
> 
> There are still some bugs in vlc which make watching rotated movies not yet
> optimal:
> 
> - When VLC wants to create a window for a movie/image, but the window module
> returns a smaller one than requested (most likely because it was bigger
> than the screen), the window has the wrong a/r for the picture. This causes
> black bars to appear. While normally these bars are on the top/bottom and
> quite small, they are huge for 90/270 rotated movies. - With the macosx
> vout, when you then try to resize a window, the window suddenly *does* seem
> to get as big as originally requested, with all controls hidden far below
> the screen. - When you then try to make the window smaller, you can't make
> as small as it needs to be again.
> 
> These bugs are not directly relates to my patch set (I tested it with a
> appropriately sized png image).

We

> 
> ---
>  include/vlc_es.h     |  8 ++++++++
>  src/libvlccore.sym   |  1 +
>  src/misc/es_format.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
> 
> diff --git a/include/vlc_es.h b/include/vlc_es.h
> index 38d63da..eb52950 100644
> --- a/include/vlc_es.h
> +++ b/include/vlc_es.h
> @@ -153,6 +153,8 @@ typedef enum video_orientation_t
>      ORIENT_RIGHT_BOTTOM, /**< Anti-transposed */
> 
>      ORIENT_NORMAL      = ORIENT_TOP_LEFT,
> +    ORIENT_TRANSPOSED  = ORIENT_LEFT_TOP,
> +    ORIENT_ANTI_TRANSPOSED = ORIENT_RIGHT_BOTTOM,
>      ORIENT_HFLIPPED    = ORIENT_TOP_RIGHT,
>      ORIENT_VFLIPPED    = ORIENT_BOTTOM_LEFT,
>      ORIENT_ROTATED_180 = ORIENT_BOTTOM_RIGHT,
> @@ -265,6 +267,12 @@ VLC_API void video_format_CopyCrop( video_format_t *,
> const video_format_t * ); VLC_API void video_format_ScaleCropAr(
> video_format_t *, const video_format_t * );
> 
>  /**
> + * This function "normalizes" the formats orientation, by switching the a/r
> according to the orientation, + * producing a format whose orientation is
> ORIENT_NORMAL. It makes a shallow copy (pallette is not alloc'ed). + */
> +VLC_API void video_format_ApplyRotation(const video_format_t * restrict in,
> video_format_t * restrict out); +
> +/**
>   * This function will check if the first video format is similar
>   * to the second one.
>   */
> diff --git a/src/libvlccore.sym b/src/libvlccore.sym
> index 3858e43..3c47d85 100644
> --- a/src/libvlccore.sym
> +++ b/src/libvlccore.sym
> @@ -467,6 +467,7 @@ var_LocationParse
>  video_format_CopyCrop
>  video_format_ScaleCropAr
>  video_format_FixRgb
> +video_format_ApplyRotation
>  video_format_IsSimilar
>  video_format_Setup
>  video_format_Print
> diff --git a/src/misc/es_format.c b/src/misc/es_format.c
> index 8936414..b0b3858 100644
> --- a/src/misc/es_format.c
> +++ b/src/misc/es_format.c
> @@ -244,6 +244,54 @@ void video_format_ScaleCropAr( video_format_t *p_dst,
> const video_format_t *p_sr p_dst->i_sar_num, p_dst->i_sar_den, 65536);
>  }
> 
> +void video_format_ApplyRotation(const video_format_t * restrict in,
> video_format_t * restrict out) { +
> +    *out = *in;
> +
> +    if(ORIENT_IS_SWAP(in->orientation)) {
> +
> +        out->i_width = in->i_height;
> +        out->i_visible_width = in->i_visible_height;
> +        out->i_height = in->i_width;
> +        out->i_visible_height = in->i_visible_width;
> +        out->i_sar_num = in->i_sar_den;
> +        out->i_sar_den = in->i_sar_num;
> +    }
> +
> +    switch (in->orientation) {
> +
> +        case ORIENT_ROTATED_90:
> +            out->i_x_offset = in->i_height - in->i_visible_height -
> in->i_y_offset; +            out->i_y_offset = in->i_x_offset;
> +            break;
> +        case ORIENT_ROTATED_180:
> +            out->i_x_offset = in->i_width - in->i_visible_width -
> in->i_x_offset; +            out->i_y_offset = in->i_height -
> in->i_visible_height - in->i_y_offset; +            break;
> +        case ORIENT_ROTATED_270:
> +            out->i_x_offset = in->i_y_offset;
> +            out->i_y_offset = in->i_width - in->i_visible_width -
> in->i_x_offset; +            break;
> +        case ORIENT_HFLIPPED:
> +            out->i_x_offset = in->i_width - in->i_visible_width -
> in->i_x_offset; +            break;
> +        case ORIENT_VFLIPPED:
> +            out->i_y_offset = in->i_height - in->i_visible_height -
> in->i_y_offset; +            break;
> +        case ORIENT_TRANSPOSED:
> +            out->i_x_offset = in->i_y_offset;
> +            out->i_y_offset = in->i_x_offset;
> +            break;
> +        case ORIENT_ANTI_TRANSPOSED:
> +            out->i_x_offset = in->i_height - in->i_visible_height -
> in->i_y_offset; +            out->i_y_offset = in->i_width -
> in->i_visible_width - in->i_x_offset; +            break;
> +    }
> +
> +    out->orientation = ORIENT_NORMAL;
> +}

Aren't some initializers missing?

> +
> +
>  bool video_format_IsSimilar( const video_format_t *p_fmt1, const
> video_format_t *p_fmt2 ) {
>      video_format_t v1 = *p_fmt1;

-- 
Rémi Denis-Courmont
http://www.remlab.net/