[vlc-devel] [PATCH 1/4] vout: move aspect ratio members from vout_display_state_t to vout_display_cfg_t

[Rémi Denis-Courmont]

On Tue, 11 Feb 2014 10:30:34 +0100, Felix Abecassis
<felix.abecassis at gmail.com> wrote:
> 2014-02-11 9:26 GMT+01:00 Rémi Denis-Courmont <remi at remlab.net>:
> 
>> On Mon, 10 Feb 2014 19:14:50 +0100, Felix Abecassis
>> <felix.abecassis at gmail.com> wrote:
>> > ---
>> >  include/vlc_vout_display.h         |  5 +++++
>> >  include/vlc_vout_wrapper.h         |  4 ----
>> >  modules/visualization/glspectrum.c |  4 ++--
>> >  src/video_output/display.c         | 16 ++++++++++++----
>> >  src/video_output/video_output.c    | 19 +++++++++++++++----
>> >  5 files changed, 34 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
>> > index d99cf7e..870e207 100644
>> > --- a/include/vlc_vout_display.h
>> > +++ b/include/vlc_vout_display.h
>> > @@ -110,6 +110,11 @@ typedef struct {
>> >          int den;
>> >      } zoom;
>> >
>> > +    /* Aspect ratio */
>> > +    struct {
>> > +        int num;
>> > +        int den;
>> > +    } sar;
>> >  } vout_display_cfg_t;
>>
>> Err, how is that different from display.sar already in the same
>> structure?
>>
> This is display aspect ratio, initialized from variable "monitor-par".
> 
> 
>> And if it is different, you presumably need to patch every vout display
>> plugin to take the value into account :-(
>>
> Why? any change to the SAR will still be forwarded to the vout module
> through the control event VOUT_DISPLAY_CHANGE_SOURCE_ASPECT.

The value does not belong in the configuration if it is not used by the
plugins. So either way, I think this patch has a problem.

> What I don't understand is why this variable was in vout_display_state_t
> and not in the vout_display_cfg_t structure with other variables.

I think state is for core internal state, while cfg is for state exposed
to the vout display plugins.

-- 
Rémi Denis-Courmont
Sent from my collocated server