From 356f91c5ba99880b2b4bd452b98fee089b9f763e Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Mon, 17 Nov 2014 11:25:48 +0100 Subject: [PATCH 1/5] AVC/h.264 common: refactor timestamping code --- src/common/mpeg4_p10.cpp | 109 ++++++++++++++++++++++----------------- src/common/mpeg4_p10.h | 5 +- 2 files changed, 65 insertions(+), 49 deletions(-) diff --git a/src/common/mpeg4_p10.cpp b/src/common/mpeg4_p10.cpp index 49d78107a..e1c61729a 100644 --- a/src/common/mpeg4_p10.cpp +++ b/src/common/mpeg4_p10.cpp @@ -992,6 +992,7 @@ mpeg4::p10::avc_es_parser_c::avc_es_parser_c() , m_have_incomplete_frame(false) , m_ignore_nalu_size_length_errors(false) , m_discard_actual_frames(false) + , m_simple_picture_order{} , m_debug_keyframe_detection{"avc_parser|avc_keyframe_detection"} , m_debug_nalu_types{ "avc_parser|avc_nalu_types"} , m_debug_timecodes{ "avc_parser|avc_timecodes"} @@ -1586,55 +1587,24 @@ mpeg4::p10::avc_es_parser_c::get_most_often_used_duration() } void -mpeg4::p10::avc_es_parser_c::cleanup() { - auto s_debug_force_simple_picture_order = debugging_option_c{"avc_parser_force_simple_picture_order"}; +mpeg4::p10::avc_es_parser_c::calculate_frame_order() { + auto frames_begin = m_frames.begin(); + auto frames_end = m_frames.end(); + auto frame_itr = frames_begin; - if (m_frames.empty()) - return; + auto const &idr = frame_itr->m_si; + auto const &sps = m_sps_info_list[idr.sps]; - if (m_discard_actual_frames) { - m_stats.num_frames_discarded += m_frames.size(); - m_stats.num_timecodes_discarded += m_provided_timecodes.size(); - - m_frames.clear(); - m_provided_timecodes.clear(); - m_provided_stream_positions.clear(); - - return; - } - - auto frames_begin = m_frames.begin(); - auto frames_end = m_frames.end(); - auto frame_itr = frames_begin; - - // This may be wrong but is needed for mkvmerge to work correctly - // (cluster_helper etc). - frame_itr->m_keyframe = true; - - slice_info_t &idr = frame_itr->m_si; - sps_info_t &sps = m_sps_info_list[idr.sps]; - bool simple_picture_order = false; - - if ( ( (AVC_SLICE_TYPE_I != idr.type) - && (AVC_SLICE_TYPE_SI != idr.type) - && (AVC_SLICE_TYPE2_I != idr.type) - && (AVC_SLICE_TYPE2_SI != idr.type)) - || (0 == idr.nal_ref_idc) - || (0 != sps.pic_order_cnt_type)) { - simple_picture_order = true; - // return; - } - - unsigned int idx = 0; - unsigned int prev_pic_order_cnt_msb = 0; - unsigned int prev_pic_order_cnt_lsb = 0; - unsigned int pic_order_cnt_msb = 0; + auto idx = 0u; + auto prev_pic_order_cnt_msb = 0u; + auto prev_pic_order_cnt_lsb = 0u; + auto pic_order_cnt_msb = 0u; while (frames_end != frame_itr) { - slice_info_t &si = frame_itr->m_si; + auto const &si = frame_itr->m_si; if (si.sps != idr.sps) { - simple_picture_order = true; + m_simple_picture_order = true; break; } @@ -1661,17 +1631,54 @@ mpeg4::p10::avc_es_parser_c::cleanup() { prev_pic_order_cnt_msb = pic_order_cnt_msb; } } +} - if (!simple_picture_order && !s_debug_force_simple_picture_order) +void +mpeg4::p10::avc_es_parser_c::cleanup() { + auto s_debug_force_simple_picture_order = debugging_option_c{"avc_parser_force_simple_picture_order"}; + + if (m_frames.empty()) + return; + + if (m_discard_actual_frames) { + m_stats.num_frames_discarded += m_frames.size(); + m_stats.num_timecodes_discarded += m_provided_timecodes.size(); + + m_frames.clear(); + m_provided_timecodes.clear(); + m_provided_stream_positions.clear(); + + return; + } + + auto const &idr = m_frames.begin()->m_si; + auto const &sps = m_sps_info_list[idr.sps]; + m_simple_picture_order = false; + + if ( ( (AVC_SLICE_TYPE_I != idr.type) + && (AVC_SLICE_TYPE_SI != idr.type) + && (AVC_SLICE_TYPE2_I != idr.type) + && (AVC_SLICE_TYPE2_SI != idr.type)) + || (0 == idr.nal_ref_idc) + || (0 != sps.pic_order_cnt_type)) { + m_simple_picture_order = true; + // return; + } + + calculate_frame_order(); + + if (!m_simple_picture_order && !s_debug_force_simple_picture_order) brng::sort(m_frames, [](const avc_frame_t &f1, const avc_frame_t &f2) { return f1.m_presentation_order < f2.m_presentation_order; }); brng::sort(m_provided_timecodes); - frames_begin = m_frames.begin(); - frames_end = m_frames.end(); + auto frames_begin = m_frames.begin(); + auto frames_end = m_frames.end(); + auto frame_itr = frames_begin; auto previous_frame_itr = frames_begin; auto provided_timecode_itr = m_provided_timecodes.begin(); - for (frame_itr = frames_begin; frames_end != frame_itr; ++frame_itr) { + + while (frames_end != frame_itr) { if (frame_itr->m_has_provided_timecode) { frame_itr->m_start = *provided_timecode_itr; ++provided_timecode_itr; @@ -1687,6 +1694,7 @@ mpeg4::p10::avc_es_parser_c::cleanup() { frame_itr->m_end = frame_itr->m_start + duration_for(frame_itr->m_si); previous_frame_itr = frame_itr; + ++frame_itr; } m_max_timecode = m_frames.back().m_end; @@ -1698,12 +1706,17 @@ mpeg4::p10::avc_es_parser_c::cleanup() { })); - if (!simple_picture_order) + if (!m_simple_picture_order) brng::sort(m_frames, [](const avc_frame_t &f1, const avc_frame_t &f2) { return f1.m_decode_order < f2.m_decode_order; }); frames_begin = m_frames.begin(); frames_end = m_frames.end(); previous_frame_itr = frames_begin; + + // This may be wrong but is needed for mkvmerge to work correctly + // (cluster_helper etc). + frames_begin->m_keyframe = true; + for (frame_itr = frames_begin; frames_end != frame_itr; ++frame_itr) { if (frames_begin != frame_itr) frame_itr->m_ref1 = previous_frame_itr->m_start - frame_itr->m_start; diff --git a/src/common/mpeg4_p10.h b/src/common/mpeg4_p10.h index 5e0745d94..06a865e7f 100644 --- a/src/common/mpeg4_p10.h +++ b/src/common/mpeg4_p10.h @@ -161,6 +161,7 @@ struct avc_frame_t { bool m_keyframe, m_has_provided_timecode; slice_info_t m_si; int m_presentation_order, m_decode_order; + bool m_order_calculated; avc_frame_t() { clear(); @@ -179,6 +180,7 @@ struct avc_frame_t { m_has_provided_timecode = false; m_presentation_order = 0; m_decode_order = 0; + m_order_calculated = false; m_data.reset(); m_si.clear(); @@ -261,7 +263,7 @@ protected: bool m_have_incomplete_frame; std::deque m_unhandled_nalus; - bool m_ignore_nalu_size_length_errors, m_discard_actual_frames; + bool m_ignore_nalu_size_length_errors, m_discard_actual_frames, m_simple_picture_order; debugging_option_c m_debug_keyframe_detection, m_debug_nalu_types, m_debug_timecodes, m_debug_sps_info, m_debug_trailing_zero_byte_removal; std::map m_nalu_names_by_type; @@ -417,6 +419,7 @@ protected: memory_cptr create_nalu_with_size(const memory_cptr &src, bool add_extra_data = false); void remove_trailing_zero_bytes(memory_c &memory); void init_nalu_names(); + void calculate_frame_order(); }; typedef std::shared_ptr avc_es_parser_cptr; From 5c54f887f7c643056ef5d55f84b34c30c2cae821 Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Mon, 17 Nov 2014 13:00:21 +0100 Subject: [PATCH 2/5] AVC/h.264: only forcefully flag the very first frame as an I frame --- src/common/mpeg4_p10.cpp | 6 +++++- src/common/mpeg4_p10.h | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/common/mpeg4_p10.cpp b/src/common/mpeg4_p10.cpp index e1c61729a..7a3b5761b 100644 --- a/src/common/mpeg4_p10.cpp +++ b/src/common/mpeg4_p10.cpp @@ -993,6 +993,7 @@ mpeg4::p10::avc_es_parser_c::avc_es_parser_c() , m_ignore_nalu_size_length_errors(false) , m_discard_actual_frames(false) , m_simple_picture_order{} + , m_first_cleanup{true} , m_debug_keyframe_detection{"avc_parser|avc_keyframe_detection"} , m_debug_nalu_types{ "avc_parser|avc_nalu_types"} , m_debug_timecodes{ "avc_parser|avc_timecodes"} @@ -1715,7 +1716,10 @@ mpeg4::p10::avc_es_parser_c::cleanup() { // This may be wrong but is needed for mkvmerge to work correctly // (cluster_helper etc). - frames_begin->m_keyframe = true; + if (m_first_cleanup) { + frames_begin->m_keyframe = true; + m_first_cleanup = false; + } for (frame_itr = frames_begin; frames_end != frame_itr; ++frame_itr) { if (frames_begin != frame_itr) diff --git a/src/common/mpeg4_p10.h b/src/common/mpeg4_p10.h index 06a865e7f..61b7b79f8 100644 --- a/src/common/mpeg4_p10.h +++ b/src/common/mpeg4_p10.h @@ -263,7 +263,7 @@ protected: bool m_have_incomplete_frame; std::deque m_unhandled_nalus; - bool m_ignore_nalu_size_length_errors, m_discard_actual_frames, m_simple_picture_order; + bool m_ignore_nalu_size_length_errors, m_discard_actual_frames, m_simple_picture_order, m_first_cleanup; debugging_option_c m_debug_keyframe_detection, m_debug_nalu_types, m_debug_timecodes, m_debug_sps_info, m_debug_trailing_zero_byte_removal; std::map m_nalu_names_by_type; From 36b7a6514ece090bc4ed1d280e8054432007185e Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Mon, 17 Nov 2014 16:28:08 +0100 Subject: [PATCH 3/5] AVC/h.264 common: refactor timestamping code --- src/common/mpeg4_p10.cpp | 22 +++++++++++++++------- src/common/mpeg4_p10.h | 16 +++++++++++++++- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/common/mpeg4_p10.cpp b/src/common/mpeg4_p10.cpp index 7a3b5761b..f06404348 100644 --- a/src/common/mpeg4_p10.cpp +++ b/src/common/mpeg4_p10.cpp @@ -987,6 +987,7 @@ mpeg4::p10::avc_es_parser_c::avc_es_parser_c() , m_b_frames_since_keyframe(false) , m_par_found(false) , m_max_timecode(0) + , m_previous_frame_start_in_display_order{} , m_stream_position(0) , m_parsed_position(0) , m_have_incomplete_frame(false) @@ -1259,6 +1260,7 @@ mpeg4::p10::avc_es_parser_c::handle_slice_nalu(memory_cptr &nalu) { || ( is_i_slice && ( (m_debug_keyframe_detection && !m_b_frames_since_keyframe) || (NALU_TYPE_IDR_SLICE == si.nalu_type))); + m_incomplete_frame.m_type = m_incomplete_frame.m_keyframe ? 'I' : is_b_slice ? 'B' : 'P'; m_recovery_point_valid = false; if (m_incomplete_frame.m_keyframe) { @@ -1267,8 +1269,11 @@ mpeg4::p10::avc_es_parser_c::handle_slice_nalu(memory_cptr &nalu) { if (!si.field_pic_flag || !si.bottom_field_flag) cleanup(); - } else - m_b_frames_since_keyframe |= is_b_slice; + } else if (is_b_slice) + m_b_frames_since_keyframe = true; + + // else if (!si.field_pic_flag || !si.bottom_field_flag) + // cleanup(); m_incomplete_frame.m_data = create_nalu_with_size(nalu, true); m_have_incomplete_frame = true; @@ -1712,7 +1717,6 @@ mpeg4::p10::avc_es_parser_c::cleanup() { frames_begin = m_frames.begin(); frames_end = m_frames.end(); - previous_frame_itr = frames_begin; // This may be wrong but is needed for mkvmerge to work correctly // (cluster_helper etc). @@ -1721,11 +1725,15 @@ mpeg4::p10::avc_es_parser_c::cleanup() { m_first_cleanup = false; } - for (frame_itr = frames_begin; frames_end != frame_itr; ++frame_itr) { - if (frames_begin != frame_itr) - frame_itr->m_ref1 = previous_frame_itr->m_start - frame_itr->m_start; + // mxinfo(boost::format("frame order calculation\n")); - previous_frame_itr = frame_itr; + for (frame_itr = frames_begin; frames_end != frame_itr; ++frame_itr) { + // mxinfo(boost::format(" type %4% decode order %1% presentation order %2% timestamp %3%\n") % frame_itr->m_decode_order % frame_itr->m_presentation_order % format_timecode(frame_itr->m_start) % frame_itr->m_type); + + if (!frame_itr->is_i_frame() && (frames_begin != frame_itr)) + frame_itr->m_ref1 = m_previous_frame_start_in_display_order - frame_itr->m_start; + + m_previous_frame_start_in_display_order = frame_itr->m_start; m_duration_frequency[frame_itr->m_end - frame_itr->m_start]++; if (frame_itr->m_si.field_pic_flag) diff --git a/src/common/mpeg4_p10.h b/src/common/mpeg4_p10.h index 61b7b79f8..109b6cc08 100644 --- a/src/common/mpeg4_p10.h +++ b/src/common/mpeg4_p10.h @@ -161,6 +161,7 @@ struct avc_frame_t { bool m_keyframe, m_has_provided_timecode; slice_info_t m_si; int m_presentation_order, m_decode_order; + char m_type; bool m_order_calculated; avc_frame_t() { @@ -180,11 +181,24 @@ struct avc_frame_t { m_has_provided_timecode = false; m_presentation_order = 0; m_decode_order = 0; + m_type = '?'; m_order_calculated = false; m_data.reset(); m_si.clear(); } + + bool is_i_frame() const { + return 'I' == m_type; + } + + bool is_p_frame() const { + return 'P' == m_type; + } + + bool is_b_frame() const { + return 'B' == m_type; + } }; class nalu_size_length_x: public mtx::exception { @@ -249,7 +263,7 @@ protected: std::deque m_frames, m_frames_out; std::deque m_provided_timecodes; std::deque m_provided_stream_positions; - int64_t m_max_timecode; + int64_t m_max_timecode, m_previous_frame_start_in_display_order; std::map m_duration_frequency; std::vector m_sps_list, m_pps_list, m_extra_data; From 04acfff20f4d716ea8f7ab1c4c7db5940c8dc5d3 Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Wed, 19 Nov 2014 12:13:51 +0100 Subject: [PATCH 4/5] mkvinfo: report picture type based on number of references, not on their value --- ChangeLog | 6 ++++++ src/info/mkvinfo.cpp | 24 ++++++++++-------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index ac840fdca..872c5b852 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2014-11-19 Moritz Bunkus + + * mkvinfo: bug fix: summary mode: reported frame types in block + groups are now derived from the number of references found and not + by the references' values. + 2014-11-16 Moritz Bunkus * mkvmerge: bug fix: Fixed muxing open GOPs after I frames in diff --git a/src/info/mkvinfo.cpp b/src/info/mkvinfo.cpp index b613475ca..591c2a6fb 100644 --- a/src/info/mkvinfo.cpp +++ b/src/info/mkvinfo.cpp @@ -1148,9 +1148,7 @@ handle_block_group(EbmlStream *&es, std::vector frame_adlers; std::vector frame_hexdumps; - bool bref_found = false; - bool fref_found = false; - + auto num_references = 0u; int64_t lf_timecode = 0; int64_t lf_tnum = 0; int64_t frame_pos = 0; @@ -1199,17 +1197,15 @@ handle_block_group(EbmlStream *&es, show_element(l3, 3, BF_BLOCK_GROUP_DURATION % (duration * s_tc_scale / 1000000) % (duration * s_tc_scale % 1000000)); } else if (Is(l3)) { + ++num_references; + int64_t reference = static_cast(l3)->GetValue() * s_tc_scale; - if (0 >= reference) { - bref_found = true; - reference *= -1; - show_element(l3, 3, BF_BLOCK_GROUP_REFERENCE_1 % (reference / 1000000) % (reference % 1000000)); + if (0 >= reference) + show_element(l3, 3, BF_BLOCK_GROUP_REFERENCE_1 % (std::abs(reference) / 1000000) % (std::abs(reference) % 1000000)); - } else if (0 < reference) { - fref_found = true; + else if (0 < reference) show_element(l3, 3, BF_BLOCK_GROUP_REFERENCE_2 % (reference / 1000000) % (reference % 1000000)); - } } else if (Is(l3)) show_element(l3, 3, BF_BLOCK_GROUP_REFERENCE_PRIORITY % static_cast(l3)->GetValue()); @@ -1294,7 +1290,7 @@ handle_block_group(EbmlStream *&es, if (bduration != -1.0) mxinfo(BF_BLOCK_GROUP_SUMMARY_WITH_DURATION - % (bref_found && fref_found ? 'B' : bref_found ? 'P' : !fref_found ? 'I' : 'P') + % (num_references >= 2 ? 'B' : num_references == 1 ? 'P' : 'I') % lf_tnum % (lf_timecode / 1000000) % format_timecode(lf_timecode, 3) @@ -1305,7 +1301,7 @@ handle_block_group(EbmlStream *&es, % position); else mxinfo(BF_BLOCK_GROUP_SUMMARY_NO_DURATION - % (bref_found && fref_found ? 'B' : bref_found ? 'P' : !fref_found ? 'I' : 'P') + % (num_references >= 2 ? 'B' : num_references == 1 ? 'P' : 'I') % lf_tnum % (lf_timecode / 1000000) % format_timecode(lf_timecode, 3) @@ -1318,14 +1314,14 @@ handle_block_group(EbmlStream *&es, } else if (g_options.m_verbose > 2) show_element(nullptr, 2, BF_BLOCK_GROUP_SUMMARY_V2 - % (bref_found && fref_found ? 'B' : bref_found ? 'P' : !fref_found ? 'I' : 'P') + % (num_references >= 2 ? 'B' : num_references == 1 ? 'P' : 'I') % lf_tnum % (lf_timecode / 1000000)); track_info_t &tinfo = s_track_info[lf_tnum]; tinfo.m_blocks += frame_sizes.size(); - tinfo.m_blocks_by_ref_num[bref_found && fref_found ? 2 : bref_found ? 1 : !fref_found ? 0 : 1] += frame_sizes.size(); + tinfo.m_blocks_by_ref_num[std::min(num_references, 2u)] += frame_sizes.size(); tinfo.m_min_timecode = std::min(tinfo.m_min_timecode, lf_timecode); tinfo.m_size += boost::accumulate(frame_sizes, 0); From 31c59559a4287f193031bc8e41af1dca537fc64f Mon Sep 17 00:00:00 2001 From: Moritz Bunkus Date: Wed, 19 Nov 2014 12:36:41 +0100 Subject: [PATCH 5/5] cosmetics: alignment --- src/info/mkvinfo.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/info/mkvinfo.cpp b/src/info/mkvinfo.cpp index 591c2a6fb..b12ef5d09 100644 --- a/src/info/mkvinfo.cpp +++ b/src/info/mkvinfo.cpp @@ -1320,10 +1320,10 @@ handle_block_group(EbmlStream *&es, track_info_t &tinfo = s_track_info[lf_tnum]; - tinfo.m_blocks += frame_sizes.size(); + tinfo.m_blocks += frame_sizes.size(); tinfo.m_blocks_by_ref_num[std::min(num_references, 2u)] += frame_sizes.size(); - tinfo.m_min_timecode = std::min(tinfo.m_min_timecode, lf_timecode); - tinfo.m_size += boost::accumulate(frame_sizes, 0); + tinfo.m_min_timecode = std::min(tinfo.m_min_timecode, lf_timecode); + tinfo.m_size += boost::accumulate(frame_sizes, 0); if (!tinfo.max_timecode_unset() && (tinfo.m_max_timecode >= lf_timecode)) return;