Bug 1147744: Part2. Properly calculate cropping value. r=k17e
authorJean-Yves Avenard <jyavenard@mozilla.com>
Wed, 08 Apr 2015 14:26:38 +1000
changeset 238094 801b4019fb00e17d660949300caff87fa8da753a
parent 238093 d540c29d5c2f574d68ba813774cf0e3f38141c40
child 238095 86b60ef17965059637a5e78fd3eeb332ccfff372
push id28554
push userryanvm@gmail.com
push dateWed, 08 Apr 2015 16:15:39 +0000
treeherdermozilla-central@256b307c35ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersk17e
bugs1147744
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1147744: Part2. Properly calculate cropping value. r=k17e 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: