[vlc-devel] [PATCH 3/6] Support for displaying rotated movies.
Matthias Keiser
matthias at tristan-inc.com
Fri Feb 28 20:57:54 CET 2014
Am 28.02.2014 um 17:34 schrieb Rémi Denis-Courmont <remi at remlab.net>:
> 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).
What should we do then? Are we allowed to change vd->fmt to something different?
Btw, currently, if filter creation fails, nothing is done about it except an error log.
>
>> + 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.
>
Right.
> IMNSHO, this belongs in the chain filter, not in the core anyway. Otherwise, I
> fear this will break transcode.
Sorry, I don't know what the "chain filter" is. Do you mean the filter chain? If yes, I assume you mean to let the transform filter figure out if it can do the conversion. This would mean changing its priority level 0 to something else. Is that ok? It probably would have to be higher than swscale (or else we could maybe add a check to swscale so that it bails if the orientations don't match).
>
>> +
>> + //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?
Because src is initialized with es_format_InitFromVideo, which potentially mallocs memory, which es_format_Clean then frees again.
>
>
>> 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.
Ok, vout modules will have to do this themselves then.
>
>> + //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/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list