[vlc-devel] [PATCH] Support for rotated MP4 Movies (bug #2882)

[Rémi Denis-Courmont]

Le vendredi 7 février 2014, 23:01:40 Matthias Keiser a écrit :
> Am 07.02.2014 um 12:59 schrieb Rémi Denis-Courmont <remi at remlab.net>:
> > On Thu, 6 Feb 2014 23:21:07 +0100, Matthias Keiser
> > 
> > <matthias at tristan-inc.com> wrote:
> >> From 0835ce77e00f83d0e15dd73e3bf28661f44e12cf Mon Sep 17 00:00:00 2001
> >> From: Matthias Keiser <matthias at tristan-inc.com>
> >> Date: Thu, 6 Feb 2014 16:22:49 +0100
> >> Subject: [PATCH 10/13] If the video formats orientation is rotated and
> > 
> > the
> > 
> >> video display is not using opengl, insert a transform filter that
> > 
> > rotates
> > 
> >> the
> >> picture.
> >> 
> >> ---
> >> src/video_output/video_output.c | 37
> > 
> > +++++++++++++++++++++++++++++++++++++
> > 
> >> 1 file changed, 37 insertions(+)
> >> 
> >> diff --git a/src/video_output/video_output.c
> >> b/src/video_output/video_output.c
> >> index ede3520..76d3c85 100644
> >> --- a/src/video_output/video_output.c
> >> +++ b/src/video_output/video_output.c
> >> @@ -743,6 +743,43 @@ static void ThreadChangeFilters(vout_thread_t
> > 
> > *vout,
> > 
> >>     vlc_array_init(&array_static);
> >>     vlc_array_init(&array_interactive);
> >> 
> >> +
> >> +    if (!vout_GetDisplayOpengl(vout->p->display.vd) &&
> >> vout->p->filter.format.orientation != ORIENT_TOP_LEFT) {
> > 
> > This precludes any other output from ever accelerating rotation. Yet
> > Direct3D, VDPAU and probably others could definitely support it... This is
> > unacceptable w.r.t. VDPAU, since the transform filter does not work with
> > it.
> 
> So we probably should use the API proposed by Jean-Baptiste Kempf:
> > Why not ask the Video Output if it can rotate by itself, like a
> > VOUT_DISPLAY_CAN_ROTATE, that would be defaulted to false?
> 
> Ok?

It depends what you mean by that, but it seems like a catch-22 non-starter.

> > I think you should only compare the video formats.
> 
> Sorry I don't understand what you mean.

You need to compare the source video format with the video display format, as 
is done with the other video format parameters.

> >> +        char *angle = NULL;
> >> +
> >> +        switch (vout->p->filter.format.orientation) {
> >> +
> >> +            case ORIENT_ROTATED_90:
> >> +                angle = "270";
> >> +                break;
> >> +            case ORIENT_ROTATED_180:
> >> +                angle = "180";
> >> +                break;
> >> +            case ORIENT_ROTATED_270:
> >> +                angle = "90";
> >> +                break;
> >> +            default:
> >> +                angle = NULL;
> >> +                break;
> > 
> > Missing the indirect orientations.
> 
> I take it this means the other orientations like ORIENT_HFLIPPED. Since the
> openGL stuff doesn't handle this either, I seems natural to restrict the
> handling to the "normal" rotation cases, no?

OpenGL is irrelevant. The transform filter can deal with them and I see no 
reasons to remove the feature. Besides, flipping the video around the 
horizontal axis in CPU is lossless and supported for all chromas (since it 
just involves swapping scanlines).

> Also I think none of the
> demuxers currently passes these kinds of orientations.

The enumeration is ripped from the EXIF standard. MP4 can code a lot more 
possibilities.

> >> +        }
> >> +
> >> +        if (angle) {
> >> +
> >> +            config_chain_t *cfg;
> >> +            char *name;
> >> +            char config[30];
> >> +            snprintf(config, 30, "transform{type=%s}", angle);
> >> +            char *next;
> >> +            VLC_UNUSED(next);
> >> +            next = config_ChainCreate(&name, &cfg, config);
> > 
> > Memory leaks.
> 
> Sorry, I can't spot it. cfg is destroyed on failure, else I think the filter
> chain takes ownership, no? name and e are freed on line 830/831.

In any case, you should not do this. Splitting conversions is nominally the 
task of the chain video filter, not the video output core.

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