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

[Romain Vimont]

On Mon, Nov 16, 2020 at 03:49:22PM +0100, Steve Lhomme wrote:
> On 2020-11-16 15:38, 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 */
> > 
> > The number of fields may vary per picture:
> > https://code.videolan.org/videolan/vlc/-/blob/a708c988091d6bc770212f5deeaba45e2395f4b2/modules/video_filter/deinterlace/deinterlace.c#L223-224
> 
> This is not what it means. In a picture buffer you never have 3 fields (3
> heights of a field). You either have 1, 2, 2 plus the first one is supposed
> to be repeated (telecine). That's what the "avcodec: repeat the pictures
> marked as repeating" patch assumes.
> 
> In telecine the field first is supposed to flip with every picture that has
> the repeat flag because of that. One picture has TopBottom(+repeat Top) and
> the next picture will have BottomTop. That's why the field order is per
> picture and not in the video format.
> 
> (I have yet to find a sample where this is properly coded)
> 
> The field I propose above is the number of field in the picture buffer. The
> picture_t can still have more. But that's not going to change the
> deinterlacing mode like this video format field.

OK.

(I still don't like an integer "num fields" here, but I guess I have no
real arguments beyond personal preference.)

Regards