Implemented HEVC video demuxing and parsing

media/formats/mp4/box_definitions.cc

Index: media/formats/mp4/box_definitions.cc
diff --git a/media/formats/mp4/box_definitions.cc b/media/formats/mp4/box_definitions.cc
index 9b4613882081b3e8bcb393e1c1120e442c23a67a..141b7ff13b3ccef5f8860e7113ba8de51d5ec165 100644
--- a/media/formats/mp4/box_definitions.cc
+++ b/media/formats/mp4/box_definitions.cc
@@ -5,9 +5,16 @@
 #include "media/formats/mp4/box_definitions.h"
 
 #include "base/logging.h"
+#include "media/base/video_types.h"
+#include "media/base/video_util.h"
+#include "media/formats/mp4/avc.h"
 #include "media/formats/mp4/es_descriptor.h"
 #include "media/formats/mp4/rcheck.h"
 
+#if defined(USE_PROPRIETARY_CODECS) && defined(ENABLE_HEVC_DEMUXING)

[wolenetz, 2015/09/02 20:43:18]

nit: why sometimes condition on both proprietary+hevc_demuxing, and other times
(like in header for this file), just condition on the latter?

[servolk, 2015/09/03 00:17:51]

Oh, these were remains of the way we had to do things when using global defined
in common.gypi. Now that ENABLE_HEVC_DEMUXING is defined in media.gyp/BUILD.gn
we can just ensure right in the build config files that HEVC is enabled only
when proprietary codecs are supported, so I'll update these #ifdef conditions
throughout the CL now.

[wolenetz, 2015/09/08 19:53:10]

Acknowledged.

+#include "media/formats/mp4/hevc.h"
+#endif
+
 namespace media {
 namespace mp4 {
 
@@ -441,6 +448,110 @@ bool AVCDecoderConfigurationRecord::ParseInternal(
   return true;
 }
 
+#if defined(ENABLE_HEVC_DEMUXING)
+HEVCDecoderConfigurationRecord::HEVCDecoderConfigurationRecord()
+    : configurationVersion(0),
+      general_profile_space(0),
+      general_tier_flag(0),
+      general_profile_idc(0),
+      general_profile_compatibility_flags(0),
+      general_constraint_indicator_flags(0),
+      general_level_idc(0),
+      min_spatial_segmentation_idc(0),
+      parallelismType(0),
+      chromaFormat(0),
+      bitDepthLumaMinus8(0),
+      bitDepthChromaMinus8(0),
+      avgFrameRate(0),
+      constantFrameRate(0),
+      numTemporalLayers(0),
+      temporalIdNested(0),
+      lengthSizeMinusOne(0),
+      numOfArrays(0) {}
+
+HEVCDecoderConfigurationRecord::~HEVCDecoderConfigurationRecord() {}
+FourCC HEVCDecoderConfigurationRecord::BoxType() const { return FOURCC_HVCC; }
+
+bool HEVCDecoderConfigurationRecord::Parse(BoxReader* reader) {
+  return ParseInternal(reader, reader->media_log());
+}
+
+bool HEVCDecoderConfigurationRecord::Parse(const uint8* data, int data_size) {
+  BufferReader reader(data, data_size);
+  return ParseInternal(&reader, new MediaLog());
+}
+
+HEVCDecoderConfigurationRecord::HVCCNALArray::HVCCNALArray()
+  : first_byte(0) {}
+
+HEVCDecoderConfigurationRecord::HVCCNALArray::~HVCCNALArray() {}
+
+bool HEVCDecoderConfigurationRecord::ParseInternal(
+    BufferReader* reader,
+    const scoped_refptr<MediaLog>& media_log) {
+  uint8 profile_indication = 0;
+  uint32 general_constraint_indicator_flags_hi = 0;
+  uint16 general_constraint_indicator_flags_lo = 0;
+  uint8 misc = 0;
+  RCHECK(reader->Read1(&configurationVersion) && configurationVersion == 1 &&

[wolenetz, 2015/09/02 20:43:18]

nit: now would be a great time to MEDIA_LOG why RCHECK might return false for
this new code. Without such detail, developers will be left wondering why decode
error resulted for (in this case, bad content). Please strongly consider adding
"MEDIA_LOG(ERROR, media_log) << ...;" describing the failure conditions for this
method here and in other current RCHECKs in this method.

Likewise, some basic sanity in the unit test, using a StrictMock<MockMediaLog>
for both a successful parse and at least one of the error conditions is a strong
recommendation.

[servolk, 2015/09/03 00:17:51]

On one hand I understand your concerns, but on the other hand I was trying to be
consistent with H.264/AVC implementation here and the code there looks exactly
the same (see lines 408-411 above), only avcC header is smaller and has fewer
parameters. We could try to improve logging here to make diagnosing issues
easier, but note that all we are doing here is reading data from hvcC box (it's
a bunch of read* + one configVersion check), so if something fails right here
there are only two possible causes - either configVersion is unsupported, or the
hvcC header that we are trying to read was too short. If there's any other issue
with the header, then we'll get failure at one of the RCHECKs below and not at
this one. So while there's definitely room for improvement, we can make that
later as a separate CL if it ever becomes a real concern. For now it's probably
fine, I've never seen a failure here, except on some very early patchsets of
this CL.

[wolenetz, 2015/09/08 19:53:10]

That's fine, thank you for providing more detail in the review. In general, if
there's something that's likely to fail parse in a non-trivial portion of real
media, I would like to have the reason why the parse failed more easily
discovered by developers without requiring debug builds, dvlogs, etc on a
potentially-hard-to-repro scenario. In this case, let's land this first and
we'll/I'll continue improving media log coverage overall. ISTM like handling a
false retval from this method (and other box parsers) could use a generic "parse
of xxxx box failed" media log message; such would not be in scope of this CL.

+         reader->Read1(&profile_indication) &&
+         reader->Read4(&general_profile_compatibility_flags) &&
+         reader->Read4(&general_constraint_indicator_flags_hi) &&
+         reader->Read2(&general_constraint_indicator_flags_lo) &&
+         reader->Read1(&general_level_idc) &&
+         reader->Read2(&min_spatial_segmentation_idc) &&
+         reader->Read1(&parallelismType) &&
+         reader->Read1(&chromaFormat) &&
+         reader->Read1(&bitDepthLumaMinus8) &&
+         reader->Read1(&bitDepthChromaMinus8) &&
+         reader->Read2(&avgFrameRate) &&
+         reader->Read1(&misc) &&
+         reader->Read1(&numOfArrays));
+
+  general_profile_space = profile_indication >> 6;
+  general_tier_flag = (profile_indication >> 5) & 1;
+  general_profile_idc = profile_indication & 0x1f;
+
+  general_constraint_indicator_flags = general_constraint_indicator_flags_hi;
+  general_constraint_indicator_flags <<= 16;
+  general_constraint_indicator_flags |= general_constraint_indicator_flags_lo;
+
+  min_spatial_segmentation_idc &= 0xfff;
+  parallelismType &= 3;
+  chromaFormat &= 3;
+  bitDepthLumaMinus8 &= 7;
+  bitDepthChromaMinus8 &= 7;
+
+  constantFrameRate = misc >> 6;
+  numTemporalLayers = (misc >> 3) & 7;
+  temporalIdNested = (misc >> 2) & 1;
+  lengthSizeMinusOne = misc & 3;
+
+  DVLOG(2) << __FUNCTION__ << " numOfArrays=" << (int)numOfArrays;
+  arrays.resize(numOfArrays);
+  for (uint32 j = 0; j < numOfArrays; j++) {
+    RCHECK(reader->Read1(&arrays[j].first_byte));
+    uint16 numNalus = 0;
+    RCHECK(reader->Read2(&numNalus));
+    arrays[j].units.resize(numNalus);
+    for (uint32 i = 0; i < numNalus; ++i) {
+      uint16 naluLength = 0;
+      RCHECK(reader->Read2(&naluLength) &&
+             reader->ReadVec(&arrays[j].units[i], naluLength));
+      DVLOG(4) << __FUNCTION__ << " naluType="
+               << (int)(arrays[j].first_byte & 0x3f)
+               << " size=" << arrays[j].units[i].size();
+    }
+  }
+
+  if (media_log.get()) {
+    MEDIA_LOG(INFO, media_log) << "Video codec: hevc";
+  }
+
+  return true;
+}
+#endif
+
 PixelAspectRatioBox::PixelAspectRatioBox() : h_spacing(1), v_spacing(1) {}
 PixelAspectRatioBox::~PixelAspectRatioBox() {}
 FourCC PixelAspectRatioBox::BoxType() const { return FOURCC_PASP; }
@@ -455,7 +566,9 @@ VideoSampleEntry::VideoSampleEntry()
     : format(FOURCC_NULL),
       data_reference_index(0),
       width(0),
-      height(0) {}
+      height(0),
+      video_codec(kUnknownVideoCodec),
+      video_codec_profile(VIDEO_CODEC_PROFILE_UNKNOWN) {}
 
 VideoSampleEntry::~VideoSampleEntry() {}
 FourCC VideoSampleEntry::BoxType() const {
@@ -464,6 +577,26 @@ FourCC VideoSampleEntry::BoxType() const {
   return FOURCC_NULL;
 }
 
+namespace {
+
+bool IsFormatValidH264(const FourCC& format,
+                       const ProtectionSchemeInfo& sinf) {
+  return format == FOURCC_AVC1 || format == FOURCC_AVC3 ||
+      (format == FOURCC_ENCV && (sinf.format.format == FOURCC_AVC1 ||
+                                 sinf.format.format == FOURCC_AVC3));
+}
+
+#if defined(ENABLE_HEVC_DEMUXING)
+bool IsFormatValidHEVC(const FourCC& format,
+                       const ProtectionSchemeInfo& sinf) {
+  return format == FOURCC_HEV1 || format == FOURCC_HVC1 ||
+      (format == FOURCC_ENCV && (sinf.format.format == FOURCC_HEV1 ||
+                                 sinf.format.format == FOURCC_HVC1));
+}
+#endif
+
+}
+
 bool VideoSampleEntry::Parse(BoxReader* reader) {
   format = reader->type();
   RCHECK(reader->SkipBytes(6) &&
@@ -485,21 +618,44 @@ bool VideoSampleEntry::Parse(BoxReader* reader) {
     }
   }
 
-  if (IsFormatValid()) {
+  if (IsFormatValidH264(format, sinf)) {
+    DVLOG(2) << __FUNCTION__
+             << " reading AVCDecoderConfigurationRecord (avcC)";
     scoped_ptr<AVCDecoderConfigurationRecord> avcConfig(
         new AVCDecoderConfigurationRecord());
     RCHECK(reader->ReadChild(avcConfig.get()));
     frame_bitstream_converter = make_scoped_refptr(
         new AVCBitstreamConverter(avcConfig.Pass()));
+    video_codec = kCodecH264;
+    video_codec_profile = H264PROFILE_MAIN;
+#if defined(ENABLE_HEVC_DEMUXING)
+  } else if (IsFormatValidHEVC(format, sinf)) {
+    DVLOG(2) << __FUNCTION__
+             << " parsing HEVCDecoderConfigurationRecord (hvcC)";
+    scoped_ptr<HEVCDecoderConfigurationRecord> hevcConfig(
+        new HEVCDecoderConfigurationRecord());
+    RCHECK(reader->ReadChild(hevcConfig.get()));
+    frame_bitstream_converter = make_scoped_refptr(
+        new HEVCBitstreamConverter(hevcConfig.Pass()));
+    video_codec = kCodecHEVC;
+#endif
+  } else {
+    // Unknown/unsupported format
+    MEDIA_LOG(ERROR, reader->media_log()) << __FUNCTION__

[wolenetz, 2015/09/02 20:43:18]

nit: We haven't been including __FUNCTION__ in most of our current MEDIA_LOG()
in ToT, rather more user-friendly but still precise text. I don't have a strong
feeling here, but consider this message will likely eventually be plumbed into
devtools console output, which similarly doesn't usually show Chrome code
function names explicitly (IIRC).

+                                          << " unsupported video format "
+                                          << FourCCToString(format);
+    return false;
   }
 
   return true;
 }
 
 bool VideoSampleEntry::IsFormatValid() const {
-  return format == FOURCC_AVC1 || format == FOURCC_AVC3 ||
-      (format == FOURCC_ENCV && (sinf.format.format == FOURCC_AVC1 ||
-                                 sinf.format.format == FOURCC_AVC3));
+#if defined(ENABLE_HEVC_DEMUXING)
+  if (IsFormatValidHEVC(format, sinf))
+    return true;
+#endif
+  return IsFormatValidH264(format, sinf);
 }
 
 ElementaryStreamDescriptor::ElementaryStreamDescriptor()