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

[Andreas Rheinhardt]

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 at 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;
>                  }