[FFmpeg-devel] [PATCH 1/3] mpegvideo_parser: implement parsing of the picture structure field

[Michael Niedermayer]

On Mon, Feb 12, 2018 at 05:37:32AM +0900, Yusuke Nakamura wrote:
> 2018-02-11 23:37 GMT+09:00 Jan Ekström <jeebjp at gmail.com>:
> 
> > From: Masaki Tanaka <maki.rxrz at gmail.com>
> >
> > Lets one receive the proper field order from pictures coded in
> > field picture mode, until now forcibly set to BFF.
> > ---
> >  libavcodec/mpegvideo_parser.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
> > index be240b6890..3406346a8b 100644
> > --- a/libavcodec/mpegvideo_parser.c
> > +++ b/libavcodec/mpegvideo_parser.c
> > @@ -41,7 +41,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext
> > *s,
> >      uint32_t start_code;
> >      int frame_rate_index, ext_type, bytes_left;
> >      int frame_rate_ext_n, frame_rate_ext_d;
> > -    int top_field_first, repeat_first_field, progressive_frame;
> > +    int picture_structure, top_field_first, repeat_first_field,
> > progressive_frame;
> >      int horiz_size_ext, vert_size_ext, bit_rate_ext;
> >      int did_set_size=0;
> >      int set_dim_ret = 0;
> > @@ -51,6 +51,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext
> > *s,
> >      enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE;
> >  //FIXME replace the crap with get_bits()
> >      s->repeat_pict = 0;
> > +    s->picture_structure = AV_PICTURE_STRUCTURE_UNKNOWN;
> >
> >      while (buf < buf_end) {
> >          start_code= -1;
> > @@ -114,6 +115,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext
> > *s,
> >                      break;
> >                  case 0x8: /* picture coding extension */
> >                      if (bytes_left >= 5) {
> > +                        picture_structure = buf[2] & 0x03;
> >                          top_field_first = buf[3] & (1 << 7);
> >                          repeat_first_field = buf[3] & (1 << 1);
> >                          progressive_frame = buf[4] & (1 << 7);
> > @@ -138,6 +140,18 @@ static void mpegvideo_extract_headers(AVCodecParserContext
> > *s,
> >                                  s->field_order = AV_FIELD_BB;
> >                          } else
> >                              s->field_order = AV_FIELD_PROGRESSIVE;
> > +
> > +                        switch (picture_structure) {
> > +                        case PICT_TOP_FIELD:
> > +                            s->picture_structure =
> > AV_PICTURE_STRUCTURE_TOP_FIELD;
> > +                            break;
> > +                        case PICT_BOTTOM_FIELD:
> > +                            s->picture_structure =
> > AV_PICTURE_STRUCTURE_BOTTOM_FIELD;
> > +                            break;
> > +                        case PICT_FRAME:
> > +                            s->picture_structure =
> > AV_PICTURE_STRUCTURE_FRAME;
> > +                            break;
> > +                        }
> >
> 
> Libavcodec handles MPEG-2 Video packet per frame but not picture, and the
> parser stops immediately after parsing the first slice of the picture. So,
> the parser returns per full frame but picture_structure says it's a field
> picture. This is evil and  this is the reason why I hesitate to post this
> patch. I'm interested in how other devs consider this evil solution. I
> think the parser should parse the second field too if encountering a field
> coded picture, and treat the packet as frame coded and set field_order
> appropriate.

I think a better API is needed to export the picture_structure correctly.

With the API here, i think unspecified is better than delcaring field pictures
to be frames. The field value is also what decoders would use, they have to
2 field pics arent the same as a frame picture

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180211/87948b36/attachment.sig>