Bug 1147744 - Part 2: Properly calculate cropping value. r=k17e, a=sledru
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 08 Apr 2015 14:26:38 +1000
changeset 258433 c4a01c159cb6
parent 258432 8f8ebd186863
child 258434 ab0337907115
push id4668
push userryanvm@gmail.com
push date2015-04-13 16:23 +0000
treeherdermozilla-beta@002faed66e96 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersk17e, sledru
bugs1147744
milestone38.0
Bug 1147744 - Part 2: Properly calculate cropping value. r=k17e, a=sledru If chroma_format_idc isn't defined, chroma_format_idc shall be inferred to be equal to 1 (4:2:0 chroma format). Also, ignore nonsensical cropping values as per spec.
media/libstagefright/binding/H264.cpp
media/libstagefright/binding/include/mp4_demuxer/H264.h
--- a/media/libstagefright/binding/H264.cpp
+++ b/media/libstagefright/binding/H264.cpp
@@ -4,16 +4,17 @@
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/PodOperations.h"
 #include "mp4_demuxer/AnnexB.h"
 #include "mp4_demuxer/ByteReader.h"
 #include "mp4_demuxer/ByteWriter.h"
 #include "mp4_demuxer/H264.h"
 #include <media/stagefright/foundation/ABitReader.h>
+#include <limits>
 
 using namespace mozilla;
 
 namespace mp4_demuxer
 {
 
 class BitReader
 {
@@ -69,16 +70,17 @@ public:
 private:
   stagefright::ABitReader mBitReader;
 };
 
 SPSData::SPSData()
 {
   PodZero(this);
   // Default values when they aren't defined as per ITU-T H.264 (2014/02).
+  chroma_format_idc = 1;
   video_format = 5;
   colour_primaries = 2;
   transfer_characteristics = 2;
   sample_ratio = 1.0;
 }
 
 /* static */ already_AddRefed<ByteBuffer>
 H264::DecodeNALUnit(const ByteBuffer* aNAL)
@@ -177,16 +179,21 @@ H264::DecodeSPS(const ByteBuffer* aSPS, 
               deltaScale = br.ReadSE();
               nextScale = (lastScale + deltaScale + 256) % 256;
             }
             lastScale = (nextScale == 0) ? lastScale : nextScale;
           }
         }
       }
     }
+  } else if (aDest.profile_idc == 183) {
+      aDest.chroma_format_idc = 0;
+  } else {
+    // default value if chroma_format_idc isn't set.
+    aDest.chroma_format_idc = 1;
   }
   aDest.log2_max_frame_num = br.ReadUE() + 4;
   aDest.pic_order_cnt_type = br.ReadUE();
   if (aDest.pic_order_cnt_type == 0) {
     aDest.log2_max_pic_order_cnt_lsb = br.ReadUE() + 4;
   } else if (aDest.pic_order_cnt_type == 1) {
     aDest.delta_pic_order_always_zero_flag = br.ReadBit();
     aDest.offset_for_non_ref_pic = br.ReadSE();
@@ -217,36 +224,51 @@ H264::DecodeSPS(const ByteBuffer* aSPS, 
   aDest.sample_ratio = 1.0f;
   aDest.vui_parameters_present_flag = br.ReadBit();
   if (aDest.vui_parameters_present_flag) {
     vui_parameters(br, aDest);
   }
 
   // Calculate common values.
 
-  // FFmpeg and VLC ignore the left and top cropping. Do the same here.
-
   uint8_t ChromaArrayType =
     aDest.separate_colour_plane_flag ? 0 : aDest.chroma_format_idc;
   // Calculate width.
   uint32_t CropUnitX = 1;
   uint32_t SubWidthC = aDest.chroma_format_idc == 3 ? 1 : 2;
   if (ChromaArrayType != 0) {
     CropUnitX = SubWidthC;
   }
-  uint32_t cropX = CropUnitX * aDest.frame_crop_right_offset;
-  aDest.pic_width = aDest.pic_width_in_mbs * 16 - cropX;
 
   // Calculate Height
   uint32_t CropUnitY = 2 - aDest.frame_mbs_only_flag;
   uint32_t SubHeightC = aDest.chroma_format_idc <= 1 ? 2 : 1;
-  if (ChromaArrayType != 0)
+  if (ChromaArrayType != 0) {
     CropUnitY *= SubHeightC;
-  uint32_t cropY = CropUnitY * aDest.frame_crop_bottom_offset;
-  aDest.pic_height = aDest.pic_height_in_map_units * 16 - cropY;
+  }
+
+  uint32_t width = aDest.pic_width_in_mbs * 16;
+  uint32_t height = aDest.pic_height_in_map_units * 16;
+  if (aDest.frame_crop_left_offset <= std::numeric_limits<int32_t>::max() / 4 / CropUnitX &&
+      aDest.frame_crop_right_offset <= std::numeric_limits<int32_t>::max() / 4 / CropUnitX &&
+      aDest.frame_crop_top_offset <= std::numeric_limits<int32_t>::max() / 4 / CropUnitY &&
+      aDest.frame_crop_bottom_offset <= std::numeric_limits<int32_t>::max() / 4 / CropUnitY &&
+      (aDest.frame_crop_left_offset + aDest.frame_crop_right_offset) * CropUnitX < width &&
+      (aDest.frame_crop_top_offset + aDest.frame_crop_bottom_offset) * CropUnitY < height) {
+    aDest.crop_left = aDest.frame_crop_left_offset * CropUnitX;
+    aDest.crop_right = aDest.frame_crop_right_offset * CropUnitX;
+    aDest.crop_top = aDest.frame_crop_top_offset * CropUnitY;
+    aDest.crop_bottom = aDest.frame_crop_bottom_offset * CropUnitY;
+  } else {
+    // Nonsensical value, ignore them.
+    aDest.crop_left = aDest.crop_right = aDest.crop_top = aDest.crop_bottom = 0;
+  }
+
+  aDest.pic_width = width - aDest.crop_left - aDest.crop_right;
+  aDest.pic_height = height - aDest.crop_top - aDest.crop_bottom;
 
   aDest.interlaced = !aDest.frame_mbs_only_flag;
 
   // Determine display size.
   if (aDest.sample_ratio > 1.0) {
     // Increase the intrinsic width
     aDest.display_width =
       ConditionDimension(aDest.pic_width * aDest.sample_ratio);
--- a/media/libstagefright/binding/include/mp4_demuxer/H264.h
+++ b/media/libstagefright/binding/include/mp4_demuxer/H264.h
@@ -35,16 +35,21 @@ struct SPSData
    display_width and display_height are adjusted according to the display
    sample aspect ratio.
    */
   uint32_t display_width;
   uint32_t display_height;
 
   float sample_ratio;
 
+  uint32_t crop_left;
+  uint32_t crop_right;
+  uint32_t crop_top;
+  uint32_t crop_bottom;
+
   /*
     H264 decoding parameters according to ITU-T H.264 (T-REC-H.264-201402-I/en)
    http://www.itu.int/rec/T-REC-H.264-201402-I/en
    */
 
   bool constraint_set0_flag;
   bool constraint_set1_flag;
   bool constraint_set2_flag;
@@ -63,28 +68,35 @@ struct SPSData
   /*
     seq_parameter_set_id identifies the sequence parameter set that is referred
     to by the picture parameter set. The value of seq_parameter_set_id shall be
     in the range of 0 to 31, inclusive.
    */
   uint8_t seq_parameter_set_id;
 
   /*
-    When the value of chroma_format_idc is equal to 1, the nominal vertical
-    and horizontal relative locations of luma and chroma samples in frames are
-    shown in Figure 6-1. Alternative chroma sample relative locations may be
-    indicated in video usability information (see Annex E).
+    chroma_format_idc specifies the chroma sampling relative to the luma
+    sampling as specified in clause 6.2. The value of chroma_format_idc shall be
+    in the range of 0 to 3, inclusive. When chroma_format_idc is not present,
+    it shall be inferred to be equal to 1 (4:2:0 chroma format).
+    When profile_idc is equal to 183, chroma_format_idc shall be equal to 0
+    (4:0:0 chroma format).
    */
   uint8_t chroma_format_idc;
 
   /*
-    If separate_colour_plane_flag is equal to 0, each of the two chroma arrays
-    has the same height and width as the luma array. Otherwise
-    (separate_colour_plane_flag is equal to 1), the three colour planes are
-    separately processed as monochrome sampled pictures.
+    separate_colour_plane_flag equal to 1 specifies that the three colour
+    components of the 4:4:4 chroma format are coded separately.
+    separate_colour_plane_flag equal to 0 specifies that the colour components
+    are not coded separately. When separate_colour_plane_flag is not present,
+    it shall be inferred to be equal to 0. When separate_colour_plane_flag is
+    equal to 1, the primary coded picture consists of three separate components,
+    each of which consists of coded samples of one colour plane (Y, Cb or Cr)
+    that each use the monochrome coding syntax. In this case, each colour plane
+    is associated with a specific colour_plane_id value.
    */
   bool separate_colour_plane_flag;
 
   /*
     log2_max_frame_num_minus4 specifies the value of the variable
     MaxFrameNum that is used in frame_num related derivations as
     follows: