2013/9/6 Vittorio Giovara <[email protected]>
> Il giorno giovedì 5 settembre 2013, Yusuke Nakamura ha scritto:
>
> > ---
> >
> > Sorry for parsing wrong output_picture_number since I was under the
> > impression that the prev_ values are set in ff_init_poc().
> > This patch decodes up to MMCOs to set the prev_ values correctly if
> needed.
> >
> >
> I think this is a small behavioral change and it should be documented
> somewhere, if possible.
>
> libavcodec/h264_parser.c | 142
> > ++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 140 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
> > index ef5da98..56d0d4a 100644
> > --- a/libavcodec/h264_parser.c
> > +++ b/libavcodec/h264_parser.c
> > @@ -106,6 +106,7 @@ static inline int
> parse_nal_units(AVCodecParserContext
> > *s,
> > int state = -1;
> > const uint8_t *ptr;
> > int field_poc[2];
> > + MMCOOpcode opcode;
> >
> > /* set some sane default values */
> > s->pict_type = AV_PICTURE_TYPE_I;
> > @@ -132,8 +133,14 @@ static inline int
> > parse_nal_units(AVCodecParserContext *s,
> > case NAL_SLICE:
> > case NAL_IDR_SLICE:
> > // Do not walk the whole buffer just to decode slice header
> > - if (src_length > 20)
> > - src_length = 20;
> > + if (state & 0x1f == NAL_IDR_SLICE || (state >> 5) & 0x3 ==
> 0)
> > {
> > + if (src_length > 50)
> > + src_length = 50;
> > + } else {
> > + /* To decode up to MMCO */
> > + if (src_length > 1000)
> > + src_length = 1000;
> > + }
>
>
> Where did you get this numbers from? If they come from the internal mmco
> file you could try reference that.
>
>
I count manually the approximate possible maximum number of bits according
to the spec.
Oh, I didn't consider emulation_prevention_three_bytes.
Maybe, 60 is enough safe number without decoding up to MMCOs.
pred_weight_table() could cost a lot of bits; at most ~6700 bits.
1000 is the enough safety number I concluded together with other parts.
> break;
> > }
> > ptr = ff_h264_decode_nal(h, buf, &dst_length, &consumed,
> > src_length);
> > @@ -219,8 +226,139 @@ static inline int
> > parse_nal_units(AVCodecParserContext *s,
> > h->delta_poc[1] = get_se_golomb(&h->gb);
> > }
> >
> > + field_poc[0] = field_poc[1] = INT_MAX;
> > +
> > ff_init_poc(h, field_poc, &s->output_picture_number);
> >
> > + opcode = MMCO_END;
> > + if (h->nal_ref_idc && h->nal_unit_type != NAL_IDR_SLICE) {
> > + /* Continue parsing to check if MMCO_RESET is present.
> */
> > + unsigned int slice_type_nos = s->pict_type & 3;
> > + int max_refs = h->picture_structure == PICT_FRAME ? 16 :
> > 32;
> > +
> > + if (h->pps.redundant_pic_cnt_present)
> > + get_ue_golomb(&h->gb); // redundant_pic_count
> > +
> > + h->ref_count[0] = h->pps.ref_count[0];
> > + h->ref_count[1] = h->pps.ref_count[1];
> > + if (slice_type_nos != AV_PICTURE_TYPE_I) {
> > + if (slice_type_nos == AV_PICTURE_TYPE_B)
> > + get_bits1(&h->gb); // direct_spatial_mv_pred
> > +
> > + if (get_bits1(&h->gb)) { //
> > num_ref_idx_active_override_flag
> > + h->ref_count[0] = get_ue_golomb(&h->gb) + 1;
> > + if (h->ref_count[0] < 1)
> > + return AVERROR_INVALIDDATA;
> > + if (slice_type_nos == AV_PICTURE_TYPE_B) {
> > + h->ref_count[1] = get_ue_golomb(&h->gb) + 1;
> > + if (h->ref_count[1] < 1)
> > + return AVERROR_INVALIDDATA;
> > + }
> > + }
> > +
> > + if (slice_type_nos == AV_PICTURE_TYPE_B)
> > + h->list_count = 2;
> > + else
> > + h->list_count = 1;
> > + } else {
> > + h->list_count = 0;
> > + h->ref_count[0] = h->ref_count[1] = 0;
> > + }
> > +
> > + if (h->ref_count[0] > max_refs || h->ref_count[1] >
> > max_refs) {
> > + av_log(h->avctx, AV_LOG_ERROR, "reference
> > overflow\n");
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + if (slice_type_nos != AV_PICTURE_TYPE_I) {
> > + int list;
> > + for (list = 0; list < h->list_count; list++) {
> > + if (get_bits1(&h->gb)) {
> > + int index;
> > + for (index = 0; ; index++) {
> > + unsigned int reordering_of_pic_nums_idc
> =
> > get_ue_golomb_31(&h->gb);
> > +
> > + if (reordering_of_pic_nums_idc < 3)
> > + get_ue_golomb(&h->gb);
> > + else if (reordering_of_pic_nums_idc >
> 3) {
> > + av_log(h->avctx, AV_LOG_ERROR,
> > + "illegal
> > reordering_of_pic_nums_idc %d\n",
> > + reordering_of_pic_nums_idc);
> > + return AVERROR_INVALIDDATA;
> > + }
> > + else
> > + break;
>
>
> Keep else and { on the same line}
>
>
done locally.
> > +
> > + if (index >= h->ref_count[list]) {
> > + av_log(h->avctx, AV_LOG_ERROR,
> > "reference count overflow\n");
> > + return AVERROR_INVALIDDATA;
> > + }
> > + }
> > + }
> > + }
> > + }
> > +
> > + if ((h->pps.weighted_pred && slice_type_nos ==
> > AV_PICTURE_TYPE_P) ||
> > + (h->pps.weighted_bipred_idc == 1 && slice_type_nos
> ==
> > AV_PICTURE_TYPE_B)) {
> > + int list;
> > +
> > + get_ue_golomb(&h->gb); // luma_log2_weight_denom
> > + if (h->sps.chroma_format_idc)
> > + get_ue_golomb(&h->gb); //
> chroma_log2_weight_denom
> > + for (list = 0; list < h->list_count; list++) {
> > + int i;
> > + for (i = 0; i < h->ref_count[list]; i++) {
> > + if (get_bits1(&h->gb)) { // luma_weight_flag
> > + get_se_golomb(&h->gb); // luma_weight
> > + get_se_golomb(&h->gb); // luma_offset
> > +
> > + if (h->sps.chroma_format_idc) {
> > + if(get_bits1(&h->gb)) { //
> > chroma_weight_flag
>
>
> Space before (
>
>
done locally.
> > + int j;
> > + for (j = 0; j < 2; j++) {
> > + get_se_golomb(&h->gb); //
> > chroma_weight
> > + get_se_golomb(&h->gb); //
> > chroma_offset
> > + }
> > + }
> > + }
> > + }
> > + }
> > + }
> > + }
> > +
> > + if (get_bits1(&h->gb)) { //
> > adaptive_ref_pic_marking_mode_flag
> > + int i;
> > + for (i = 0; i < MAX_MMCO_COUNT; i++) {
> > + opcode = get_ue_golomb_31(&h->gb);
> > + if (opcode > (unsigned) MMCO_LONG) {
> > + av_log(h->avctx, AV_LOG_ERROR,
> > + "illegal memory management control
> > operation %d\n",
> > + opcode);
> > + return AVERROR_INVALIDDATA;
> > + }
> > + if (opcode == MMCO_END || opcode == MMCO_RESET)
> > + break;
> > +
> > + if (opcode == MMCO_SHORT2UNUSED || opcode ==
> > MMCO_SHORT2LONG)
> > + get_ue_golomb(&h->gb);
> > + if (opcode == MMCO_SHORT2LONG || opcode ==
> > MMCO_LONG2UNUSED ||
> > + opcode == MMCO_LONG || opcode ==
> > MMCO_SET_MAX_LONG)
> > + get_ue_golomb_31(&h->gb);
> > + }
> > + }
> > + }
> > +
> > + h->prev_frame_num = h->frame_num;
> > + h->prev_frame_num_offset = opcode != MMCO_RESET ?
> > h->frame_num_offset : 0;
> > + if (h->nal_ref_idc != 0) {
> > + h->prev_poc_msb = h->poc_msb;
> > + if (opcode != MMCO_RESET)
> > + h->prev_poc_lsb = h->poc_lsb;
> > + else
> > + h->prev_poc_lsb =
> > + h->picture_structure == PICT_BOTTOM_FIELD ? 0 :
> > field_poc[0];
> > + }
> > +
> > if (h->sps.pic_struct_present_flag) {
> > switch (h->sei_pic_struct) {
> > case SEI_PIC_STRUCT_TOP_FIELD:
> > --
>
>
> The rest seems ok although I wouldn't mind a secon opinion from more
> experienced h264/mmco devs.
>
> Yusuke, could you provide one h264 sample that triggers this code path in
> some way?
>
>
Sorry, I haven't found any sample affected in MMCO path.
But, without MMCO path, the part of setting of prev_ values affects enough
length samples.
My fault was that I didn't test enough length sample.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
