[vlc-devel] [PATCH 3/6] Support for displaying rotated movies.

Rémi Denis-Courmont remi at remlab.net
Fri Feb 28 17:34:09 CET 2014


Le jeudi 27 février 2014, 23:34:52 Matthias Keiser a écrit :
> @@ -214,6 +221,10 @@ void vout_display_PlacePicture(vout_display_place_t
> *place, unsigned display_width;
>      unsigned display_height;
> 
> +    video_format_t sourceRot;
> +    video_format_t *p_sourceRot = &sourceRot;

Why do you need a new pointer? Also please try to keep the coding style of the 
function.

> +    video_format_ApplyRotation(source, p_sourceRot);
> +
>      if (cfg->is_display_filled) {
>          display_width  = cfg->display.width;
>          display_height = cfg->display.height;
> @@ -223,7 +234,7 @@ void vout_display_PlacePicture(vout_display_place_t
> *place, cfg_tmp.display.width  = 0;
>          cfg_tmp.display.height = 0;
>          vout_display_GetDefaultDisplaySize(&display_width, &display_height,
> -                                           source, &cfg_tmp);
> +                                           p_sourceRot, &cfg_tmp);
> 
>          if (do_clipping) {
>              display_width  = __MIN(display_width,  cfg->display.width);
> @@ -231,12 +242,12 @@ void vout_display_PlacePicture(vout_display_place_t
> *place, }
>      }
> 
> -    const unsigned width  = source->i_visible_width;
> -    const unsigned height = source->i_visible_height;
> +    const unsigned width  = p_sourceRot->i_visible_width;
> +    const unsigned height = p_sourceRot->i_visible_height;
>      /* Compute the height if we use the width to fill up display_width */
> -    const int64_t scaled_height = (int64_t)height * display_width  *
> cfg->display.sar.num * source->i_sar_den / width  / source->i_sar_num /
> cfg->display.sar.den; +    const int64_t scaled_height = (int64_t)height *
> display_width  * cfg->display.sar.num * p_sourceRot->i_sar_den / width  /
> p_sourceRot->i_sar_num / cfg->display.sar.den; /* And the same but
> switching width/height */
> -    const int64_t scaled_width  = (int64_t)width  * display_height *
> cfg->display.sar.den * source->i_sar_num / height / source->i_sar_den /
> cfg->display.sar.num; +    const int64_t scaled_width  = (int64_t)width  *
> display_height * cfg->display.sar.den * p_sourceRot->i_sar_num / height /
> p_sourceRot->i_sar_den / cfg->display.sar.num;
> 
>      /* We keep the solution that avoid filling outside the display */
>      if (scaled_width <= cfg->display.width) {
> @@ -385,6 +396,88 @@ static void VoutDisplayCreateRender(vout_display_t *vd)
> v_dst.i_sar_num = 0;
>      v_dst.i_sar_den = 0;
> 
> +    es_format_t src;
> +    es_format_InitFromVideo(&src, &v_src);
> +
> +    filter_t *filter;
> +
> +    //We only handle orientaion conversion to ORIENT_NORMAL.

Skip other combinations if you want, but you cannot just ignore it (and likely 
crash afterwards).

> +    if (v_src.orientation != v_dst.orientation && v_dst.orientation ==
> ORIENT_NORMAL) {

> +
> +        const char *type;
> +
> +        switch (vd->source.orientation) {
> +
> +            case ORIENT_ROTATED_90:
> +                type = "90";
> +                break;
> +            case ORIENT_ROTATED_180:
> +                type = "180";
> +                break;
> +            case ORIENT_ROTATED_270:
> +                type = "270";
> +                break;
> +            case ORIENT_HFLIPPED:
> +                type = "hflip";
> +                break;
> +            case ORIENT_VFLIPPED:
> +                type = "vflip";
> +                break;
> +            case ORIENT_TRANSPOSED:
> +                type = "transpose";
> +                break;
> +            case ORIENT_ANTI_TRANSPOSED:
> +                type = "antitranspose";
> +                break;
> +            default:
> +                type = NULL;
> +                break;
> +        }
> +
> +        if (type) {
> +
> +            osys->filters = filter_chain_New(vd, "video filter2", true,
> +                                             FilterAllocationInit,
> +                                             FilterAllocationClean, vd);
> +            assert(osys->filters); /* TODO critical */
> +
> +            config_chain_t *cfg;
> +            char *name;
> +            char config[100];
> +            snprintf(config, 100, "transform{type=%s}", type);
> +            char *next;
> +            VLC_UNUSED(next);
> +            next = config_ChainCreate(&name, &cfg, config);

For the last time, this is leaking.

IMNSHO, this belongs in the chain filter, not in the core anyway. Otherwise, I 
fear this will break transcode.

> +
> +            //Don't make the transform filter trip over chroma conversions.
> +            video_format_t v_src_norm = src.video;
> +            v_src_norm.orientation = ORIENT_NORMAL; //Transform filter
> fills in all other geometric values. +            es_format_t src_norm;
> +            es_format_InitFromVideo(&src_norm, &v_src_norm);
> +            filter = filter_chain_AppendFilter(osys->filters, name, cfg,
> &src, &src_norm);
> +
> +            es_format_Clean(&src_norm);
> +            es_format_Clean(&src);
> +            config_ChainDestroy(cfg);
> +
> +            if (!filter) {
> +                msg_Dbg(vd, "Failed to create picture orientation transform
> filter."); +                filter_chain_Delete(osys->filters);
> +                osys->filters = NULL;
> +            }
> +            else {
> +                es_format_Copy(&src,
> filter_chain_GetFmtOut(osys->filters)); +                v_src = src.video;
> +
> +                //FIXME:
> +                //The transform filter produces x/y offsets which seem to
> be off by 1. +                //Instead of triggering the swscale filter
> because of this we simeply ignore it. +                v_src.i_x_offset =
> v_dst.i_x_offset;
> +                v_src.i_y_offset = v_dst.i_y_offset;
> +            }
> +        }
> +    }
> +
>      video_format_t v_dst_cmp = v_dst;
>      if ((v_src.i_chroma == VLC_CODEC_J420 && v_dst.i_chroma ==
> VLC_CODEC_I420) || (v_src.i_chroma == VLC_CODEC_J422 && v_dst.i_chroma ==
> VLC_CODEC_I422) || @@ -393,34 +486,39 @@ static void
> VoutDisplayCreateRender(vout_display_t *vd) v_dst_cmp.i_chroma =
> v_src.i_chroma;
> 
>      const bool convert = memcmp(&v_src, &v_dst_cmp, sizeof(v_src)) != 0;
> -    if (!convert)
> +
> +    if (!convert) {
> +        es_format_Clean(&src);

Why?

>          return;
> +    }
> 
>      msg_Dbg(vd, "A filter to adapt decoder to display is needed");
> 
> -    osys->filters = filter_chain_New(vd, "video filter2", false,
> -                                     FilterAllocationInit,
> -                                     FilterAllocationClean, vd);
> -    assert(osys->filters); /* TODO critical */
> -
>      /* */
> -    es_format_t src;
> -    es_format_InitFromVideo(&src, &v_src);
> +    if (!osys->filters)
> +        osys->filters = filter_chain_New(vd, "video filter2", true,
> +                                         FilterAllocationInit,
> +                                         FilterAllocationClean, vd);
> 
> -    /* */
> -    es_format_t dst;
> +    assert(osys->filters); /* TODO critical */
> 
> -    filter_t *filter;
>      for (int i = 0; i < 1 + (v_dst_cmp.i_chroma != v_dst.i_chroma); i++) {
> 
> +        es_format_t dst;
> +
>          es_format_InitFromVideo(&dst, i == 0 ? &v_dst : &v_dst_cmp);
> 
> -        filter_chain_Reset(osys->filters, &src, &dst);
>          filter = filter_chain_AppendFilter(osys->filters,
>                                             NULL, NULL, &src, &dst);
> +
> +        es_format_Clean(&dst);
> +
>          if (filter)
>              break;
>      }
> +
> +    es_format_Clean(&src);
> +
>      if (!filter)
>          msg_Err(vd, "Failed to adapt decoder format to display");
>  }
> @@ -520,6 +618,46 @@ static void VoutDisplayEventMouse(vout_display_t *vd,
> int event, va_list args) }
>      }
> 
> +    //vout modules report the mouse position in display coordinates,

That's simply not true. Vout modules are currently reporting mouse coordinates 
relative to the source video format. The core does not have enough 
informations to do the mapping anyway.

> +    //but the receivers most likely expect non-transformed coordinates.
> +    switch (vd->source.orientation) {
> +
> +        int store;
> +
> +        case ORIENT_ROTATED_90:
> +            store = m.i_x;
> +            m.i_x = m.i_y;
> +            m.i_y = vd->source.i_visible_height - store;
> +            break;
> +        case ORIENT_ROTATED_180:
> +            m.i_x = vd->source.i_visible_width - m.i_x;
> +            m.i_y = vd->source.i_visible_height - m.i_y;
> +            break;
> +        case ORIENT_ROTATED_270:
> +            store = m.i_x;
> +            m.i_x = vd->source.i_visible_width - m.i_y;
> +            m.i_y = store;
> +            break;
> +        case ORIENT_HFLIPPED:
> +            m.i_x = vd->source.i_visible_width - m.i_x;
> +            break;
> +        case ORIENT_VFLIPPED:
> +            m.i_y = vd->source.i_visible_height - m.i_y;
> +            break;
> +        case ORIENT_TRANSPOSED:
> +            store = m.i_x;
> +            m.i_x = m.i_y;
> +            m.i_y = store;
> +            break;
> +        case ORIENT_ANTI_TRANSPOSED:
> +            store = m.i_x;
> +            m.i_x = vd->source.i_visible_width - m.i_y;
> +            m.i_y = vd->source.i_visible_height - store;
> +            break;
> +        default:
> +            break;
> +    }
> +
>      /* */
>      osys->mouse.state = m;

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




More information about the vlc-devel mailing list