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.
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}
> +
> + 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 (
> + 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?
Thank you.
Vittorio
1.8.3.msysgit.0
>
> _______________________________________________
> libav-devel mailing list
> [email protected] <javascript:;>
> https://lists.libav.org/mailman/listinfo/libav-devel
>
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel