2013/5/8 Yusuke Nakamura <[email protected]>

> 2013/5/8 Janne Grunau <[email protected]>
>
>> On 2013-04-25 06:58:05 +0900, Yusuke Nakamura wrote:
>> > 2013/4/15 Luca Barbato <[email protected]>
>> >
>> > I got some opinions about the H.264 one by Anton and Janne on IRC, and I
>> > took them in a new patch.
>>
>> > From 0013dc39354577b015e8971f6ebacb037914756f Mon Sep 17 00:00:00 2001
>> > From: Yusuke Nakamura <[email protected]>
>> > Date: Tue, 9 Apr 2013 16:46:29 +0900
>> > Subject: [PATCH 4/4] h264_parser: Set field_order.
>> >
>> > ---
>> >  libavcodec/h264.c        |   12 +++++-----
>> >  libavcodec/h264.h        |    1 +
>> >  libavcodec/h264_parser.c |   51
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> >  3 files changed, 58 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/libavcodec/h264.c b/libavcodec/h264.c
>> > index 38e471d..c53ea13 100644
>> > --- a/libavcodec/h264.c
>> > +++ b/libavcodec/h264.c
>> > @@ -2642,11 +2642,10 @@ static void flush_dpb(AVCodecContext *avctx)
>> >      h->parse_context.last_index        = 0;
>> >  }
>> >
>> > -static int init_poc(H264Context *h)
>> > +int ff_init_poc(H264Context *h, int pic_field_poc[2], int *pic_poc)
>> >  {
>> >      const int max_frame_num = 1 << h->sps.log2_max_frame_num;
>> >      int field_poc[2];
>> > -    Picture *cur = h->cur_pic_ptr;
>> >
>> >      h->frame_num_offset = h->prev_frame_num_offset;
>> >      if (h->frame_num < h->prev_frame_num)
>> > @@ -2711,10 +2710,11 @@ static int init_poc(H264Context *h)
>> >      }
>> >
>> >      if (h->picture_structure != PICT_BOTTOM_FIELD)
>> > -        h->cur_pic_ptr->field_poc[0] = field_poc[0];
>> > +        pic_field_poc[0] = field_poc[0];
>> >      if (h->picture_structure != PICT_TOP_FIELD)
>> > -        h->cur_pic_ptr->field_poc[1] = field_poc[1];
>> > -    cur->poc = FFMIN(cur->field_poc[0], cur->field_poc[1]);
>> > +        pic_field_poc[1] = field_poc[1];
>> > +    if (pic_poc)
>> > +        *pic_poc = FFMIN(pic_field_poc[0], pic_field_poc[1]);
>> >
>> >      return 0;
>> >  }
>> > @@ -3516,7 +3516,7 @@ static int decode_slice_header(H264Context *h,
>> H264Context *h0)
>> >              h->delta_poc[1] = get_se_golomb(&h->gb);
>> >      }
>> >
>> > -    init_poc(h);
>> > +    ff_init_poc(h, h->cur_pic_ptr->field_poc, &h->cur_pic_ptr->poc);
>> >
>> >      if (h->pps.redundant_pic_cnt_present)
>> >          h->redundant_pic_count = get_ue_golomb(&h->gb);
>> > diff --git a/libavcodec/h264.h b/libavcodec/h264.h
>> > index 484c9d3..3ef8420 100644
>> > --- a/libavcodec/h264.h
>> > +++ b/libavcodec/h264.h
>> > @@ -949,5 +949,6 @@ static av_always_inline int
>> get_dct8x8_allowed(H264Context *h)
>> >  }
>> >
>> >  void ff_h264_draw_horiz_band(H264Context *h, int y, int height);
>> > +int ff_init_poc(H264Context *h, int pic_field_poc[2], int *pic_poc);
>> >
>> >  #endif /* AVCODEC_H264_H */
>> > diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
>> > index 3c98c84..cbc40ff 100644
>> > --- a/libavcodec/h264_parser.c
>> > +++ b/libavcodec/h264_parser.c
>> > @@ -115,6 +115,7 @@ static inline int
>> parse_nal_units(AVCodecParserContext *s,
>> >      unsigned int slice_type;
>> >      int state = -1;
>> >      const uint8_t *ptr;
>> > +    int field_poc[2];
>> >
>> >      /* set some sane default values */
>> >      s->pict_type = AV_PICTURE_TYPE_I;
>> > @@ -161,6 +162,11 @@ static inline int
>> parse_nal_units(AVCodecParserContext *s,
>> >              break;
>> >          case NAL_IDR_SLICE:
>> >              s->key_frame = 1;
>> > +
>> > +            h->prev_frame_num        = 0;
>> > +            h->prev_frame_num_offset = 0;
>> > +            h->prev_poc_msb          =
>> > +            h->prev_poc_lsb          = 0;
>> >              /* fall through */
>> >          case NAL_SLICE:
>> >              get_ue_golomb(&h->gb);  // skip first_mb_in_slice
>> > @@ -200,6 +206,24 @@ static inline int
>> parse_nal_units(AVCodecParserContext *s,
>> >                  }
>> >              }
>> >
>> > +            if (h->nal_unit_type == NAL_IDR_SLICE)
>> > +                get_ue_golomb(&h->gb); /* idr_pic_id */
>> > +            if (h->sps.poc_type == 0) {
>> > +                h->poc_lsb = get_bits(&h->gb, h->sps.log2_max_poc_lsb);
>> > +
>> > +                if (h->pps.pic_order_present == 1 &&
>> h->picture_structure == PICT_FRAME)
>> > +                    h->delta_poc_bottom = get_se_golomb(&h->gb);
>> > +            }
>> > +
>> > +            if (h->sps.poc_type == 1 &&
>> !h->sps.delta_pic_order_always_zero_flag) {
>> > +                h->delta_poc[0] = get_se_golomb(&h->gb);
>> > +
>> > +                if (h->pps.pic_order_present == 1 &&
>> h->picture_structure == PICT_FRAME)
>> > +                    h->delta_poc[1] = get_se_golomb(&h->gb);
>> > +            }
>> > +
>> > +            ff_init_poc(h, field_poc, NULL);
>> > +
>> >              if(h->sps.pic_struct_present_flag) {
>> >                  switch (h->sei_pic_struct) {
>> >                      case SEI_PIC_STRUCT_TOP_FIELD:
>> > @@ -225,8 +249,35 @@ static inline int
>> parse_nal_units(AVCodecParserContext *s,
>> >                          s->repeat_pict = h->picture_structure ==
>> PICT_FRAME ? 1 : 0;
>> >                          break;
>> >                  }
>> > +
>> > +                switch (h->sei_pic_struct) {
>> > +                    case SEI_PIC_STRUCT_TOP_FIELD:
>>
>> Doesn't the parser split separate fields into separate frames/packets?
>> Setting field order in this case wouldn't make much sense then. It would
>> change from AV_FIELD_TT to AV_FIELD_BB with each parsed field.
>>
>> > +                    case SEI_PIC_STRUCT_TOP_BOTTOM:
>> > +                    case SEI_PIC_STRUCT_TOP_BOTTOM_TOP:
>> > +                        s->field_order = AV_FIELD_TT;
>> > +                        break;
>> > +                    case SEI_PIC_STRUCT_BOTTOM_FIELD:
>>
>> see above
>>
>> > +                    case SEI_PIC_STRUCT_BOTTOM_TOP:
>> > +                    case SEI_PIC_STRUCT_BOTTOM_TOP_BOTTOM:
>> > +                        s->field_order = AV_FIELD_BB;
>> > +                        break;
>> > +                    default:
>> > +                        s->field_order = AV_FIELD_PROGRESSIVE;
>> > +                        break;
>> > +                }
>> >              } else {
>> >                  s->repeat_pict = h->picture_structure == PICT_FRAME ?
>> 1 : 0;
>> > +
>> > +                if (h->picture_structure != PICT_FRAME)
>> > +                    s->field_order = h->picture_structure ==
>> PICT_TOP_FIELD ? AV_FIELD_TT : AV_FIELD_BB;
>>
>> and here too
>>
>
> Yes, the parser split separate field coded pictures into separate packets.
> I don't think this case wouldn't make much sense.
> If field order doesn't change per each parsed field, we can detect
> dropping of another field.
>
>
OK. I misunderstood what Janne wants to say.
Field order is defined in not field but frame.

I replaced the H.264 one with two new patches to indicate additionally that
a picture is coded as frame or field.
For field coded pictures, I couldn't know field_order for a pair of both
top and bottom fields without delay since output order is governed by POC
and I think two consecutive field coded pictures in coded order are not
always output in the same order.
So, field_order is set to AV_FIELD_UNKNOWN for field coded pictures.
Tell me if I'm wrong i.e. it is illegal that two consecutive field coded
pictures in coded order could be output in different order.
(The focus is that the case of TTBB in coded order and TBTB in output order
is allowed or not.)

Attachment: 0001-avcodec-Add-picture-structure-information-to-AVCodec.patch
Description: Binary data

Attachment: 0002-h264_parser-Set-field_order-and-picture_structure.patch
Description: Binary data

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to