[2/5] vaapi_h265: Fix slice header writing

Message ID b14022ec-031c-dd01-4c6c-4c0890b977bb@jkqxz.net
State Committed
Headers show

Commit Message

Mark Thompson Sept. 30, 2016, 4:56 p.m.
This was not observed earlier because the only syntax element which
it normally misses with the current setup is slice_qp_delta, but that
is always going to be zero (in IDR frames QP isn't varied on the
slice) which will always exp-golomb code as a single 1 bit.  The
immediately following part is the byte alignment, which is always a 1
bit followed by 0s which are ignored, so as long as the bitstream is
never aligned at that point we will never notice because the only
difference is that an ignored bit is a 1 instead of a 0.
---
 libavcodec/vaapi_encode_h265.c | 116 ++++++++++++++++++++---------------------
 1 file changed, 58 insertions(+), 58 deletions(-)

Comments

Luca Barbato Sept. 30, 2016, 6:02 p.m. | #1
On 30/09/16 18:56, Mark Thompson wrote:
> This was not observed earlier because the only syntax element which
> it normally misses with the current setup is slice_qp_delta, but that
> is always going to be zero (in IDR frames QP isn't varied on the
> slice) which will always exp-golomb code as a single 1 bit.  The
> immediately following part is the byte alignment, which is always a 1
> bit followed by 0s which are ignored, so as long as the bitstream is
> never aligned at that point we will never notice because the only
> difference is that an ignored bit is a 1 instead of a 0.
> ---




Sure.

Patch

diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
index 7cd9782..db339aa 100644
--- a/libavcodec/vaapi_encode_h265.c
+++ b/libavcodec/vaapi_encode_h265.c
@@ -616,78 +616,78 @@  static void vaapi_encode_h265_write_slice_header2(PutBitContext *pbc,
             if (mseq->long_term_ref_pics_present_flag) {
                 av_assert0(0);
             }
+        }

-            if (vseq->seq_fields.bits.sps_temporal_mvp_enabled_flag) {
-                u(1, vslice_field(slice_temporal_mvp_enabled_flag));
-            }
+        if (vseq->seq_fields.bits.sps_temporal_mvp_enabled_flag) {
+            u(1, vslice_field(slice_temporal_mvp_enabled_flag));
+        }

-            if (vseq->seq_fields.bits.sample_adaptive_offset_enabled_flag) {
-                u(1, vslice_field(slice_sao_luma_flag));
-                if (!vseq->seq_fields.bits.separate_colour_plane_flag &&
-                   vseq->seq_fields.bits.chroma_format_idc != 0) {
-                    u(1, vslice_field(slice_sao_chroma_flag));
-                }
+        if (vseq->seq_fields.bits.sample_adaptive_offset_enabled_flag) {
+            u(1, vslice_field(slice_sao_luma_flag));
+            if (!vseq->seq_fields.bits.separate_colour_plane_flag &&
+                vseq->seq_fields.bits.chroma_format_idc != 0) {
+                u(1, vslice_field(slice_sao_chroma_flag));
             }
+        }

-            if (vslice->slice_type == P_SLICE || vslice->slice_type == B_SLICE) {
-                u(1, vslice_field(num_ref_idx_active_override_flag));
-                if (vslice->slice_fields.bits.num_ref_idx_active_override_flag) {
-                    ue(vslice_var(num_ref_idx_l0_active_minus1));
-                    if (vslice->slice_type == B_SLICE) {
-                        ue(vslice_var(num_ref_idx_l1_active_minus1));
-                    }
-                }
-
-                if (mseq->lists_modification_present_flag) {
-                    av_assert0(0);
-                    // ref_pic_lists_modification()
-                }
+        if (vslice->slice_type == P_SLICE || vslice->slice_type == B_SLICE) {
+            u(1, vslice_field(num_ref_idx_active_override_flag));
+            if (vslice->slice_fields.bits.num_ref_idx_active_override_flag) {
+                ue(vslice_var(num_ref_idx_l0_active_minus1));
                 if (vslice->slice_type == B_SLICE) {
-                    u(1, vslice_field(mvd_l1_zero_flag));
-                }
-                if (mseq->cabac_init_present_flag) {
-                    u(1, vslice_field(cabac_init_flag));
-                }
-                if (vslice->slice_fields.bits.slice_temporal_mvp_enabled_flag) {
-                    if (vslice->slice_type == B_SLICE)
-                        u(1, vslice_field(collocated_from_l0_flag));
-                    ue(vpic->collocated_ref_pic_index, collocated_ref_idx);
-                }
-                if ((vpic->pic_fields.bits.weighted_pred_flag &&
-                     vslice->slice_type == P_SLICE) ||
-                    (vpic->pic_fields.bits.weighted_bipred_flag &&
-                     vslice->slice_type == B_SLICE)) {
-                    av_assert0(0);
-                    // pred_weight_table()
+                    ue(vslice_var(num_ref_idx_l1_active_minus1));
                 }
-                ue(5 - vslice->max_num_merge_cand, five_minus_max_num_merge_cand);
             }

-            se(vslice_var(slice_qp_delta));
-            if (mseq->pps_slice_chroma_qp_offsets_present_flag) {
-                se(vslice_var(slice_cb_qp_offset));
-                se(vslice_var(slice_cr_qp_offset));
+            if (mseq->lists_modification_present_flag) {
+                av_assert0(0);
+                // ref_pic_lists_modification()
             }
-            if (mseq->pps_slice_chroma_offset_list_enabled_flag) {
-                u(1, 0, cu_chroma_qp_offset_enabled_flag);
+            if (vslice->slice_type == B_SLICE) {
+                u(1, vslice_field(mvd_l1_zero_flag));
             }
-            if (mseq->deblocking_filter_override_enabled_flag) {
-                u(1, mslice_var(deblocking_filter_override_flag));
+            if (mseq->cabac_init_present_flag) {
+                u(1, vslice_field(cabac_init_flag));
             }
-            if (mslice->deblocking_filter_override_flag) {
-                u(1, vslice_field(slice_deblocking_filter_disabled_flag));
-                if (!vslice->slice_fields.bits.slice_deblocking_filter_disabled_flag) {
-                    se(vslice_var(slice_beta_offset_div2));
-                    se(vslice_var(slice_tc_offset_div2));
-                }
+            if (vslice->slice_fields.bits.slice_temporal_mvp_enabled_flag) {
+                if (vslice->slice_type == B_SLICE)
+                    u(1, vslice_field(collocated_from_l0_flag));
+                ue(vpic->collocated_ref_pic_index, collocated_ref_idx);
             }
-            if (vpic->pic_fields.bits.pps_loop_filter_across_slices_enabled_flag &&
-                (vslice->slice_fields.bits.slice_sao_luma_flag ||
-                 vslice->slice_fields.bits.slice_sao_chroma_flag ||
-                 vslice->slice_fields.bits.slice_deblocking_filter_disabled_flag)) {
-                u(1, vslice_field(slice_loop_filter_across_slices_enabled_flag));
+            if ((vpic->pic_fields.bits.weighted_pred_flag &&
+                 vslice->slice_type == P_SLICE) ||
+                (vpic->pic_fields.bits.weighted_bipred_flag &&
+                 vslice->slice_type == B_SLICE)) {
+                av_assert0(0);
+                // pred_weight_table()
+            }
+            ue(5 - vslice->max_num_merge_cand, five_minus_max_num_merge_cand);
+        }
+
+        se(vslice_var(slice_qp_delta));
+        if (mseq->pps_slice_chroma_qp_offsets_present_flag) {
+            se(vslice_var(slice_cb_qp_offset));
+            se(vslice_var(slice_cr_qp_offset));
+        }
+        if (mseq->pps_slice_chroma_offset_list_enabled_flag) {
+            u(1, 0, cu_chroma_qp_offset_enabled_flag);
+        }
+        if (mseq->deblocking_filter_override_enabled_flag) {
+            u(1, mslice_var(deblocking_filter_override_flag));
+        }
+        if (mslice->deblocking_filter_override_flag) {
+            u(1, vslice_field(slice_deblocking_filter_disabled_flag));
+            if (!vslice->slice_fields.bits.slice_deblocking_filter_disabled_flag) {
+                se(vslice_var(slice_beta_offset_div2));
+                se(vslice_var(slice_tc_offset_div2));
             }
         }
+        if (vpic->pic_fields.bits.pps_loop_filter_across_slices_enabled_flag &&
+            (vslice->slice_fields.bits.slice_sao_luma_flag ||
+             vslice->slice_fields.bits.slice_sao_chroma_flag ||
+             vslice->slice_fields.bits.slice_deblocking_filter_disabled_flag)) {
+            u(1, vslice_field(slice_loop_filter_across_slices_enabled_flag));
+        }

         if (vpic->pic_fields.bits.tiles_enabled_flag ||
             vpic->pic_fields.bits.entropy_coding_sync_enabled_flag) {