Re: [FFmpeg-devel] [PATCH v2] mpegvideo_parser: check picture_structure for field order

[Tom Yan <tom.ty89@gmail.com>]

This is really just a matter of choice. If you don't care about
spec-conformance in this particular case, top_field_first *could* be 1
and/or picture_structure *could* be not 3 even when
progressive_sequence is 1, so which one do you choose to believe? The
truth is, it doesn't even matter.

If you insist on making it more clumsy and silly, I can resend.

On Sun, 6 Feb 2022 at 13:46, Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Tom Yan:
> > the top_field_first bit is only used to indicate the field order
> > when the picture is a frame picture (which consists of two fields)
> > but not when it is a field picture (which consists of one single
> > top or bottom field).
> >
> > also removing the unnecessary progressive_sequence check (the bit
> > is mandated to be 0 if progressive_frame is 0 on any picture in the
> > sequence).
> >
>
> Just because something is mandated does not mean that it is so;
> spec-incompliant files exist.
>
> > Signed-off-by: Tom Yan <tom.ty89@gmail.com>
> > ---
> >  libavcodec/mpegvideo_parser.c | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
> > index c5dc867d24..004ff602f6 100644
> > --- a/libavcodec/mpegvideo_parser.c
> > +++ b/libavcodec/mpegvideo_parser.c
> > @@ -181,6 +181,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
> >                      break;
> >                  case 0x8: /* picture coding extension */
> >                      if (bytes_left >= 5) {
> > +                        s->picture_structure = buf[2] & 0x3;
> >                          top_field_first = buf[3] & (1 << 7);
> >                          repeat_first_field = buf[3] & (1 << 1);
> >                          progressive_frame = buf[4] & (1 << 7);
> > @@ -198,13 +199,15 @@ static void mpegvideo_extract_headers(AVCodecParserContext *s,
> >                              }
> >                          }
> >
> > -                        if (!pc->progressive_sequence && !progressive_frame) {
> > -                            if (top_field_first)
> > -                                s->field_order = AV_FIELD_TT;
> > -                            else
> > -                                s->field_order = AV_FIELD_BB;
> > -                        } else
> > +                        if (progressive_frame)
> >                              s->field_order = AV_FIELD_PROGRESSIVE;
> > +                        else if (top_field_first ||
> > +                                 /* top_field_first is mandated to be 0 when
> > +                                    the picture is not a frame picture) */
> > +                                 s->picture_structure == AV_PICTURE_STRUCTURE_TOP_FIELD)
> > +                            s->field_order = AV_FIELD_TT;
> > +                        else
> > +                            s->field_order = AV_FIELD_BB;
> >                      }
> >                      break;
> >                  }
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".