Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-07 Thread Tomasz Stanislawski

Hi Mauro, Hans,

I am really surprised by the havoc caused by the little 2-line patch.

Let me sum up what I (don't) like in Hans' and Mauro's approaches:

Hans approach:
- extend v4l2_enum_dv_preset with fps and flags fields,
- allow enumerating presets by both index and preset code
- add standard to macro names for presets

Pros:
- backward compatible with existing api
- very simple and effective. Setting desired preset using only 2 lines 
of code

- easy to add unfortunate 1080p59_94
- easy to differentiate 1080p59_94 from 1080p60 using VIDIOC_ENUM_DV_PRESET

Cons:
- number of existing macros must increase extensionally with number of 
features. Height, progressiveness, frequency are already present. 
Standard family is added in Hans' RFC. Some presets involve width. 
Therefore multiple holes are expected making usage of macros very 
unreliable.
- it is not possible to use VIDIOC_S_DV_PRESET to handle case when 
application just wants a preset that has 720 height. The application has 
to enumerate all existing/possible presets  (number of possible 
combinations may be large) to find a preset that suits to the 
application's needs.

- unnecessary redundancy, preset is nothing more than a standardized index

Mauro's approach:
- enumerate all possible presets using VIDIOC_ENUM_DV_PRESETS2
- choose suitable preset using index field from

Pros:
- consistency: preset can only be addressed by index field,
- no preset macros

Cons:
- structure v4l2_dv_enum_preset2 contains BT.656/BT.1120 timing related 
data, the structure should be more general. Most application would not 
use timing fields, so maybe there is no need of adding them to the 
structure.
- applications still has to enumerate through all possible combinations 
to find a suitable preset

- not compatible with existing API, two way to configure DV hardware

I propose following requirements for DV preset api basing on pros and 
cons from overmentioned approaches.
- an application should be able to choose a preset with desired 
parameters using single ioctl call
- preset should be accessed using single key. I prefer to use index as a 
key because it gives more flexibility to a driver.

- compatible with existing api as much as possible

What do you think about approach similar to S_FMT?
Please look at the code below.

struct v4l2_dv_preset2 {
   u16 width;
   u16 height;
   v4l2_fract fps;
   u32 flags; /* progressiveness, standard hints, rounding constraints */
   u32 reserved[];
};

/* Values for the standard field */
#define V4L2_DV_BT_656_1120 0   /* BT.656/1120 timing type */

struct v4l2_enum_dv_preset2 {
   u32 index;
   char name[32];
   struct v4l2_dv_preset2 preset;
   struct v4l2_dv_timings timings; /* to be removed ? */
   u32 reserved[];
};

#defineVIDIOC_ENUM_DV_PRESETS2_IOWR('V', 83, struct 
v4l2_dv_enum_preset2)

#defineVIDIOC_S_DV_PRESET2_IOWR('V', 84, struct v4l2_dv_preset2)
#defineVIDIOC_G_DV_PRESET2_IOWR('V', 85, struct v4l2_dv_preset2)
#defineVIDIOC_TRY_DV_PRESET2_IOWR('V', 86, struct v4l2_dv_preset2)

To set a mode with height 720 lines the applications would execute code 
below:


struct v4l2_dv_preset2 preset = {.height = 720 };
ioctl(fd, VIDIOC_S_DV_PRESET2, &preset);

The preset is selected using the most interesting features like 
width/height/frequency and progressiveness.
The driver would find the preset with vertical resolution as close as 
possible to 720.

The width and fps is zero so driver is free to choose suitable/default ones.
The field flags may contain hind about choosing preset that belong to 
specific DV standard family.


I do not feel competent in the field of DV standard. Could give me more 
ideas about flags?
The flags could contain  constraint  bits similar to ones presented in 
SELECTION api.
Maybe structures v4l2_dv_preset and v4l2_enum_dv_presets could be 
utilized for purpose of presented api.
Maybe using some union/structure align magic it would be possible to 
keep binary compatibility with existing programs.


For now, I have removed unfortunate 1080P59_94 format from S5P-TV driver.
I would be very happy if the driver was merged into 3.1.
I think that it would be not possible due to 1080p59_94 issue.
The driver did not lose much of its functionality because it still 
supports 1080p60.

Moreover, adding 1080p59_94 is relatively trivial.

I hope you find this information useful.

Regards,
Tomasz Stanislawski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-07 Thread Mauro Carvalho Chehab
Em 07-07-2011 11:58, Hans Verkuil escreveu:
> On Thursday, July 07, 2011 15:52:53 Mauro Carvalho Chehab wrote:
>> Em 07-07-2011 08:33, Hans Verkuil escreveu:
>>> On Wednesday, July 06, 2011 21:39:46 Mauro Carvalho Chehab wrote:
 Em 06-07-2011 09:14, Hans Verkuil escreveu:
>> Em 06-07-2011 08:31, Hans Verkuil escreveu:
 Em 05-07-2011 10:20, Hans Verkuil escreveu:

>> I failed to see what information is provided by the "presets" name.
>> If
>> this were removed
>> from the ioctl, and fps would be added instead, the API would be
>> clearer. The only
>> adjustment would be to use "index" as the preset selection key.
>> Anyway,
>> it is too late
>> for such change. We need to live with that.
>
> Adding the fps solves nothing. Because that still does not give you
> specific timings.
> You can have 1920x1080P60 that has quite different timings from the
> CEA-861 standard
> and that may not be supported by a TV.
>
> If you are working with HDMI, then you may want to filter all
> supported
> presets to
> those of the CEA standard.
>
> That's one thing that is missing at the moment: that presets belonging
> to a certain
> standard get their own range. Since we only do CEA861 right now it
> hasn't been an
> issue, but it will.

 I prepared a long email about that, but then I realized that we're
 investing our time into
 something broken, at the light of all DV timing standards. So, I've
 dropped it and
 started from scratch.

 From what I've got, there are some hardware that can only do a limited
 set
 of DV timings.
 If this were not the case, we could simply just use the
 VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
 and put the CEA 861 and VESA timings into some userspace library.

 In other words, the PRESET API is meant to solve the case where
 hardware
 only support
 a limited set of frequencies, that may or may not be inside the CEA
 standard.

 Let's assume we never added the current API, and discuss how it would
 properly fulfill
 the user needs. An API that would likely work is:

 struct v4l2_dv_enum_preset2 {
__u32 index;
__u8  name[32]; /* Name of the preset timing */

struct v4l2_fract fps;

 #define DV_PRESET_IS_PROGRESSIVE   1<<31
 #define DV_PRESET_SPEC(flag)   (flag && 0xff)
 #define DV_PRESET_IS_CEA8611
 #define DV_PRESET_IS_DMT   2
 #define DV_PRESET_IS_CVF   3
 #define DV_PRESET_IS_GTF   4
 #define DV_PRESET_IS_VENDOR_SPECIFIC   5

__u32   flags;  /* Interlaced/progressive, DV specs, 
 etc */

__u32   width;  /* width in pixels */
__u32   height; /* height in lines */
__u32   polarities; /* Positive or negative polarity */
__u64   pixelclock; /* Pixel clock in HZ. Ex. 
 74.25MHz->7425 */
__u32   hfrontporch;/* Horizpontal front porch in pixels */
__u32   hsync;  /* Horizontal Sync length in pixels */
__u32   hbackporch; /* Horizontal back porch in pixels */
__u32   vfrontporch;/* Vertical front porch in pixels */
__u32   vsync;  /* Vertical Sync length in lines */
__u32   vbackporch; /* Vertical back porch in lines */
__u32   il_vfrontporch; /* Vertical front porch for bottom 
 field of
 * interlaced field formats
 */
__u32   il_vsync;   /* Vertical sync length for bottom 
 field of
 * interlaced field formats
 */
__u32   il_vbackporch;  /* Vertical back porch for bottom field 
 of
 * interlaced field formats
 */
__u32 reserved[4];
 };

 #defineVIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
 v4l2_dv_enum_preset2)
 #defineVIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
 #defineVIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)

 Such preset API seems to work for all cases. Userspace can use any DV
 timing
 information to select the desired format, and don't need to have a
 switch

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-07 Thread Hans Verkuil
On Thursday, July 07, 2011 15:52:53 Mauro Carvalho Chehab wrote:
> Em 07-07-2011 08:33, Hans Verkuil escreveu:
> > On Wednesday, July 06, 2011 21:39:46 Mauro Carvalho Chehab wrote:
> >> Em 06-07-2011 09:14, Hans Verkuil escreveu:
>  Em 06-07-2011 08:31, Hans Verkuil escreveu:
> >> Em 05-07-2011 10:20, Hans Verkuil escreveu:
> >>
>  I failed to see what information is provided by the "presets" name.
>  If
>  this were removed
>  from the ioctl, and fps would be added instead, the API would be
>  clearer. The only
>  adjustment would be to use "index" as the preset selection key.
>  Anyway,
>  it is too late
>  for such change. We need to live with that.
> >>>
> >>> Adding the fps solves nothing. Because that still does not give you
> >>> specific timings.
> >>> You can have 1920x1080P60 that has quite different timings from the
> >>> CEA-861 standard
> >>> and that may not be supported by a TV.
> >>>
> >>> If you are working with HDMI, then you may want to filter all
> >>> supported
> >>> presets to
> >>> those of the CEA standard.
> >>>
> >>> That's one thing that is missing at the moment: that presets belonging
> >>> to a certain
> >>> standard get their own range. Since we only do CEA861 right now it
> >>> hasn't been an
> >>> issue, but it will.
> >>
> >> I prepared a long email about that, but then I realized that we're
> >> investing our time into
> >> something broken, at the light of all DV timing standards. So, I've
> >> dropped it and
> >> started from scratch.
> >>
> >> From what I've got, there are some hardware that can only do a limited
> >> set
> >> of DV timings.
> >> If this were not the case, we could simply just use the
> >> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
> >> and put the CEA 861 and VESA timings into some userspace library.
> >>
> >> In other words, the PRESET API is meant to solve the case where
> >> hardware
> >> only support
> >> a limited set of frequencies, that may or may not be inside the CEA
> >> standard.
> >>
> >> Let's assume we never added the current API, and discuss how it would
> >> properly fulfill
> >> the user needs. An API that would likely work is:
> >>
> >> struct v4l2_dv_enum_preset2 {
> >>__u32 index;
> >>__u8  name[32]; /* Name of the preset timing */
> >>
> >>struct v4l2_fract fps;
> >>
> >> #define DV_PRESET_IS_PROGRESSIVE   1<<31
> >> #define DV_PRESET_SPEC(flag)   (flag && 0xff)
> >> #define DV_PRESET_IS_CEA8611
> >> #define DV_PRESET_IS_DMT   2
> >> #define DV_PRESET_IS_CVF   3
> >> #define DV_PRESET_IS_GTF   4
> >> #define DV_PRESET_IS_VENDOR_SPECIFIC   5
> >>
> >>__u32   flags;  /* Interlaced/progressive, DV specs, 
> >> etc */
> >>
> >>__u32   width;  /* width in pixels */
> >>__u32   height; /* height in lines */
> >>__u32   polarities; /* Positive or negative polarity */
> >>__u64   pixelclock; /* Pixel clock in HZ. Ex. 
> >> 74.25MHz->7425 */
> >>__u32   hfrontporch;/* Horizpontal front porch in pixels */
> >>__u32   hsync;  /* Horizontal Sync length in pixels */
> >>__u32   hbackporch; /* Horizontal back porch in pixels */
> >>__u32   vfrontporch;/* Vertical front porch in pixels */
> >>__u32   vsync;  /* Vertical Sync length in lines */
> >>__u32   vbackporch; /* Vertical back porch in lines */
> >>__u32   il_vfrontporch; /* Vertical front porch for bottom 
> >> field of
> >> * interlaced field formats
> >> */
> >>__u32   il_vsync;   /* Vertical sync length for bottom 
> >> field of
> >> * interlaced field formats
> >> */
> >>__u32   il_vbackporch;  /* Vertical back porch for bottom field 
> >> of
> >> * interlaced field formats
> >> */
> >>__u32 reserved[4];
> >> };
> >>
> >> #defineVIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
> >> v4l2_dv_enum_preset2)
> >> #defineVIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
> >> #defineVIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
> >>
> >> Such preset API seems to work for all cases. Userspace can use any DV
> >> timing
> >> information to select the desired format, and don't need to have a
> >> switch
> >> for
> >> a preset macro to try to gues

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-07 Thread Mauro Carvalho Chehab
Em 07-07-2011 08:33, Hans Verkuil escreveu:
> On Wednesday, July 06, 2011 21:39:46 Mauro Carvalho Chehab wrote:
>> Em 06-07-2011 09:14, Hans Verkuil escreveu:
 Em 06-07-2011 08:31, Hans Verkuil escreveu:
>> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>>
 I failed to see what information is provided by the "presets" name.
 If
 this were removed
 from the ioctl, and fps would be added instead, the API would be
 clearer. The only
 adjustment would be to use "index" as the preset selection key.
 Anyway,
 it is too late
 for such change. We need to live with that.
>>>
>>> Adding the fps solves nothing. Because that still does not give you
>>> specific timings.
>>> You can have 1920x1080P60 that has quite different timings from the
>>> CEA-861 standard
>>> and that may not be supported by a TV.
>>>
>>> If you are working with HDMI, then you may want to filter all
>>> supported
>>> presets to
>>> those of the CEA standard.
>>>
>>> That's one thing that is missing at the moment: that presets belonging
>>> to a certain
>>> standard get their own range. Since we only do CEA861 right now it
>>> hasn't been an
>>> issue, but it will.
>>
>> I prepared a long email about that, but then I realized that we're
>> investing our time into
>> something broken, at the light of all DV timing standards. So, I've
>> dropped it and
>> started from scratch.
>>
>> From what I've got, there are some hardware that can only do a limited
>> set
>> of DV timings.
>> If this were not the case, we could simply just use the
>> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
>> and put the CEA 861 and VESA timings into some userspace library.
>>
>> In other words, the PRESET API is meant to solve the case where
>> hardware
>> only support
>> a limited set of frequencies, that may or may not be inside the CEA
>> standard.
>>
>> Let's assume we never added the current API, and discuss how it would
>> properly fulfill
>> the user needs. An API that would likely work is:
>>
>> struct v4l2_dv_enum_preset2 {
>>  __u32 index;
>>  __u8  name[32]; /* Name of the preset timing */
>>
>>  struct v4l2_fract fps;
>>
>> #define DV_PRESET_IS_PROGRESSIVE 1<<31
>> #define DV_PRESET_SPEC(flag) (flag && 0xff)
>> #define DV_PRESET_IS_CEA861  1
>> #define DV_PRESET_IS_DMT 2
>> #define DV_PRESET_IS_CVF 3
>> #define DV_PRESET_IS_GTF 4
>> #define DV_PRESET_IS_VENDOR_SPECIFIC 5
>>
>>  __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>>
>>  __u32   width;  /* width in pixels */
>>  __u32   height; /* height in lines */
>>  __u32   polarities; /* Positive or negative polarity */
>>  __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>>  __u32   hfrontporch;/* Horizpontal front porch in pixels */
>>  __u32   hsync;  /* Horizontal Sync length in pixels */
>>  __u32   hbackporch; /* Horizontal back porch in pixels */
>>  __u32   vfrontporch;/* Vertical front porch in pixels */
>>  __u32   vsync;  /* Vertical Sync length in lines */
>>  __u32   vbackporch; /* Vertical back porch in lines */
>>  __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32   il_vsync;   /* Vertical sync length for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32 reserved[4];
>> };
>>
>> #define  VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>> v4l2_dv_enum_preset2)
>> #define  VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>> #define  VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>>
>> Such preset API seems to work for all cases. Userspace can use any DV
>> timing
>> information to select the desired format, and don't need to have a
>> switch
>> for
>> a preset macro to try to guess what the format actually means. Also,
>> there's no
>> need to touch at the API spec every time a new DV timeline is needed.
>>
>> Also, it should be noticed that, since the size of the data on the
>> above
>> definitions
>> are different than the old ones, _IO macros will provide a different
>> magic
>> number,
>> so, adding these won't break the existing API.
>>
>> So, I think we should work on this proposal, and mark the existing one
>> as

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-07 Thread Hans Verkuil
On Wednesday, July 06, 2011 21:39:46 Mauro Carvalho Chehab wrote:
> Em 06-07-2011 09:14, Hans Verkuil escreveu:
> >> Em 06-07-2011 08:31, Hans Verkuil escreveu:
>  Em 05-07-2011 10:20, Hans Verkuil escreveu:
> 
> >> I failed to see what information is provided by the "presets" name.
> >> If
> >> this were removed
> >> from the ioctl, and fps would be added instead, the API would be
> >> clearer. The only
> >> adjustment would be to use "index" as the preset selection key.
> >> Anyway,
> >> it is too late
> >> for such change. We need to live with that.
> >
> > Adding the fps solves nothing. Because that still does not give you
> > specific timings.
> > You can have 1920x1080P60 that has quite different timings from the
> > CEA-861 standard
> > and that may not be supported by a TV.
> >
> > If you are working with HDMI, then you may want to filter all
> > supported
> > presets to
> > those of the CEA standard.
> >
> > That's one thing that is missing at the moment: that presets belonging
> > to a certain
> > standard get their own range. Since we only do CEA861 right now it
> > hasn't been an
> > issue, but it will.
> 
>  I prepared a long email about that, but then I realized that we're
>  investing our time into
>  something broken, at the light of all DV timing standards. So, I've
>  dropped it and
>  started from scratch.
> 
>  From what I've got, there are some hardware that can only do a limited
>  set
>  of DV timings.
>  If this were not the case, we could simply just use the
>  VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
>  and put the CEA 861 and VESA timings into some userspace library.
> 
>  In other words, the PRESET API is meant to solve the case where
>  hardware
>  only support
>  a limited set of frequencies, that may or may not be inside the CEA
>  standard.
> 
>  Let's assume we never added the current API, and discuss how it would
>  properly fulfill
>  the user needs. An API that would likely work is:
> 
>  struct v4l2_dv_enum_preset2 {
>   __u32 index;
>   __u8  name[32]; /* Name of the preset timing */
> 
>   struct v4l2_fract fps;
> 
>  #define DV_PRESET_IS_PROGRESSIVE 1<<31
>  #define DV_PRESET_SPEC(flag) (flag && 0xff)
>  #define DV_PRESET_IS_CEA861  1
>  #define DV_PRESET_IS_DMT 2
>  #define DV_PRESET_IS_CVF 3
>  #define DV_PRESET_IS_GTF 4
>  #define DV_PRESET_IS_VENDOR_SPECIFIC 5
> 
>   __u32   flags;  /* Interlaced/progressive, DV specs, etc */
> 
>   __u32   width;  /* width in pixels */
>   __u32   height; /* height in lines */
>   __u32   polarities; /* Positive or negative polarity */
>   __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>   __u32   hfrontporch;/* Horizpontal front porch in pixels */
>   __u32   hsync;  /* Horizontal Sync length in pixels */
>   __u32   hbackporch; /* Horizontal back porch in pixels */
>   __u32   vfrontporch;/* Vertical front porch in pixels */
>   __u32   vsync;  /* Vertical Sync length in lines */
>   __u32   vbackporch; /* Vertical back porch in lines */
>   __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>    * interlaced field formats
>    */
>   __u32   il_vsync;   /* Vertical sync length for bottom field of
>    * interlaced field formats
>    */
>   __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>    * interlaced field formats
>    */
>   __u32 reserved[4];
>  };
> 
>  #define  VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>  v4l2_dv_enum_preset2)
>  #define  VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>  #define  VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
> 
>  Such preset API seems to work for all cases. Userspace can use any DV
>  timing
>  information to select the desired format, and don't need to have a
>  switch
>  for
>  a preset macro to try to guess what the format actually means. Also,
>  there's no
>  need to touch at the API spec every time a new DV timeline is needed.
> 
>  Also, it should be noticed that, since the size of the data on the
>  above
>  definitions
>  are different than the old ones, _IO macros will provide a different
>  magic
>  number,
>  so, adding these won't break the existing API.
> 
>  So, I think we should work on this proposal, and mark the existing one
>  as
>  deprecated.
> >>>
> >>> This proposal makes 

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 06-07-2011 09:14, Hans Verkuil escreveu:
>> Em 06-07-2011 08:31, Hans Verkuil escreveu:
 Em 05-07-2011 10:20, Hans Verkuil escreveu:

>> I failed to see what information is provided by the "presets" name.
>> If
>> this were removed
>> from the ioctl, and fps would be added instead, the API would be
>> clearer. The only
>> adjustment would be to use "index" as the preset selection key.
>> Anyway,
>> it is too late
>> for such change. We need to live with that.
>
> Adding the fps solves nothing. Because that still does not give you
> specific timings.
> You can have 1920x1080P60 that has quite different timings from the
> CEA-861 standard
> and that may not be supported by a TV.
>
> If you are working with HDMI, then you may want to filter all
> supported
> presets to
> those of the CEA standard.
>
> That's one thing that is missing at the moment: that presets belonging
> to a certain
> standard get their own range. Since we only do CEA861 right now it
> hasn't been an
> issue, but it will.

 I prepared a long email about that, but then I realized that we're
 investing our time into
 something broken, at the light of all DV timing standards. So, I've
 dropped it and
 started from scratch.

 From what I've got, there are some hardware that can only do a limited
 set
 of DV timings.
 If this were not the case, we could simply just use the
 VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
 and put the CEA 861 and VESA timings into some userspace library.

 In other words, the PRESET API is meant to solve the case where
 hardware
 only support
 a limited set of frequencies, that may or may not be inside the CEA
 standard.

 Let's assume we never added the current API, and discuss how it would
 properly fulfill
 the user needs. An API that would likely work is:

 struct v4l2_dv_enum_preset2 {
__u32 index;
__u8  name[32]; /* Name of the preset timing */

struct v4l2_fract fps;

 #define DV_PRESET_IS_PROGRESSIVE   1<<31
 #define DV_PRESET_SPEC(flag)   (flag && 0xff)
 #define DV_PRESET_IS_CEA8611
 #define DV_PRESET_IS_DMT   2
 #define DV_PRESET_IS_CVF   3
 #define DV_PRESET_IS_GTF   4
 #define DV_PRESET_IS_VENDOR_SPECIFIC   5

__u32   flags;  /* Interlaced/progressive, DV specs, etc */

__u32   width;  /* width in pixels */
__u32   height; /* height in lines */
__u32   polarities; /* Positive or negative polarity */
__u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
__u32   hfrontporch;/* Horizpontal front porch in pixels */
__u32   hsync;  /* Horizontal Sync length in pixels */
__u32   hbackporch; /* Horizontal back porch in pixels */
__u32   vfrontporch;/* Vertical front porch in pixels */
__u32   vsync;  /* Vertical Sync length in lines */
__u32   vbackporch; /* Vertical back porch in lines */
__u32   il_vfrontporch; /* Vertical front porch for bottom field of
 * interlaced field formats
 */
__u32   il_vsync;   /* Vertical sync length for bottom field of
 * interlaced field formats
 */
__u32   il_vbackporch;  /* Vertical back porch for bottom field of
 * interlaced field formats
 */
__u32 reserved[4];
 };

 #defineVIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
 v4l2_dv_enum_preset2)
 #defineVIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
 #defineVIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)

 Such preset API seems to work for all cases. Userspace can use any DV
 timing
 information to select the desired format, and don't need to have a
 switch
 for
 a preset macro to try to guess what the format actually means. Also,
 there's no
 need to touch at the API spec every time a new DV timeline is needed.

 Also, it should be noticed that, since the size of the data on the
 above
 definitions
 are different than the old ones, _IO macros will provide a different
 magic
 number,
 so, adding these won't break the existing API.

 So, I think we should work on this proposal, and mark the existing one
 as
 deprecated.
>>>
>>> This proposal makes it very hard for applications to directly select a
>>> format like 720p50 because the indices can change at any time.
>>
>> Why? All the application needs to do is to call VIDIOC_ENUM_DV_PRESETS2,
>> check what line it wants,
> 
> It's 

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 06-07-2011 09:56, Hans Verkuil escreveu:
>> Em 06-07-2011 09:14, Hans Verkuil escreveu:
 Em 06-07-2011 08:31, Hans Verkuil escreveu:
>> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>>
 I failed to see what information is provided by the "presets" name.
 If
 this were removed
 from the ioctl, and fps would be added instead, the API would be
 clearer. The only
 adjustment would be to use "index" as the preset selection key.
 Anyway,
 it is too late
 for such change. We need to live with that.
>>>
>>> Adding the fps solves nothing. Because that still does not give you
>>> specific timings.
>>> You can have 1920x1080P60 that has quite different timings from the
>>> CEA-861 standard
>>> and that may not be supported by a TV.
>>>
>>> If you are working with HDMI, then you may want to filter all
>>> supported
>>> presets to
>>> those of the CEA standard.
>>>
>>> That's one thing that is missing at the moment: that presets
>>> belonging
>>> to a certain
>>> standard get their own range. Since we only do CEA861 right now it
>>> hasn't been an
>>> issue, but it will.
>>
>> I prepared a long email about that, but then I realized that we're
>> investing our time into
>> something broken, at the light of all DV timing standards. So, I've
>> dropped it and
>> started from scratch.
>>
>> From what I've got, there are some hardware that can only do a
>> limited
>> set
>> of DV timings.
>> If this were not the case, we could simply just use the
>> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
>> and put the CEA 861 and VESA timings into some userspace library.
>>
>> In other words, the PRESET API is meant to solve the case where
>> hardware
>> only support
>> a limited set of frequencies, that may or may not be inside the CEA
>> standard.
>>
>> Let's assume we never added the current API, and discuss how it would
>> properly fulfill
>> the user needs. An API that would likely work is:
>>
>> struct v4l2_dv_enum_preset2 {
>>  __u32 index;
>>  __u8  name[32]; /* Name of the preset timing */
>>
>>  struct v4l2_fract fps;
>>
>> #define DV_PRESET_IS_PROGRESSIVE 1<<31
>> #define DV_PRESET_SPEC(flag) (flag && 0xff)
>> #define DV_PRESET_IS_CEA861  1
>> #define DV_PRESET_IS_DMT 2
>> #define DV_PRESET_IS_CVF 3
>> #define DV_PRESET_IS_GTF 4
>> #define DV_PRESET_IS_VENDOR_SPECIFIC 5
>>
>>  __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>>
>>  __u32   width;  /* width in pixels */
>>  __u32   height; /* height in lines */
>>  __u32   polarities; /* Positive or negative polarity */
>>  __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>>  __u32   hfrontporch;/* Horizpontal front porch in pixels */
>>  __u32   hsync;  /* Horizontal Sync length in pixels */
>>  __u32   hbackporch; /* Horizontal back porch in pixels */
>>  __u32   vfrontporch;/* Vertical front porch in pixels */
>>  __u32   vsync;  /* Vertical Sync length in lines */
>>  __u32   vbackporch; /* Vertical back porch in lines */
>>  __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32   il_vsync;   /* Vertical sync length for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32 reserved[4];
>> };
>>
>> #define  VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>> v4l2_dv_enum_preset2)
>> #define  VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>> #define  VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>>
>> Such preset API seems to work for all cases. Userspace can use any DV
>> timing
>> information to select the desired format, and don't need to have a
>> switch
>> for
>> a preset macro to try to guess what the format actually means. Also,
>> there's no
>> need to touch at the API spec every time a new DV timeline is needed.
>>
>> Also, it should be noticed that, since the size of the data on the
>> above
>> definitions
>> are different than the old ones, _IO macros will provide a different
>> magic
>> number,
>> so, adding these won't break the existing API.
>>
>> So, I think we should work on this proposal, and mark the existing
>> one
>> as
>> deprecated.
>
> This proposal makes

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Hans Verkuil
> Em 06-07-2011 09:14, Hans Verkuil escreveu:
>>> Em 06-07-2011 08:31, Hans Verkuil escreveu:
> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>
>>> I failed to see what information is provided by the "presets" name.
>>> If
>>> this were removed
>>> from the ioctl, and fps would be added instead, the API would be
>>> clearer. The only
>>> adjustment would be to use "index" as the preset selection key.
>>> Anyway,
>>> it is too late
>>> for such change. We need to live with that.
>>
>> Adding the fps solves nothing. Because that still does not give you
>> specific timings.
>> You can have 1920x1080P60 that has quite different timings from the
>> CEA-861 standard
>> and that may not be supported by a TV.
>>
>> If you are working with HDMI, then you may want to filter all
>> supported
>> presets to
>> those of the CEA standard.
>>
>> That's one thing that is missing at the moment: that presets
>> belonging
>> to a certain
>> standard get their own range. Since we only do CEA861 right now it
>> hasn't been an
>> issue, but it will.
>
> I prepared a long email about that, but then I realized that we're
> investing our time into
> something broken, at the light of all DV timing standards. So, I've
> dropped it and
> started from scratch.
>
> From what I've got, there are some hardware that can only do a
> limited
> set
> of DV timings.
> If this were not the case, we could simply just use the
> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
> and put the CEA 861 and VESA timings into some userspace library.
>
> In other words, the PRESET API is meant to solve the case where
> hardware
> only support
> a limited set of frequencies, that may or may not be inside the CEA
> standard.
>
> Let's assume we never added the current API, and discuss how it would
> properly fulfill
> the user needs. An API that would likely work is:
>
> struct v4l2_dv_enum_preset2 {
>   __u32 index;
>   __u8  name[32]; /* Name of the preset timing */
>
>   struct v4l2_fract fps;
>
> #define DV_PRESET_IS_PROGRESSIVE  1<<31
> #define DV_PRESET_SPEC(flag)  (flag && 0xff)
> #define DV_PRESET_IS_CEA861   1
> #define DV_PRESET_IS_DMT  2
> #define DV_PRESET_IS_CVF  3
> #define DV_PRESET_IS_GTF  4
> #define DV_PRESET_IS_VENDOR_SPECIFIC  5
>
>   __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>
>   __u32   width;  /* width in pixels */
>   __u32   height; /* height in lines */
>   __u32   polarities; /* Positive or negative polarity */
>   __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>   __u32   hfrontporch;/* Horizpontal front porch in pixels */
>   __u32   hsync;  /* Horizontal Sync length in pixels */
>   __u32   hbackporch; /* Horizontal back porch in pixels */
>   __u32   vfrontporch;/* Vertical front porch in pixels */
>   __u32   vsync;  /* Vertical Sync length in lines */
>   __u32   vbackporch; /* Vertical back porch in lines */
>   __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>* interlaced field formats
>*/
>   __u32   il_vsync;   /* Vertical sync length for bottom field of
>* interlaced field formats
>*/
>   __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>* interlaced field formats
>*/
>   __u32 reserved[4];
> };
>
> #define   VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
> v4l2_dv_enum_preset2)
> #define   VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
> #define   VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>
> Such preset API seems to work for all cases. Userspace can use any DV
> timing
> information to select the desired format, and don't need to have a
> switch
> for
> a preset macro to try to guess what the format actually means. Also,
> there's no
> need to touch at the API spec every time a new DV timeline is needed.
>
> Also, it should be noticed that, since the size of the data on the
> above
> definitions
> are different than the old ones, _IO macros will provide a different
> magic
> number,
> so, adding these won't break the existing API.
>
> So, I think we should work on this proposal, and mark the existing
> one
> as
> deprecated.

 This proposal makes it very hard for applications to directly select a
 format like 720p50 because the indices can change at any time.
>>>
>>> Why?

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 06-07-2011 09:14, Hans Verkuil escreveu:
>> Em 06-07-2011 08:31, Hans Verkuil escreveu:
 Em 05-07-2011 10:20, Hans Verkuil escreveu:

>> I failed to see what information is provided by the "presets" name.
>> If
>> this were removed
>> from the ioctl, and fps would be added instead, the API would be
>> clearer. The only
>> adjustment would be to use "index" as the preset selection key.
>> Anyway,
>> it is too late
>> for such change. We need to live with that.
>
> Adding the fps solves nothing. Because that still does not give you
> specific timings.
> You can have 1920x1080P60 that has quite different timings from the
> CEA-861 standard
> and that may not be supported by a TV.
>
> If you are working with HDMI, then you may want to filter all
> supported
> presets to
> those of the CEA standard.
>
> That's one thing that is missing at the moment: that presets belonging
> to a certain
> standard get their own range. Since we only do CEA861 right now it
> hasn't been an
> issue, but it will.

 I prepared a long email about that, but then I realized that we're
 investing our time into
 something broken, at the light of all DV timing standards. So, I've
 dropped it and
 started from scratch.

 From what I've got, there are some hardware that can only do a limited
 set
 of DV timings.
 If this were not the case, we could simply just use the
 VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
 and put the CEA 861 and VESA timings into some userspace library.

 In other words, the PRESET API is meant to solve the case where
 hardware
 only support
 a limited set of frequencies, that may or may not be inside the CEA
 standard.

 Let's assume we never added the current API, and discuss how it would
 properly fulfill
 the user needs. An API that would likely work is:

 struct v4l2_dv_enum_preset2 {
__u32 index;
__u8  name[32]; /* Name of the preset timing */

struct v4l2_fract fps;

 #define DV_PRESET_IS_PROGRESSIVE   1<<31
 #define DV_PRESET_SPEC(flag)   (flag && 0xff)
 #define DV_PRESET_IS_CEA8611
 #define DV_PRESET_IS_DMT   2
 #define DV_PRESET_IS_CVF   3
 #define DV_PRESET_IS_GTF   4
 #define DV_PRESET_IS_VENDOR_SPECIFIC   5

__u32   flags;  /* Interlaced/progressive, DV specs, etc */

__u32   width;  /* width in pixels */
__u32   height; /* height in lines */
__u32   polarities; /* Positive or negative polarity */
__u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
__u32   hfrontporch;/* Horizpontal front porch in pixels */
__u32   hsync;  /* Horizontal Sync length in pixels */
__u32   hbackporch; /* Horizontal back porch in pixels */
__u32   vfrontporch;/* Vertical front porch in pixels */
__u32   vsync;  /* Vertical Sync length in lines */
__u32   vbackporch; /* Vertical back porch in lines */
__u32   il_vfrontporch; /* Vertical front porch for bottom field of
 * interlaced field formats
 */
__u32   il_vsync;   /* Vertical sync length for bottom field of
 * interlaced field formats
 */
__u32   il_vbackporch;  /* Vertical back porch for bottom field of
 * interlaced field formats
 */
__u32 reserved[4];
 };

 #defineVIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
 v4l2_dv_enum_preset2)
 #defineVIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
 #defineVIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)

 Such preset API seems to work for all cases. Userspace can use any DV
 timing
 information to select the desired format, and don't need to have a
 switch
 for
 a preset macro to try to guess what the format actually means. Also,
 there's no
 need to touch at the API spec every time a new DV timeline is needed.

 Also, it should be noticed that, since the size of the data on the
 above
 definitions
 are different than the old ones, _IO macros will provide a different
 magic
 number,
 so, adding these won't break the existing API.

 So, I think we should work on this proposal, and mark the existing one
 as
 deprecated.
>>>
>>> This proposal makes it very hard for applications to directly select a
>>> format like 720p50 because the indices can change at any time.
>>
>> Why? All the application needs to do is to call VIDIOC_ENUM_DV_PRESETS2,
>> check what line it wants,
> 
> It's 

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 06-07-2011 09:13, Laurent Pinchart escreveu:
> On Wednesday 06 July 2011 14:09:38 Mauro Carvalho Chehab wrote:
>> Em 06-07-2011 09:03, Laurent Pinchart escreveu:
>>> On Wednesday 06 July 2011 13:48:35 Mauro Carvalho Chehab wrote:
 Em 06-07-2011 08:31, Hans Verkuil escreveu:
>> Em 05-07-2011 10:20, Hans Verkuil escreveu:
 I failed to see what information is provided by the "presets" name.
 If this were removed from the ioctl, and fps would be added
 instead, the API would be clearer. The only adjustment would be to
 use "index" as the preset selection key. Anyway, it is too late for
 such change. We need to live with that.
>>>
>>> Adding the fps solves nothing. Because that still does not give you
>>> specific timings. You can have 1920x1080P60 that has quite different
>>> timings from the CEA-861 standard and that may not be supported by a
>>> TV.
>>>
>>> If you are working with HDMI, then you may want to filter all
>>> supported presets to those of the CEA standard.
>>>
>>> That's one thing that is missing at the moment: that presets
>>> belonging to a certain standard get their own range. Since we only
>>> do CEA861 right now it hasn't been an issue, but it will.
>>
>> I prepared a long email about that, but then I realized that we're
>> investing our time intosomething broken, at the light of all DV timing
>> standards. So, I've dropped it and started from scratch.
>>
>> From what I've got, there are some hardware that can only do a limited
>> set of DV timings. If this were not the case, we could simply just use
>> the VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS, and put the CEA 861 and
>> VESA timings into some userspace library.
>>
>> In other words, the PRESET API is meant to solve the case where
>> hardware only support a limited set of frequencies, that may or may
>> not be inside the CEA standard.
>>
>> Let's assume we never added the current API, and discuss how it would
>> properly fulfill the user needs. An API that would likely work is:
>>
>> struct v4l2_dv_enum_preset2 {
>>
>>  __u32 index;
>>  __u8  name[32]; /* Name of the preset timing */
>>  
>>  struct v4l2_fract fps;
>>
>> #define DV_PRESET_IS_PROGRESSIVE 1<<31
>> #define DV_PRESET_SPEC(flag) (flag && 0xff)
>> #define DV_PRESET_IS_CEA861  1
>> #define DV_PRESET_IS_DMT 2
>> #define DV_PRESET_IS_CVF 3
>> #define DV_PRESET_IS_GTF 4
>> #define DV_PRESET_IS_VENDOR_SPECIFIC 5
>>
>>  __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>>  
>>  __u32   width;  /* width in pixels */
>>  __u32   height; /* height in lines */
>>  __u32   polarities; /* Positive or negative polarity */
>>  __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>>  __u32   hfrontporch;/* Horizpontal front porch in pixels */
>>  __u32   hsync;  /* Horizontal Sync length in pixels */
>>  __u32   hbackporch; /* Horizontal back porch in pixels */
>>  __u32   vfrontporch;/* Vertical front porch in pixels */
>>  __u32   vsync;  /* Vertical Sync length in lines */
>>  __u32   vbackporch; /* Vertical back porch in lines */
>>  __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>>  
>>   * interlaced field formats
>>   */
>>  
>>  __u32   il_vsync;   /* Vertical sync length for bottom field of
>>  
>>   * interlaced field formats
>>   */
>>  
>>  __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>>  
>>   * interlaced field formats
>>   */
>>  
>>  __u32 reserved[4];
>>
>> };
>>
>> #define  VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>> v4l2_dv_enum_preset2)
>> #define  VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>> #define  VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>>
>> Such preset API seems to work for all cases. Userspace can use any DV
>> timing information to select the desired format, and don't need to
>> have a switch for a preset macro to try to guess what the format
>> actually means. Also, there's no need to touch at the API spec every
>> time a new DV timeline is needed.
>>
>> Also, it should be noticed that, since the size of the data on the
>> above definitions are different than the old ones, _IO macros will
>> provide a different magic number, so, adding these won't break the
>> existing API.
>>
>> So, I think we should work on this proposal, and mark the existing one
>> as deprecated.
>
> This prop

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Hans Verkuil
> Em 06-07-2011 08:31, Hans Verkuil escreveu:
>>> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>>>
> I failed to see what information is provided by the "presets" name.
> If
> this were removed
> from the ioctl, and fps would be added instead, the API would be
> clearer. The only
> adjustment would be to use "index" as the preset selection key.
> Anyway,
> it is too late
> for such change. We need to live with that.

 Adding the fps solves nothing. Because that still does not give you
 specific timings.
 You can have 1920x1080P60 that has quite different timings from the
 CEA-861 standard
 and that may not be supported by a TV.

 If you are working with HDMI, then you may want to filter all
 supported
 presets to
 those of the CEA standard.

 That's one thing that is missing at the moment: that presets belonging
 to a certain
 standard get their own range. Since we only do CEA861 right now it
 hasn't been an
 issue, but it will.
>>>
>>> I prepared a long email about that, but then I realized that we're
>>> investing our time into
>>> something broken, at the light of all DV timing standards. So, I've
>>> dropped it and
>>> started from scratch.
>>>
>>> From what I've got, there are some hardware that can only do a limited
>>> set
>>> of DV timings.
>>> If this were not the case, we could simply just use the
>>> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
>>> and put the CEA 861 and VESA timings into some userspace library.
>>>
>>> In other words, the PRESET API is meant to solve the case where
>>> hardware
>>> only support
>>> a limited set of frequencies, that may or may not be inside the CEA
>>> standard.
>>>
>>> Let's assume we never added the current API, and discuss how it would
>>> properly fulfill
>>> the user needs. An API that would likely work is:
>>>
>>> struct v4l2_dv_enum_preset2 {
>>> __u32 index;
>>> __u8  name[32]; /* Name of the preset timing */
>>>
>>> struct v4l2_fract fps;
>>>
>>> #define DV_PRESET_IS_PROGRESSIVE1<<31
>>> #define DV_PRESET_SPEC(flag)(flag && 0xff)
>>> #define DV_PRESET_IS_CEA861 1
>>> #define DV_PRESET_IS_DMT2
>>> #define DV_PRESET_IS_CVF3
>>> #define DV_PRESET_IS_GTF4
>>> #define DV_PRESET_IS_VENDOR_SPECIFIC5
>>>
>>> __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>>>
>>> __u32   width;  /* width in pixels */
>>> __u32   height; /* height in lines */
>>> __u32   polarities; /* Positive or negative polarity */
>>> __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>>> __u32   hfrontporch;/* Horizpontal front porch in pixels */
>>> __u32   hsync;  /* Horizontal Sync length in pixels */
>>> __u32   hbackporch; /* Horizontal back porch in pixels */
>>> __u32   vfrontporch;/* Vertical front porch in pixels */
>>> __u32   vsync;  /* Vertical Sync length in lines */
>>> __u32   vbackporch; /* Vertical back porch in lines */
>>> __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>>>  * interlaced field formats
>>>  */
>>> __u32   il_vsync;   /* Vertical sync length for bottom field of
>>>  * interlaced field formats
>>>  */
>>> __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>>>  * interlaced field formats
>>>  */
>>> __u32 reserved[4];
>>> };
>>>
>>> #define VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>>> v4l2_dv_enum_preset2)
>>> #define VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>>> #define VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>>>
>>> Such preset API seems to work for all cases. Userspace can use any DV
>>> timing
>>> information to select the desired format, and don't need to have a
>>> switch
>>> for
>>> a preset macro to try to guess what the format actually means. Also,
>>> there's no
>>> need to touch at the API spec every time a new DV timeline is needed.
>>>
>>> Also, it should be noticed that, since the size of the data on the
>>> above
>>> definitions
>>> are different than the old ones, _IO macros will provide a different
>>> magic
>>> number,
>>> so, adding these won't break the existing API.
>>>
>>> So, I think we should work on this proposal, and mark the existing one
>>> as
>>> deprecated.
>>
>> This proposal makes it very hard for applications to directly select a
>> format like 720p50 because the indices can change at any time.
>
> Why? All the application needs to do is to call VIDIOC_ENUM_DV_PRESETS2,
> check what line it wants,

It's not so easy as you think to find the right timings: you have to check
many parameters to be certain you have the right one and not some subtle
v

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Laurent Pinchart
On Wednesday 06 July 2011 14:09:38 Mauro Carvalho Chehab wrote:
> Em 06-07-2011 09:03, Laurent Pinchart escreveu:
> > On Wednesday 06 July 2011 13:48:35 Mauro Carvalho Chehab wrote:
> >> Em 06-07-2011 08:31, Hans Verkuil escreveu:
>  Em 05-07-2011 10:20, Hans Verkuil escreveu:
> >> I failed to see what information is provided by the "presets" name.
> >> If this were removed from the ioctl, and fps would be added
> >> instead, the API would be clearer. The only adjustment would be to
> >> use "index" as the preset selection key. Anyway, it is too late for
> >> such change. We need to live with that.
> > 
> > Adding the fps solves nothing. Because that still does not give you
> > specific timings. You can have 1920x1080P60 that has quite different
> > timings from the CEA-861 standard and that may not be supported by a
> > TV.
> > 
> > If you are working with HDMI, then you may want to filter all
> > supported presets to those of the CEA standard.
> > 
> > That's one thing that is missing at the moment: that presets
> > belonging to a certain standard get their own range. Since we only
> > do CEA861 right now it hasn't been an issue, but it will.
>  
>  I prepared a long email about that, but then I realized that we're
>  investing our time intosomething broken, at the light of all DV timing
>  standards. So, I've dropped it and started from scratch.
>  
>  From what I've got, there are some hardware that can only do a limited
>  set of DV timings. If this were not the case, we could simply just use
>  the VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS, and put the CEA 861 and
>  VESA timings into some userspace library.
>  
>  In other words, the PRESET API is meant to solve the case where
>  hardware only support a limited set of frequencies, that may or may
>  not be inside the CEA standard.
>  
>  Let's assume we never added the current API, and discuss how it would
>  properly fulfill the user needs. An API that would likely work is:
>  
>  struct v4l2_dv_enum_preset2 {
>  
>   __u32 index;
>   __u8  name[32]; /* Name of the preset timing */
>   
>   struct v4l2_fract fps;
>  
>  #define DV_PRESET_IS_PROGRESSIVE 1<<31
>  #define DV_PRESET_SPEC(flag) (flag && 0xff)
>  #define DV_PRESET_IS_CEA861  1
>  #define DV_PRESET_IS_DMT 2
>  #define DV_PRESET_IS_CVF 3
>  #define DV_PRESET_IS_GTF 4
>  #define DV_PRESET_IS_VENDOR_SPECIFIC 5
>  
>   __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>   
>   __u32   width;  /* width in pixels */
>   __u32   height; /* height in lines */
>   __u32   polarities; /* Positive or negative polarity */
>   __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>   __u32   hfrontporch;/* Horizpontal front porch in pixels */
>   __u32   hsync;  /* Horizontal Sync length in pixels */
>   __u32   hbackporch; /* Horizontal back porch in pixels */
>   __u32   vfrontporch;/* Vertical front porch in pixels */
>   __u32   vsync;  /* Vertical Sync length in lines */
>   __u32   vbackporch; /* Vertical back porch in lines */
>   __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>   
>    * interlaced field formats
>    */
>   
>   __u32   il_vsync;   /* Vertical sync length for bottom field of
>   
>    * interlaced field formats
>    */
>   
>   __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>   
>    * interlaced field formats
>    */
>   
>   __u32 reserved[4];
>  
>  };
>  
>  #define  VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>  v4l2_dv_enum_preset2)
>  #define  VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>  #define  VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>  
>  Such preset API seems to work for all cases. Userspace can use any DV
>  timing information to select the desired format, and don't need to
>  have a switch for a preset macro to try to guess what the format
>  actually means. Also, there's no need to touch at the API spec every
>  time a new DV timeline is needed.
>  
>  Also, it should be noticed that, since the size of the data on the
>  above definitions are different than the old ones, _IO macros will
>  provide a different magic number, so, adding these won't break the
>  existing API.
>  
>  So, I think we should work on this proposal, and mark the existing one
>  as deprecated.
> >>> 
> >>> This proposal makes it very hard for applic

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 06-07-2011 09:03, Laurent Pinchart escreveu:
> On Wednesday 06 July 2011 13:48:35 Mauro Carvalho Chehab wrote:
>> Em 06-07-2011 08:31, Hans Verkuil escreveu:
 Em 05-07-2011 10:20, Hans Verkuil escreveu:
>> I failed to see what information is provided by the "presets" name. If
>> this were removed from the ioctl, and fps would be added instead, the
>> API would be clearer. The only adjustment would be to use "index" as
>> the preset selection key. Anyway, it is too late for such change. We
>> need to live with that.
>
> Adding the fps solves nothing. Because that still does not give you
> specific timings. You can have 1920x1080P60 that has quite different
> timings from the CEA-861 standard and that may not be supported by a TV.
>
> If you are working with HDMI, then you may want to filter all supported
> presets to those of the CEA standard.
>
> That's one thing that is missing at the moment: that presets belonging
> to a certain standard get their own range. Since we only do CEA861 right
> now it hasn't been an issue, but it will.

 I prepared a long email about that, but then I realized that we're
 investing our time intosomething broken, at the light of all DV timing
 standards. So, I've dropped it and started from scratch.

 From what I've got, there are some hardware that can only do a limited
 set of DV timings. If this were not the case, we could simply just use
 the VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS, and put the CEA 861 and VESA
 timings into some userspace library.

 In other words, the PRESET API is meant to solve the case where hardware
 only support a limited set of frequencies, that may or may not be inside
 the CEA standard.

 Let's assume we never added the current API, and discuss how it would
 properly fulfill the user needs. An API that would likely work is:

 struct v4l2_dv_enum_preset2 {

__u32 index;
__u8  name[32]; /* Name of the preset timing */

struct v4l2_fract fps;

 #define DV_PRESET_IS_PROGRESSIVE   1<<31
 #define DV_PRESET_SPEC(flag)   (flag && 0xff)
 #define DV_PRESET_IS_CEA8611
 #define DV_PRESET_IS_DMT   2
 #define DV_PRESET_IS_CVF   3
 #define DV_PRESET_IS_GTF   4
 #define DV_PRESET_IS_VENDOR_SPECIFIC   5

__u32   flags;  /* Interlaced/progressive, DV specs, etc */

__u32   width;  /* width in pixels */
__u32   height; /* height in lines */
__u32   polarities; /* Positive or negative polarity */
__u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
__u32   hfrontporch;/* Horizpontal front porch in pixels */
__u32   hsync;  /* Horizontal Sync length in pixels */
__u32   hbackporch; /* Horizontal back porch in pixels */
__u32   vfrontporch;/* Vertical front porch in pixels */
__u32   vsync;  /* Vertical Sync length in lines */
__u32   vbackporch; /* Vertical back porch in lines */
__u32   il_vfrontporch; /* Vertical front porch for bottom field of

 * interlaced field formats
 */

__u32   il_vsync;   /* Vertical sync length for bottom field of

 * interlaced field formats
 */

__u32   il_vbackporch;  /* Vertical back porch for bottom field of

 * interlaced field formats
 */

__u32 reserved[4];

 };

 #defineVIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
 v4l2_dv_enum_preset2)
 #defineVIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
 #defineVIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)

 Such preset API seems to work for all cases. Userspace can use any DV
 timing information to select the desired format, and don't need to have a
 switch for a preset macro to try to guess what the format actually means.
 Also, there's no need to touch at the API spec every time a new DV
 timeline is needed.

 Also, it should be noticed that, since the size of the data on the above
 definitions are different than the old ones, _IO macros will provide a
 different magic number, so, adding these won't break the existing API.

 So, I think we should work on this proposal, and mark the existing one
 as deprecated.
>>>
>>> This proposal makes it very hard for applications to directly select a
>>> format like 720p50 because the indices can change at any time.
>>
>> Why? All the application needs to do is to call VIDIOC_ENUM_DV_PRESETS2,
>> check what line it wants, and do a S_DV

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Laurent Pinchart
On Wednesday 06 July 2011 13:48:35 Mauro Carvalho Chehab wrote:
> Em 06-07-2011 08:31, Hans Verkuil escreveu:
> >> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>  I failed to see what information is provided by the "presets" name. If
>  this were removed from the ioctl, and fps would be added instead, the
>  API would be clearer. The only adjustment would be to use "index" as
>  the preset selection key. Anyway, it is too late for such change. We
>  need to live with that.
> >>> 
> >>> Adding the fps solves nothing. Because that still does not give you
> >>> specific timings. You can have 1920x1080P60 that has quite different
> >>> timings from the CEA-861 standard and that may not be supported by a TV.
> >>> 
> >>> If you are working with HDMI, then you may want to filter all supported
> >>> presets to those of the CEA standard.
> >>> 
> >>> That's one thing that is missing at the moment: that presets belonging
> >>> to a certain standard get their own range. Since we only do CEA861 right
> >>> now it hasn't been an issue, but it will.
> >> 
> >> I prepared a long email about that, but then I realized that we're
> >> investing our time intosomething broken, at the light of all DV timing
> >> standards. So, I've dropped it and started from scratch.
> >> 
> >> From what I've got, there are some hardware that can only do a limited
> >> set of DV timings. If this were not the case, we could simply just use
> >> the VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS, and put the CEA 861 and VESA
> >> timings into some userspace library.
> >> 
> >> In other words, the PRESET API is meant to solve the case where hardware
> >> only support a limited set of frequencies, that may or may not be inside
> >> the CEA standard.
> >> 
> >> Let's assume we never added the current API, and discuss how it would
> >> properly fulfill the user needs. An API that would likely work is:
> >> 
> >> struct v4l2_dv_enum_preset2 {
> >> 
> >>__u32 index;
> >>__u8  name[32]; /* Name of the preset timing */
> >>
> >>struct v4l2_fract fps;
> >> 
> >> #define DV_PRESET_IS_PROGRESSIVE   1<<31
> >> #define DV_PRESET_SPEC(flag)   (flag && 0xff)
> >> #define DV_PRESET_IS_CEA8611
> >> #define DV_PRESET_IS_DMT   2
> >> #define DV_PRESET_IS_CVF   3
> >> #define DV_PRESET_IS_GTF   4
> >> #define DV_PRESET_IS_VENDOR_SPECIFIC   5
> >> 
> >>__u32   flags;  /* Interlaced/progressive, DV specs, etc */
> >>
> >>__u32   width;  /* width in pixels */
> >>__u32   height; /* height in lines */
> >>__u32   polarities; /* Positive or negative polarity */
> >>__u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
> >>__u32   hfrontporch;/* Horizpontal front porch in pixels */
> >>__u32   hsync;  /* Horizontal Sync length in pixels */
> >>__u32   hbackporch; /* Horizontal back porch in pixels */
> >>__u32   vfrontporch;/* Vertical front porch in pixels */
> >>__u32   vsync;  /* Vertical Sync length in lines */
> >>__u32   vbackporch; /* Vertical back porch in lines */
> >>__u32   il_vfrontporch; /* Vertical front porch for bottom field of
> >>
> >> * interlaced field formats
> >> */
> >>
> >>__u32   il_vsync;   /* Vertical sync length for bottom field of
> >>
> >> * interlaced field formats
> >> */
> >>
> >>__u32   il_vbackporch;  /* Vertical back porch for bottom field of
> >>
> >> * interlaced field formats
> >> */
> >>
> >>__u32 reserved[4];
> >> 
> >> };
> >> 
> >> #defineVIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
> >> v4l2_dv_enum_preset2)
> >> #defineVIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
> >> #defineVIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
> >> 
> >> Such preset API seems to work for all cases. Userspace can use any DV
> >> timing information to select the desired format, and don't need to have a
> >> switch for a preset macro to try to guess what the format actually means.
> >> Also, there's no need to touch at the API spec every time a new DV
> >> timeline is needed.
> >> 
> >> Also, it should be noticed that, since the size of the data on the above
> >> definitions are different than the old ones, _IO macros will provide a
> >> different magic number, so, adding these won't break the existing API.
> >> 
> >> So, I think we should work on this proposal, and mark the existing one
> >> as deprecated.
> > 
> > This proposal makes it very hard for applications to directly select a
> > format like 720p50 because the indices can change at any time.
> 
> Why? All the application needs to do is to call VIDIOC_ENUM_DV_PRESETS2,
> check what line it wants, and do a S_DV_PRESET2, just like any other place

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 06-07-2011 08:31, Hans Verkuil escreveu:
>> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>>
 I failed to see what information is provided by the "presets" name. If
 this were removed
 from the ioctl, and fps would be added instead, the API would be
 clearer. The only
 adjustment would be to use "index" as the preset selection key. Anyway,
 it is too late
 for such change. We need to live with that.
>>>
>>> Adding the fps solves nothing. Because that still does not give you
>>> specific timings.
>>> You can have 1920x1080P60 that has quite different timings from the
>>> CEA-861 standard
>>> and that may not be supported by a TV.
>>>
>>> If you are working with HDMI, then you may want to filter all supported
>>> presets to
>>> those of the CEA standard.
>>>
>>> That's one thing that is missing at the moment: that presets belonging
>>> to a certain
>>> standard get their own range. Since we only do CEA861 right now it
>>> hasn't been an
>>> issue, but it will.
>>
>> I prepared a long email about that, but then I realized that we're
>> investing our time into
>> something broken, at the light of all DV timing standards. So, I've
>> dropped it and
>> started from scratch.
>>
>> From what I've got, there are some hardware that can only do a limited set
>> of DV timings.
>> If this were not the case, we could simply just use the
>> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
>> and put the CEA 861 and VESA timings into some userspace library.
>>
>> In other words, the PRESET API is meant to solve the case where hardware
>> only support
>> a limited set of frequencies, that may or may not be inside the CEA
>> standard.
>>
>> Let's assume we never added the current API, and discuss how it would
>> properly fulfill
>> the user needs. An API that would likely work is:
>>
>> struct v4l2_dv_enum_preset2 {
>>  __u32 index;
>>  __u8  name[32]; /* Name of the preset timing */
>>
>>  struct v4l2_fract fps;
>>
>> #define DV_PRESET_IS_PROGRESSIVE 1<<31
>> #define DV_PRESET_SPEC(flag) (flag && 0xff)
>> #define DV_PRESET_IS_CEA861  1
>> #define DV_PRESET_IS_DMT 2
>> #define DV_PRESET_IS_CVF 3
>> #define DV_PRESET_IS_GTF 4
>> #define DV_PRESET_IS_VENDOR_SPECIFIC 5
>>
>>  __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>>
>>  __u32   width;  /* width in pixels */
>>  __u32   height; /* height in lines */
>>  __u32   polarities; /* Positive or negative polarity */
>>  __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>>  __u32   hfrontporch;/* Horizpontal front porch in pixels */
>>  __u32   hsync;  /* Horizontal Sync length in pixels */
>>  __u32   hbackporch; /* Horizontal back porch in pixels */
>>  __u32   vfrontporch;/* Vertical front porch in pixels */
>>  __u32   vsync;  /* Vertical Sync length in lines */
>>  __u32   vbackporch; /* Vertical back porch in lines */
>>  __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32   il_vsync;   /* Vertical sync length for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>>   * interlaced field formats
>>   */
>>  __u32 reserved[4];
>> };
>>
>> #define  VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
>> v4l2_dv_enum_preset2)
>> #define  VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
>> #define  VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>>
>> Such preset API seems to work for all cases. Userspace can use any DV
>> timing
>> information to select the desired format, and don't need to have a switch
>> for
>> a preset macro to try to guess what the format actually means. Also,
>> there's no
>> need to touch at the API spec every time a new DV timeline is needed.
>>
>> Also, it should be noticed that, since the size of the data on the above
>> definitions
>> are different than the old ones, _IO macros will provide a different magic
>> number,
>> so, adding these won't break the existing API.
>>
>> So, I think we should work on this proposal, and mark the existing one as
>> deprecated.
> 
> This proposal makes it very hard for applications to directly select a
> format like 720p50 because the indices can change at any time.

Why? All the application needs to do is to call VIDIOC_ENUM_DV_PRESETS2,
check what line it wants, and do a S_DV_PRESET2, just like any other place
where V4L2 defines an ENUM function.

The enum won't change during application runtime, so, they can be stored
if the application would need to switch to other formats latter.

> I think
> this is a very desirable feature, particularly for

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Hans Verkuil
> Em 05-07-2011 10:20, Hans Verkuil escreveu:
>
>>> I failed to see what information is provided by the "presets" name. If
>>> this were removed
>>> from the ioctl, and fps would be added instead, the API would be
>>> clearer. The only
>>> adjustment would be to use "index" as the preset selection key. Anyway,
>>> it is too late
>>> for such change. We need to live with that.
>>
>> Adding the fps solves nothing. Because that still does not give you
>> specific timings.
>> You can have 1920x1080P60 that has quite different timings from the
>> CEA-861 standard
>> and that may not be supported by a TV.
>>
>> If you are working with HDMI, then you may want to filter all supported
>> presets to
>> those of the CEA standard.
>>
>> That's one thing that is missing at the moment: that presets belonging
>> to a certain
>> standard get their own range. Since we only do CEA861 right now it
>> hasn't been an
>> issue, but it will.
>
> I prepared a long email about that, but then I realized that we're
> investing our time into
> something broken, at the light of all DV timing standards. So, I've
> dropped it and
> started from scratch.
>
> From what I've got, there are some hardware that can only do a limited set
> of DV timings.
> If this were not the case, we could simply just use the
> VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
> and put the CEA 861 and VESA timings into some userspace library.
>
> In other words, the PRESET API is meant to solve the case where hardware
> only support
> a limited set of frequencies, that may or may not be inside the CEA
> standard.
>
> Let's assume we never added the current API, and discuss how it would
> properly fulfill
> the user needs. An API that would likely work is:
>
> struct v4l2_dv_enum_preset2 {
>   __u32 index;
>   __u8  name[32]; /* Name of the preset timing */
>
>   struct v4l2_fract fps;
>
> #define DV_PRESET_IS_PROGRESSIVE  1<<31
> #define DV_PRESET_SPEC(flag)  (flag && 0xff)
> #define DV_PRESET_IS_CEA861   1
> #define DV_PRESET_IS_DMT  2
> #define DV_PRESET_IS_CVF  3
> #define DV_PRESET_IS_GTF  4
> #define DV_PRESET_IS_VENDOR_SPECIFIC  5
>
>   __u32   flags;  /* Interlaced/progressive, DV specs, etc */
>
>   __u32   width;  /* width in pixels */
>   __u32   height; /* height in lines */
>   __u32   polarities; /* Positive or negative polarity */
>   __u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
>   __u32   hfrontporch;/* Horizpontal front porch in pixels */
>   __u32   hsync;  /* Horizontal Sync length in pixels */
>   __u32   hbackporch; /* Horizontal back porch in pixels */
>   __u32   vfrontporch;/* Vertical front porch in pixels */
>   __u32   vsync;  /* Vertical Sync length in lines */
>   __u32   vbackporch; /* Vertical back porch in lines */
>   __u32   il_vfrontporch; /* Vertical front porch for bottom field of
>* interlaced field formats
>*/
>   __u32   il_vsync;   /* Vertical sync length for bottom field of
>* interlaced field formats
>*/
>   __u32   il_vbackporch;  /* Vertical back porch for bottom field of
>* interlaced field formats
>*/
>   __u32 reserved[4];
> };
>
> #define   VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct
> v4l2_dv_enum_preset2)
> #define   VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
> #define   VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)
>
> Such preset API seems to work for all cases. Userspace can use any DV
> timing
> information to select the desired format, and don't need to have a switch
> for
> a preset macro to try to guess what the format actually means. Also,
> there's no
> need to touch at the API spec every time a new DV timeline is needed.
>
> Also, it should be noticed that, since the size of the data on the above
> definitions
> are different than the old ones, _IO macros will provide a different magic
> number,
> so, adding these won't break the existing API.
>
> So, I think we should work on this proposal, and mark the existing one as
> deprecated.

This proposal makes it very hard for applications to directly select a
format like 720p50 because the indices can change at any time. I think
this is a very desirable feature, particularly for apps running on
embedded systems where the hardware is known. This was one of the design
considerations at the time this API was made.

But looking at this I wonder if we shouldn't just make a
VIDIOC_G_PRESET_TIMINGS function? You give it the preset ID and you get
all the timing information back. No need to deprecate anything. I'm not
even sure if with this change we need to modify struct v4l2_dv_enum_preset
as I proposed in my RFC, although I thi

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-06 Thread Mauro Carvalho Chehab
Em 05-07-2011 10:20, Hans Verkuil escreveu:
 
>> I failed to see what information is provided by the "presets" name. If this 
>> were removed
>> from the ioctl, and fps would be added instead, the API would be clearer. 
>> The only
>> adjustment would be to use "index" as the preset selection key. Anyway, it 
>> is too late
>> for such change. We need to live with that.
> 
> Adding the fps solves nothing. Because that still does not give you specific 
> timings.
> You can have 1920x1080P60 that has quite different timings from the CEA-861 
> standard
> and that may not be supported by a TV.
> 
> If you are working with HDMI, then you may want to filter all supported 
> presets to
> those of the CEA standard.
> 
> That's one thing that is missing at the moment: that presets belonging to a 
> certain
> standard get their own range. Since we only do CEA861 right now it hasn't 
> been an
> issue, but it will.

I prepared a long email about that, but then I realized that we're investing 
our time into
something broken, at the light of all DV timing standards. So, I've dropped it 
and
started from scratch.

>From what I've got, there are some hardware that can only do a limited set of 
>DV timings.
If this were not the case, we could simply just use the 
VIDIOC_S_DV_TIMINGS/VIDIOC_G_DV_TIMINGS,
and put the CEA 861 and VESA timings into some userspace library.

In other words, the PRESET API is meant to solve the case where hardware only 
support
a limited set of frequencies, that may or may not be inside the CEA standard.

Let's assume we never added the current API, and discuss how it would properly 
fulfill
the user needs. An API that would likely work is:

struct v4l2_dv_enum_preset2 {
__u32 index;
__u8  name[32]; /* Name of the preset timing */

struct v4l2_fract fps;

#define DV_PRESET_IS_PROGRESSIVE1<<31
#define DV_PRESET_SPEC(flag)(flag && 0xff)
#define DV_PRESET_IS_CEA861 1
#define DV_PRESET_IS_DMT2
#define DV_PRESET_IS_CVF3
#define DV_PRESET_IS_GTF4
#define DV_PRESET_IS_VENDOR_SPECIFIC5

__u32   flags;  /* Interlaced/progressive, DV specs, etc */

__u32   width;  /* width in pixels */
__u32   height; /* height in lines */
__u32   polarities; /* Positive or negative polarity */
__u64   pixelclock; /* Pixel clock in HZ. Ex. 74.25MHz->7425 */
__u32   hfrontporch;/* Horizpontal front porch in pixels */
__u32   hsync;  /* Horizontal Sync length in pixels */
__u32   hbackporch; /* Horizontal back porch in pixels */
__u32   vfrontporch;/* Vertical front porch in pixels */
__u32   vsync;  /* Vertical Sync length in lines */
__u32   vbackporch; /* Vertical back porch in lines */
__u32   il_vfrontporch; /* Vertical front porch for bottom field of
 * interlaced field formats
 */
__u32   il_vsync;   /* Vertical sync length for bottom field of
 * interlaced field formats
 */
__u32   il_vbackporch;  /* Vertical back porch for bottom field of
 * interlaced field formats
 */
__u32 reserved[4];
};

#define VIDIOC_ENUM_DV_PRESETS2 _IOWR('V', 83, struct v4l2_dv_enum_preset2)
#define VIDIOC_S_DV_PRESET2 _IOWR('V', 84, u32 index)
#define VIDIOC_G_DV_PRESET2 _IOWR('V', 85, u32 index)

Such preset API seems to work for all cases. Userspace can use any DV timing
information to select the desired format, and don't need to have a switch for
a preset macro to try to guess what the format actually means. Also, there's no
need to touch at the API spec every time a new DV timeline is needed.

Also, it should be noticed that, since the size of the data on the above 
definitions
are different than the old ones, _IO macros will provide a different magic 
number,
so, adding these won't break the existing API.

So, I think we should work on this proposal, and mark the existing one as 
deprecated.

Thanks,
Mauro


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-05 Thread Andy Walls
Mauro Carvalho Chehab  wrote:

>Em 05-07-2011 16:02, Andy Walls escreveu:
>> Hans Verkuil  wrote:
>>> I can work on the proposal this week for that. The only reason the
>fps
>>> hasn't been added
>>> yet is that I never had the time to do the research on how to
>represent
>>> the fps reliably
>>> for all CEA/VESA formats. Hmm, pixelclock / total_framesize should
>>> always work, of course.
>>>
>>> We can add a flags field as well (for interlaced vs progressive and
>>> perhaps others such as
>>> normal vs reduced blanking).
>>>
>>> That leaves the problem with GTF/CVT. I'll get back to that
>tomorrow. I
>>> have ideas, but
>>> I need to discuss it first.
>>>
>>> Regards,
>>>
>>> Hans
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>linux-media"
>>> in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
>> For fps you could use horizontal_line_freq/lines_per_frame.
>> 
>> However, all of the non-integer fps numbers I have seen in this email
>chain all seem to be multiples of 29.97002997 Hz. So maybe you could
>just use the closest integer rate with a flag labeled
>"ntsc_bw_timing_hack" to indicate the fractional rates. :) 
>
>CEA-861 has some other timings that are not 60 Hz * 1000/1001. Yet,
>v4l2_fract
>is capable of handling any of such timings, as, whatever frequency
>taken, it
>needs to be a fractional number. Btw, even some userspace libraries
>prefer to
>represent fps using a fraction, instead of a float, to avoid rounding
>issues.
>
>> 
>> That 29.97 Hz number comes from the NTSC decision in 1953(!) to
>change the horizontal line freq to 4.5 MHz/286.  Note that
>> 
>> (4.5 MHz/286)/525 = 30 * (1000/1001) = 29.97002997 Hz
>
>One of the rationale for that decision was to avoid flicking issues
>with cathodic 
>ray monitors and fluorescent lamps.
> 
>> It is interesting to see one of the most ingenious analog hacks in TV
>history (to achieve color and B&W backward compatabilty while staying
>in the 10% tolerance of the old B&W receivers) being codified in
>digital standards over 50 years later. It boggles the mind...
>
>Yes. Bad (and good) API decisions will stay forever.
>
>Cheers,
>Mauro.

Oops, yes I see the 60.054 Hz rate corresponds to a 262 line field (524 line 
frame) at the standard NTSC line rate.

Weird stuff.

Regards,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-05 Thread Mauro Carvalho Chehab
Em 05-07-2011 16:02, Andy Walls escreveu:
> Hans Verkuil  wrote:
>> I can work on the proposal this week for that. The only reason the fps
>> hasn't been added
>> yet is that I never had the time to do the research on how to represent
>> the fps reliably
>> for all CEA/VESA formats. Hmm, pixelclock / total_framesize should
>> always work, of course.
>>
>> We can add a flags field as well (for interlaced vs progressive and
>> perhaps others such as
>> normal vs reduced blanking).
>>
>> That leaves the problem with GTF/CVT. I'll get back to that tomorrow. I
>> have ideas, but
>> I need to discuss it first.
>>
>> Regards,
>>
>>  Hans
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media"
>> in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> For fps you could use horizontal_line_freq/lines_per_frame.
> 
> However, all of the non-integer fps numbers I have seen in this email chain 
> all seem to be multiples of 29.97002997 Hz. So maybe you could just use the 
> closest integer rate with a flag labeled "ntsc_bw_timing_hack" to indicate 
> the fractional rates. :) 

CEA-861 has some other timings that are not 60 Hz * 1000/1001. Yet, v4l2_fract
is capable of handling any of such timings, as, whatever frequency taken, it
needs to be a fractional number. Btw, even some userspace libraries prefer to
represent fps using a fraction, instead of a float, to avoid rounding issues.

> 
> That 29.97 Hz number comes from the NTSC decision in 1953(!) to change the 
> horizontal line freq to 4.5 MHz/286.  Note that
> 
> (4.5 MHz/286)/525 = 30 * (1000/1001) = 29.97002997 Hz

One of the rationale for that decision was to avoid flicking issues with 
cathodic 
ray monitors and fluorescent lamps.
 
> It is interesting to see one of the most ingenious analog hacks in TV history 
> (to achieve color and B&W backward compatabilty while staying in the 10% 
> tolerance of the old B&W receivers) being codified in digital standards over 
> 50 years later. It boggles the mind...

Yes. Bad (and good) API decisions will stay forever.

Cheers,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-05 Thread Andy Walls
Hans Verkuil  wrote:
>I can work on the proposal this week for that. The only reason the fps
>hasn't been added
>yet is that I never had the time to do the research on how to represent
>the fps reliably
>for all CEA/VESA formats. Hmm, pixelclock / total_framesize should
>always work, of course.
>
>We can add a flags field as well (for interlaced vs progressive and
>perhaps others such as
>normal vs reduced blanking).
>
>That leaves the problem with GTF/CVT. I'll get back to that tomorrow. I
>have ideas, but
>I need to discuss it first.
>
>Regards,
>
>   Hans
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media"
>in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

For fps you could use horizontal_line_freq/lines_per_frame.

However, all of the non-integer fps numbers I have seen in this email chain all 
seem to be multiples of 29.97002997 Hz. So maybe you could just use the closest 
integer rate with a flag labeled "ntsc_bw_timing_hack" to indicate the 
fractional rates. :) 

That 29.97 Hz number comes from the NTSC decision in 1953(!) to change the 
horizontal line freq to 4.5 MHz/286.  Note that

(4.5 MHz/286)/525 = 30 * (1000/1001) = 29.97002997 Hz

It is interesting to see one of the most ingenious analog hacks in TV history 
(to achieve color and B&W backward compatabilty while staying in the 10% 
tolerance of the old B&W receivers) being codified in digital standards over 50 
years later. It boggles the mind...

Regards,
Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-05 Thread Hans Verkuil
On Tuesday, July 05, 2011 14:08:03 Mauro Carvalho Chehab wrote:
> Em 05-07-2011 04:26, Hans Verkuil escreveu:
> > On Monday, July 04, 2011 18:09:18 Mauro Carvalho Chehab wrote:
> >> Em 29-06-2011 09:51, Tomasz Stanislawski escreveu:
> >>> The 1080p59_94 is supported by latest Samsung SoC.
> >>>
> >>> Signed-off-by: Tomasz Stanislawski 
> >>> Signed-off-by: Kyungmin Park 
> >>> Reviewed-by: Hans Verkuil 
> >>> ---
> >>>  drivers/media/video/v4l2-common.c |1 +
> >>>  include/linux/videodev2.h |1 +
> >>>  2 files changed, 2 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/media/video/v4l2-common.c 
> >>> b/drivers/media/video/v4l2-common.c
> >>> index 06b9f9f..003e648 100644
> >>> --- a/drivers/media/video/v4l2-common.c
> >>> +++ b/drivers/media/video/v4l2-common.c
> >>> @@ -582,6 +582,7 @@ int v4l_fill_dv_preset_info(u32 preset, struct 
> >>> v4l2_dv_enum_preset *info)
> >>>   { 1920, 1080, "1080p@30" }, /* V4L2_DV_1080P30 */
> >>>   { 1920, 1080, "1080p@50" }, /* V4L2_DV_1080P50 */
> >>>   { 1920, 1080, "1080p@60" }, /* V4L2_DV_1080P60 */
> >>> + { 1920, 1080, "1080p@59.94" },  /* V4L2_DV_1080P59_94 */
> >>>   };
> >>>  
> >>>   if (info == NULL || preset >= ARRAY_SIZE(dv_presets))
> >>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> >>> index 8a4c309..7c77c4e 100644
> >>> --- a/include/linux/videodev2.h
> >>> +++ b/include/linux/videodev2.h
> >>> @@ -872,6 +872,7 @@ struct v4l2_dv_enum_preset {
> >>>  #define  V4L2_DV_1080P30 16 /* SMPTE 296M */
> >>>  #define  V4L2_DV_1080P50 17 /* BT.1120 */
> >>>  #define  V4L2_DV_1080P60 18 /* BT.1120 */
> >>> +#define  V4L2_DV_1080P59_94  19
> >>>  
> >>>  /*
> >>>   *   D V B T T I M I N G S
> >>
> >> This patch deserves further discussions, as the specs that define the 
> >> presets
> >> are not so clear with respect to 60Hz and 60/1.001 Hz.
> >>
> >> Let me summarize the issue.
> >>
> >>
> >>
> >> 1) PRESET STANDARDS
> >>== =
> >>
> >> There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and 
> >> CEA 861.
> >>
> >> At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz". 
> >> BT.1120
> >> follows the same logic, as it uses BT.709 as a reference for video timings.
> >>
> >> The CEA-861-E spec says at item 4, that:
> >>
> >>A video timing with a vertical frequency that is an integer multiple of 
> >> 6.00 Hz (i.e. 24.00, 30.00, 60.00,
> >>120.00 or 240.00 Hz) is considered to be the same as a video timing 
> >> with the equivalent detailed timing
> >>information but where the vertical frequency is adjusted by a factor of 
> >> 1000/1001 (i.e., 24/1.001, 30/1.001,
> >>60/1.001, 120/1.001 or 240/1.001). That is, they are considered two 
> >> versions of the same video timing but
> >>with slightly different pixel clock frequencies. Therefore, a DTV that 
> >> declares it is capable of displaying a
> >>video timing with a vertical frequency that is either an integer 
> >> multiple of 6 Hz or an integer multiple of 6
> >>Hz adjusted by a factor of 1000/1001 shall be capable of displaying 
> >> both versions of the video timing.
> >>
> >> At the same item, the table 2 describes several video parameters for each 
> >> preset, associating the
> >> Video Identification Codes (VIC) for each preset.
> > 
> > No, *multiple VICs* are associated with each preset. VIC != preset.
> 
> There are two VIC's for several presets, as it is also used at the spec to 
> determine the aspect ratio, but, basically,
> for one given VIC, there's just one timings preset at the table 2.

Sigh. VIC != video timings. It just gives some additional information regarding 
aspect ration
(duplicated information actually).

The simple fact that you can't use it to determine the pixelclock and thus the 
vertical
frequency means that it is *not* suitable as a way to determine the video 
timings.

I've been working with HDMI receivers and transmitters for several years now, 
and
it simply is not suitable.

> > Also, the VICs do not differentiate between 60 and 59.94 Hz.
> 
> Yes, but V4L2 DV timings also don't differentiate.

??? Yes, they do. There is a 720P60 and a 720P59_94. Also 1080I29_97 and 
1080I30.
The 1080P59_94 is simply missing because nobody needed it yet.
 
> >> Table 4 associates each VIC with the supported formats. For example, VIC 
> >> 16 means a resolution of
> >> 1920x1080 at 59.94Hz/60Hz. The spec does explicitly allow that all 
> >> vertical frequencies that are
> >> multiple of 6 Hz to accept both 59.94 Hz and 60 Hz, as said at note 3 of 
> >> table 2:
> >>
> >>3. A video timing with a vertical frequency that is an integer multiple 
> >> of 6.00 Hz (i.e. 24.00, 30.00, 60.00, 120.00 or
> >>240.00 Hz) is considered to be the same as a video timing with the 
> >> equivalent detailed timing information but where
> >>the vertical freque

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-05 Thread Mauro Carvalho Chehab
Em 05-07-2011 04:26, Hans Verkuil escreveu:
> On Monday, July 04, 2011 18:09:18 Mauro Carvalho Chehab wrote:
>> Em 29-06-2011 09:51, Tomasz Stanislawski escreveu:
>>> The 1080p59_94 is supported by latest Samsung SoC.
>>>
>>> Signed-off-by: Tomasz Stanislawski 
>>> Signed-off-by: Kyungmin Park 
>>> Reviewed-by: Hans Verkuil 
>>> ---
>>>  drivers/media/video/v4l2-common.c |1 +
>>>  include/linux/videodev2.h |1 +
>>>  2 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/media/video/v4l2-common.c 
>>> b/drivers/media/video/v4l2-common.c
>>> index 06b9f9f..003e648 100644
>>> --- a/drivers/media/video/v4l2-common.c
>>> +++ b/drivers/media/video/v4l2-common.c
>>> @@ -582,6 +582,7 @@ int v4l_fill_dv_preset_info(u32 preset, struct 
>>> v4l2_dv_enum_preset *info)
>>> { 1920, 1080, "1080p@30" }, /* V4L2_DV_1080P30 */
>>> { 1920, 1080, "1080p@50" }, /* V4L2_DV_1080P50 */
>>> { 1920, 1080, "1080p@60" }, /* V4L2_DV_1080P60 */
>>> +   { 1920, 1080, "1080p@59.94" },  /* V4L2_DV_1080P59_94 */
>>> };
>>>  
>>> if (info == NULL || preset >= ARRAY_SIZE(dv_presets))
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 8a4c309..7c77c4e 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -872,6 +872,7 @@ struct v4l2_dv_enum_preset {
>>>  #defineV4L2_DV_1080P30 16 /* SMPTE 296M */
>>>  #defineV4L2_DV_1080P50 17 /* BT.1120 */
>>>  #defineV4L2_DV_1080P60 18 /* BT.1120 */
>>> +#defineV4L2_DV_1080P59_94  19
>>>  
>>>  /*
>>>   * D V B T T I M I N G S
>>
>> This patch deserves further discussions, as the specs that define the presets
>> are not so clear with respect to 60Hz and 60/1.001 Hz.
>>
>> Let me summarize the issue.
>>
>>
>>
>> 1) PRESET STANDARDS
>>== =
>>
>> There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and CEA 
>> 861.
>>
>> At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz". 
>> BT.1120
>> follows the same logic, as it uses BT.709 as a reference for video timings.
>>
>> The CEA-861-E spec says at item 4, that:
>>
>>  A video timing with a vertical frequency that is an integer multiple of 
>> 6.00 Hz (i.e. 24.00, 30.00, 60.00,
>>  120.00 or 240.00 Hz) is considered to be the same as a video timing 
>> with the equivalent detailed timing
>>  information but where the vertical frequency is adjusted by a factor of 
>> 1000/1001 (i.e., 24/1.001, 30/1.001,
>>  60/1.001, 120/1.001 or 240/1.001). That is, they are considered two 
>> versions of the same video timing but
>>  with slightly different pixel clock frequencies. Therefore, a DTV that 
>> declares it is capable of displaying a
>>  video timing with a vertical frequency that is either an integer 
>> multiple of 6 Hz or an integer multiple of 6
>>  Hz adjusted by a factor of 1000/1001 shall be capable of displaying 
>> both versions of the video timing.
>>
>> At the same item, the table 2 describes several video parameters for each 
>> preset, associating the
>> Video Identification Codes (VIC) for each preset.
> 
> No, *multiple VICs* are associated with each preset. VIC != preset.

There are two VIC's for several presets, as it is also used at the spec to 
determine the aspect ratio, but, basically,
for one given VIC, there's just one timings preset at the table 2.

> Also, the VICs do not differentiate between 60 and 59.94 Hz.

Yes, but V4L2 DV timings also don't differentiate.

>> Table 4 associates each VIC with the supported formats. For example, VIC 16 
>> means a resolution of
>> 1920x1080 at 59.94Hz/60Hz. The spec does explicitly allow that all vertical 
>> frequencies that are
>> multiple of 6 Hz to accept both 59.94 Hz and 60 Hz, as said at note 3 of 
>> table 2:
>>
>>  3. A video timing with a vertical frequency that is an integer multiple 
>> of 6.00 Hz (i.e. 24.00, 30.00, 60.00, 120.00 or
>>  240.00 Hz) is considered to be the same as a video timing with the 
>> equivalent detailed timing information but where
>>  the vertical frequency is adjusted by a factor of 1000/1001 (i.e., 
>> 24/1.001, 30/1.001, 60/1.001, 120/1.001 or
>>  240/1.001). That is, they are considered two versions of the same video 
>> timing but with slightly different pixel clock
>>  frequencies. The vertical frequencies of the 240p, 480p, and 480i video 
>> formats are typically adjusted by a factor of
>>  exactly 1000/1001 for NTSC video compatibility, while the 576p, 576i, 
>> and the HDTV video formats are not. The
>>  VESA DMT standard [65] specifies a ± 0.5% pixel clock frequency 
>> tolerance. Therefore, the nominally 25.175 MHz
>>  pixel clock frequency value given for video identification code 1 may 
>> be adjusted to 25.2 MHz to obtain an exact 60
>>  Hz vertical frequency.
>>
>> In other words, the preset for

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-05 Thread Hans Verkuil
On Monday, July 04, 2011 18:09:18 Mauro Carvalho Chehab wrote:
> Em 29-06-2011 09:51, Tomasz Stanislawski escreveu:
> > The 1080p59_94 is supported by latest Samsung SoC.
> > 
> > Signed-off-by: Tomasz Stanislawski 
> > Signed-off-by: Kyungmin Park 
> > Reviewed-by: Hans Verkuil 
> > ---
> >  drivers/media/video/v4l2-common.c |1 +
> >  include/linux/videodev2.h |1 +
> >  2 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/media/video/v4l2-common.c 
> > b/drivers/media/video/v4l2-common.c
> > index 06b9f9f..003e648 100644
> > --- a/drivers/media/video/v4l2-common.c
> > +++ b/drivers/media/video/v4l2-common.c
> > @@ -582,6 +582,7 @@ int v4l_fill_dv_preset_info(u32 preset, struct 
> > v4l2_dv_enum_preset *info)
> > { 1920, 1080, "1080p@30" }, /* V4L2_DV_1080P30 */
> > { 1920, 1080, "1080p@50" }, /* V4L2_DV_1080P50 */
> > { 1920, 1080, "1080p@60" }, /* V4L2_DV_1080P60 */
> > +   { 1920, 1080, "1080p@59.94" },  /* V4L2_DV_1080P59_94 */
> > };
> >  
> > if (info == NULL || preset >= ARRAY_SIZE(dv_presets))
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index 8a4c309..7c77c4e 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -872,6 +872,7 @@ struct v4l2_dv_enum_preset {
> >  #defineV4L2_DV_1080P30 16 /* SMPTE 296M */
> >  #defineV4L2_DV_1080P50 17 /* BT.1120 */
> >  #defineV4L2_DV_1080P60 18 /* BT.1120 */
> > +#defineV4L2_DV_1080P59_94  19
> >  
> >  /*
> >   * D V B T T I M I N G S
> 
> This patch deserves further discussions, as the specs that define the presets
> are not so clear with respect to 60Hz and 60/1.001 Hz.
> 
> Let me summarize the issue.
> 
> 
> 
> 1) PRESET STANDARDS
>== =
> 
> There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and CEA 
> 861.
> 
> At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz". 
> BT.1120
> follows the same logic, as it uses BT.709 as a reference for video timings.
> 
> The CEA-861-E spec says at item 4, that:
> 
>   A video timing with a vertical frequency that is an integer multiple of 
> 6.00 Hz (i.e. 24.00, 30.00, 60.00,
>   120.00 or 240.00 Hz) is considered to be the same as a video timing 
> with the equivalent detailed timing
>   information but where the vertical frequency is adjusted by a factor of 
> 1000/1001 (i.e., 24/1.001, 30/1.001,
>   60/1.001, 120/1.001 or 240/1.001). That is, they are considered two 
> versions of the same video timing but
>   with slightly different pixel clock frequencies. Therefore, a DTV that 
> declares it is capable of displaying a
>   video timing with a vertical frequency that is either an integer 
> multiple of 6 Hz or an integer multiple of 6
>   Hz adjusted by a factor of 1000/1001 shall be capable of displaying 
> both versions of the video timing.
> 
> At the same item, the table 2 describes several video parameters for each 
> preset, associating the
> Video Identification Codes (VIC) for each preset.

No, *multiple VICs* are associated with each preset. VIC != preset.

Also, the VICs do not differentiate between 60 and 59.94 Hz.

> Table 4 associates each VIC with the supported formats. For example, VIC 16 
> means a resolution of
> 1920x1080 at 59.94Hz/60Hz. The spec does explicitly allow that all vertical 
> frequencies that are
> multiple of 6 Hz to accept both 59.94 Hz and 60 Hz, as said at note 3 of 
> table 2:
> 
>   3. A video timing with a vertical frequency that is an integer multiple 
> of 6.00 Hz (i.e. 24.00, 30.00, 60.00, 120.00 or
>   240.00 Hz) is considered to be the same as a video timing with the 
> equivalent detailed timing information but where
>   the vertical frequency is adjusted by a factor of 1000/1001 (i.e., 
> 24/1.001, 30/1.001, 60/1.001, 120/1.001 or
>   240/1.001). That is, they are considered two versions of the same video 
> timing but with slightly different pixel clock
>   frequencies. The vertical frequencies of the 240p, 480p, and 480i video 
> formats are typically adjusted by a factor of
>   exactly 1000/1001 for NTSC video compatibility, while the 576p, 576i, 
> and the HDTV video formats are not. The
>   VESA DMT standard [65] specifies a ± 0.5% pixel clock frequency 
> tolerance. Therefore, the nominally 25.175 MHz
>   pixel clock frequency value given for video identification code 1 may 
> be adjusted to 25.2 MHz to obtain an exact 60
>   Hz vertical frequency.
> 
> In other words, the preset for 1920x1080p@60Hz can be used for both 60Hz and 
> 59.94 Hz,
> according with the above note, being 59.94 Hz the typical value (e. g. the 
> value that
> should be used on most places).
> 
> However, there are some "60 Hz" vertical resolutions that have VIC's with 
> different framerates (like 59.94Hz, 60.054Hz, etc). Those se

Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-04 Thread Hans Verkuil
On Tuesday, July 05, 2011 00:47:43 Laurent Pinchart wrote:
> Hi Mauro,
> 
> On Monday 04 July 2011 18:09:18 Mauro Carvalho Chehab wrote:
> 
> [snip]
> 
> > 1) PRESET STANDARDS
> >== =
> > 
> > There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and
> > CEA 861.
> > 
> > At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz".
> > BT.1120 follows the same logic, as it uses BT.709 as a reference for video
> > timings.
> > 
> > The CEA-861-E spec says at item 4, that:
> 
> [snip]
> 
> > At the same item, the table 2 describes several video parameters for each
> > preset, associating the Video Identification Codes (VIC) for each preset.
> 
> This might be a bit out of scope, but why aren't we using the VICs as DV 
> presets ?

The VIC does more than just set the timings. It also determines the pixel
aspect ratio. So exactly the same video timings may have two VICs, the only
difference being the pixel aspect which is *not* part of the timings. The VIC
is part of the AVI InfoFrame, however.

So VIC != timings.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-04 Thread Mauro Carvalho Chehab
Em 04-07-2011 19:47, Laurent Pinchart escreveu:
> Hi Mauro,
> 
> On Monday 04 July 2011 18:09:18 Mauro Carvalho Chehab wrote:
> 
> [snip]
> 
>> 1) PRESET STANDARDS
>>== =
>>
>> There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and
>> CEA 861.
>>
>> At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz".
>> BT.1120 follows the same logic, as it uses BT.709 as a reference for video
>> timings.
>>
>> The CEA-861-E spec says at item 4, that:
> 
> [snip]
> 
>> At the same item, the table 2 describes several video parameters for each
>> preset, associating the Video Identification Codes (VIC) for each preset.
> 
> This might be a bit out of scope, but why aren't we using the VICs as DV 
> presets ?

I had the same question after analyzing the specs ;)

That's said, abstracting from the spec could be a good idea if we have newer
versions of the spec re-defining the VICs.

Maybe the right thing to do would be to rename the presets as:

V4L2_DV_CEA861_VIC_16
V4L2_DV_CEA861_VIC_35_36
...

(or course, preserving the old names with compatibility macros)

Cheers,
Mauro

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-04 Thread Laurent Pinchart
Hi Mauro,

On Monday 04 July 2011 18:09:18 Mauro Carvalho Chehab wrote:

[snip]

> 1) PRESET STANDARDS
>== =
> 
> There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and
> CEA 861.
> 
> At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz".
> BT.1120 follows the same logic, as it uses BT.709 as a reference for video
> timings.
> 
> The CEA-861-E spec says at item 4, that:

[snip]

> At the same item, the table 2 describes several video parameters for each
> preset, associating the Video Identification Codes (VIC) for each preset.

This might be a bit out of scope, but why aren't we using the VICs as DV 
presets ?

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC] DV timings spec fixes at V4L2 API - was: [PATCH 1/8] v4l: add macro for 1080p59_54 preset

2011-07-04 Thread Mauro Carvalho Chehab
Em 29-06-2011 09:51, Tomasz Stanislawski escreveu:
> The 1080p59_94 is supported by latest Samsung SoC.
> 
> Signed-off-by: Tomasz Stanislawski 
> Signed-off-by: Kyungmin Park 
> Reviewed-by: Hans Verkuil 
> ---
>  drivers/media/video/v4l2-common.c |1 +
>  include/linux/videodev2.h |1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/v4l2-common.c 
> b/drivers/media/video/v4l2-common.c
> index 06b9f9f..003e648 100644
> --- a/drivers/media/video/v4l2-common.c
> +++ b/drivers/media/video/v4l2-common.c
> @@ -582,6 +582,7 @@ int v4l_fill_dv_preset_info(u32 preset, struct 
> v4l2_dv_enum_preset *info)
>   { 1920, 1080, "1080p@30" }, /* V4L2_DV_1080P30 */
>   { 1920, 1080, "1080p@50" }, /* V4L2_DV_1080P50 */
>   { 1920, 1080, "1080p@60" }, /* V4L2_DV_1080P60 */
> + { 1920, 1080, "1080p@59.94" },  /* V4L2_DV_1080P59_94 */
>   };
>  
>   if (info == NULL || preset >= ARRAY_SIZE(dv_presets))
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 8a4c309..7c77c4e 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -872,6 +872,7 @@ struct v4l2_dv_enum_preset {
>  #define  V4L2_DV_1080P30 16 /* SMPTE 296M */
>  #define  V4L2_DV_1080P50 17 /* BT.1120 */
>  #define  V4L2_DV_1080P60 18 /* BT.1120 */
> +#define  V4L2_DV_1080P59_94  19
>  
>  /*
>   *   D V B T T I M I N G S

This patch deserves further discussions, as the specs that define the presets
are not so clear with respect to 60Hz and 60/1.001 Hz.

Let me summarize the issue.



1) PRESET STANDARDS
   == =

There are 3 specs involved with DV presets: ITU-R BT 709 and BT 1120 and CEA 
861.

At ITU-R BT.709, both 60Hz and 60/1.001 Hz are equally called as "60 Hz". 
BT.1120
follows the same logic, as it uses BT.709 as a reference for video timings.

The CEA-861-E spec says at item 4, that:

A video timing with a vertical frequency that is an integer multiple of 
6.00 Hz (i.e. 24.00, 30.00, 60.00,
120.00 or 240.00 Hz) is considered to be the same as a video timing 
with the equivalent detailed timing
information but where the vertical frequency is adjusted by a factor of 
1000/1001 (i.e., 24/1.001, 30/1.001,
60/1.001, 120/1.001 or 240/1.001). That is, they are considered two 
versions of the same video timing but
with slightly different pixel clock frequencies. Therefore, a DTV that 
declares it is capable of displaying a
video timing with a vertical frequency that is either an integer 
multiple of 6 Hz or an integer multiple of 6
Hz adjusted by a factor of 1000/1001 shall be capable of displaying 
both versions of the video timing.

At the same item, the table 2 describes several video parameters for each 
preset, associating the
Video Identification Codes (VIC) for each preset.

Table 4 associates each VIC with the supported formats. For example, VIC 16 
means a resolution of
1920x1080 at 59.94Hz/60Hz. The spec does explicitly allow that all vertical 
frequencies that are
multiple of 6 Hz to accept both 59.94 Hz and 60 Hz, as said at note 3 of table 
2:

3. A video timing with a vertical frequency that is an integer multiple 
of 6.00 Hz (i.e. 24.00, 30.00, 60.00, 120.00 or
240.00 Hz) is considered to be the same as a video timing with the 
equivalent detailed timing information but where
the vertical frequency is adjusted by a factor of 1000/1001 (i.e., 
24/1.001, 30/1.001, 60/1.001, 120/1.001 or
240/1.001). That is, they are considered two versions of the same video 
timing but with slightly different pixel clock
frequencies. The vertical frequencies of the 240p, 480p, and 480i video 
formats are typically adjusted by a factor of
exactly 1000/1001 for NTSC video compatibility, while the 576p, 576i, 
and the HDTV video formats are not. The
VESA DMT standard [65] specifies a ± 0.5% pixel clock frequency 
tolerance. Therefore, the nominally 25.175 MHz
pixel clock frequency value given for video identification code 1 may 
be adjusted to 25.2 MHz to obtain an exact 60
Hz vertical frequency.

In other words, the preset for 1920x1080p@60Hz can be used for both 60Hz and 
59.94 Hz,
according with the above note, being 59.94 Hz the typical value (e. g. the 
value that
should be used on most places).

However, there are some "60 Hz" vertical resolutions that have VIC's with 
different framerates (like 59.94Hz, 60.054Hz, etc). Those seem to not be
covered by the "multiple of 6.00 Hz" rule.

2. V4L2 API
    ===

The V4L2 specs define a DV timing as having those fields:

__u32   width   Width of the active video in pixels
__u32   height  Height of the active video in lines
__u32   interlaced  Progressive (0) or interlaced (1)
__u32   polarities  This is a bit mask that defin