[vlc-devel] [PATCH 1/9] video_format: add the number of fields per picture in the video format

[Steve Lhomme]

On 2020-11-16 14:00, Romain Vimont wrote:
> On Mon, Nov 16, 2020 at 01:44:12PM +0100, Steve Lhomme wrote:
>> In general it's 2 fields. If the number of fields changes in the source, the
>> (deinterlacing) filters should be updated.
>>
>> By default the video format is progressive and the i_num_fields can be ignored.
>> It should be set to 2 for progressive to be consistent with how the value was
>> set so far.
>>
>> This field is more stable than the picture i_nb_fields that also included
>> repeated fields and pictures. It only contains the information about the
>> number of fields in each actual picture.
>> ---
>>   include/vlc_es.h                      | 3 +++
>>   modules/visualization/visual/visual.c | 1 +
>>   2 files changed, 4 insertions(+)
>>
>> diff --git a/include/vlc_es.h b/include/vlc_es.h
>> index 404a937d833..459d16b3c37 100644
>> --- a/include/vlc_es.h
>> +++ b/include/vlc_es.h
>> @@ -376,6 +376,8 @@ struct video_format_t
>>       video_multiview_mode_t multiview_mode;        /** Multiview mode, 2D, 3D */
>>       bool b_multiview_right_eye_first;   /** Multiview left or right eye first*/
>>   
>> +    unsigned int    i_num_fields;          /**< number of fields per picture */
> 
> I think there are only 2 cases:
>   - either all fields are interleaved in a single picture
>   - either the fields are split into separate pictures

Well, a picture could actually have more than 2 fields. It can be 2 and 
repeat the first field. I have yet to understand how this works in the 
real world.

> 
> So IMO using an integer adds complexity (more cases to handle) for no
> reason. I suggest something like a 'bool interlaced_split_fields'
> instead.
> 
> Also, the interlaced/progressive flag should also be in the
> video_format_t (as you suggested in another thread): having 1 field when
> the video is progressive is not the same as having 1 field when the
> video is interlaced.

That's why I said progressive sources should use a value of 2 (which 
they already do in the picture_t field). That makes things simpler. A 
value of 1 means it's interlaced and with one field at a time. That's 
what allows patch 5/9 to trigger the deinterlacer in that case.

> Regards
> 
>> +
>>       video_projection_mode_t projection_mode;            /**< projection mode */
>>       vlc_viewpoint_t pose;
>>       struct {
>> @@ -402,6 +404,7 @@ static inline void video_format_Init( video_format_t *p_src, vlc_fourcc_t i_chro
>>   {
>>       memset( p_src, 0, sizeof( video_format_t ) );
>>       p_src->i_chroma = i_chroma;
>> +    p_src->i_num_fields = 2;
>>       vlc_viewpoint_init( &p_src->pose );
>>   }
>>   
>> diff --git a/modules/visualization/visual/visual.c b/modules/visualization/visual/visual.c
>> index 13343d0f045..8a35b0ddcc6 100644
>> --- a/modules/visualization/visual/visual.c
>> +++ b/modules/visualization/visual/visual.c
>> @@ -309,6 +309,7 @@ static int Open( vlc_object_t *p_this )
>>           .transfer = TRANSFER_FUNC_SRGB,
>>           .primaries = COLOR_PRIMARIES_SRGB,
>>           .space = COLOR_SPACE_SRGB,
>> +        .i_num_fields = 2,
>>       };
>>       p_sys->p_vout = aout_filter_GetVout( p_filter, &fmt );
>>       if( p_sys->p_vout == NULL )
>> -- 
>> 2.26.2
>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>