[vlc-devel] [PATCH 1/7] display: only call CHANGE_SOURCE_ASPECT if the value changed on osys->source

[Steve Lhomme]

On 2020-11-13 19:57, Rémi Denis-Courmont wrote:
> Le vendredi 13 novembre 2020, 17:56:28 EET Steve Lhomme a écrit :
>> ---
>>   src/video_output/display.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/video_output/display.c b/src/video_output/display.c
>> index 1d304e2b32b..0ddfc3bd452 100644
>> --- a/src/video_output/display.c
>> +++ b/src/video_output/display.c
>> @@ -530,10 +530,10 @@ static int vout_SetSourceAspect(vout_display_t *vd,
>>       if (sar_num > 0 && sar_den > 0) {
>>           osys->source.i_sar_num = sar_num;
>>           osys->source.i_sar_den = sar_den;
>> -    }
>>
>> -    if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_SOURCE_ASPECT))
>> -        ret = -1;
>> +        if (vout_display_Control(vd, VOUT_DISPLAY_CHANGE_SOURCE_ASPECT))
>> +            ret = -1;
>> +    }
>>
>>       /* If a crop ratio is requested, recompute the parameters */
>>       if (osys->crop.num != 0 && osys->crop.den != 0
> 
> The current logic does not make much sense as it stands. Either invalid SAR
> should mean some default value, or it should be forbidden.
> 
> With that, I don't really see the point of the patch. We should rather fix the
> logic that optimise a senseless case.

The goal of the patchset is to track more closely how osys variables 
(crop and sar) and used. For example the fact the sar was reset to odd 
values after be set to odd values. With proper push, if we want to 
restart a new display on format change, we need to be able to set back 
the crop/sar values set by the user. So it's needs to be clean in the 
first place.

This particular patch avoids doing a VOUT_DISPLAY_CHANGE_SOURCE_ASPECT 
if nothing changed. Because the receiver uses osys->source->i_sar_xxx to 
get the new aspect ratio (via vd->source).