Add a Vp9Parser implementation

media/filters/vp9_parser.h

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

It would be easier to understand this file, if there was short description of
the targeted use case for the functionality described in this header and example
showing the relationships between Vp9FrameHeader, Vp9ReferenceSlots and
Vp9Parser, it would help potential users to understand the code quicker.

[kcwu, 2015/08/03 10:05:52]

Done.

+
+#ifndef MEDIA_FILTERS_VP9_PARSER_H_
+#define MEDIA_FILTERS_VP9_PARSER_H_
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include "base/macros.h"
+#include "media/base/media_export.h"
+#include "media/filters/vp9_raw_bits_reader.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace media {
+
+const int kVp9MaxProfile = 4;
+const int kVp9NumRefFramesLog2 = 3;
+const int kVp9NumRefFrames = 1 << kVp9NumRefFramesLog2;
+const uint8_t kVp9MaxProb = 255;
+const int kVp9NumRefsPerFrame = 3;
+
+enum class Vp9ColorSpace {
+  UNKNOWN = 0,
+  BT_601 = 1,
+  BT_709 = 2,
+  SMPTE_170 = 3,
+  SMPTE_240 = 4,
+  BT_2020 = 5,
+  RESERVED = 6,
+  SRGB = 7,
+};
+
+enum class Vp9InterpFilter {
+  INTERP_FILTER_SELECT = 0,
+  EIGHTTAP_SMOOTH = 1,
+  EIGHTTAP = 2,
+  EIGHTTAP_SHARP = 3,
+  BILINEAR = 4,
+};
+
+// Member of Vp9FrameHeader and will be 0-initialized in Vp9FrameHeader's ctor.

[Ville-Mikko Rautio, 2015/08/02 23:43:50]

There's no (explicit) Vp9FrameHeader ctor. Maybe you meant:

"... 0-initialized by Vp9Parser::ParseFrame"

If you meant by implicit ctor, I thought c++ does not mandate the
zero-initialization even though most compilers might do it.

[Pawel Osciak, 2015/08/02 23:58:15]

Good catch, I think we were both thinking about the memset, not the constructor.
But we should have a constructor instead of the memset, something like
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/vp8_....

Here and in all the other structs.

[kcwu, 2015/08/03 10:05:52]

I'm not so comfortable to write memset(this). If all these structures memset
themselves, there are redundant work since they are nested. If only
Vp9FrameHeader memset(this) and others don't, then the others still relay on
external memset (and they are inconsistent).
I'd prefer memset by Vp9Parser.
 

[kcwu, 2015/08/03 10:05:53]

Done.

+struct MEDIA_EXPORT Vp9Segmentation {
+  static const int kNumSegments = 8;
+  static const int kNumTreeProbs = kNumSegments - 1;
+  static const int kNumPredictionProbs = 3;
+  static const int kNumFeature = 4;
+
+  bool enabled;
+
+  bool update_map;
+  uint8_t tree_probs[kNumTreeProbs];
+  bool temporal_update;
+  uint8_t pred_probs[kNumPredictionProbs];
+
+  bool update_data;
+  bool abs_delta;
+  bool feature_enabled[kNumSegments][kNumFeature];
+  int8_t feature_data[kNumSegments][kNumFeature];
+};
+
+// Member of Vp9FrameHeader and will be 0-initialized in Vp9FrameHeader's ctor.

[Pawel Osciak, 2015/08/01 15:22:48]

s/Member/Members/
s/and //

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

ditto

[kcwu, 2015/08/03 10:05:52]

Done.

[kcwu, 2015/08/03 10:05:52]

Done.

+struct MEDIA_EXPORT Vp9LoopFilter {
+  static const int kNumRefDeltas = 4;
+  static const int kNumModeDeltas = 2;
+
+  uint8_t filter_level;
+  uint8_t sharpness_level;
+
+  bool mode_ref_delta_enabled;
+  bool mode_ref_delta_update;
+  bool update_ref_deltas[kNumRefDeltas];
+  int8_t ref_deltas[kNumRefDeltas];
+  bool update_mode_deltas[kNumModeDeltas];
+  int8_t mode_deltas[kNumModeDeltas];
+};
+
+// Member of Vp9FrameHeader and will be 0-initialized in Vp9FrameHeader's ctor.

[Pawel Osciak, 2015/08/01 15:22:49]

Ditto.

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

ditto

[kcwu, 2015/08/03 10:05:52]

Done.

[kcwu, 2015/08/03 10:05:53]

Done.

+struct MEDIA_EXPORT Vp9QuantizationParams {
+  bool IsLossless() const {
+    return base_qindex == 0 && y_dc_delta == 0 && uv_dc_delta == 0 &&
+           uv_ac_delta == 0;

[Pawel Osciak, 2015/08/01 15:22:48]

Is this a correct indent (not 4 spaces?). What does git cl format say?

[kcwu, 2015/08/03 10:05:52]

This is "git cl format" did.

+  }
+
+  uint8_t base_qindex;
+  int8_t y_dc_delta;
+  int8_t uv_dc_delta;
+  int8_t uv_ac_delta;
+};
+
+// VP9 frame header.
+struct MEDIA_EXPORT Vp9FrameHeader {
+  enum FrameType {
+    KEYFRAME = 0,
+    INTERFRAME = 1,
+  };
+
+  bool IsKeyframe() const { return frame_type == KEYFRAME; }
+
+  uint8_t profile;
+
+  bool show_existing_frame;
+  uint8_t frame_to_show;
+
+  FrameType frame_type;
+
+  bool show_frame;
+  bool error_resilient_mode;
+
+  uint8_t bit_depth;
+  Vp9ColorSpace color_space;
+  bool yuv_range;
+  uint8_t subsampling_x;
+  uint8_t subsampling_y;
+
+  // The range of width and height is 1..2^16.
+  uint32_t width;
+  uint32_t height;
+  uint32_t display_width;
+  uint32_t display_height;
+
+  bool intra_only;
+  uint8_t reset_context;
+  bool refresh_flag[kVp9NumRefFrames];
+  uint8_t frame_refs[kVp9NumRefsPerFrame];
+  bool ref_sign_biases[kVp9NumRefsPerFrame];
+  bool allow_high_precision_mv;
+  Vp9InterpFilter interp_filter;
+
+  bool refresh_frame_context;
+  bool frame_parallel_decoding_mode;
+  uint8_t frame_context_idx;
+
+  Vp9LoopFilter loop_filter;
+  Vp9QuantizationParams quant_params;
+  Vp9Segmentation segment;
+
+  uint8_t log2_tile_cols;
+  uint8_t log2_tile_rows;
+
+  // Size of compressed header in bytes.
+  size_t first_partition_size;
+
+  // Size of uncompressed header in bytes.
+  size_t uncompressed_header_size;
+};
+
+// The parsing context for Vp9Parser to keep track references.

[Pawel Osciak, 2015/08/01 15:22:48]

s/track/track of/

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

What is the designed purpose for having the list of frame sizes maintained?  How
is this going to be used?

[kcwu, 2015/08/03 10:05:52]

Done.

[kcwu, 2015/08/03 10:05:52]

The sizes are needed to parse next frames as ReadFrameSizeFromRefs. Isn't it?
I revised to make it internal state of the parser.

+struct MEDIA_EXPORT Vp9ReferenceSlots {

[Pawel Osciak, 2015/08/01 15:22:48]

Do we need this 1-member structure? Perhaps move the slots array into Vp9Parser
as ref_slots[] ?

[kcwu, 2015/08/03 10:05:52]

I was under impression that it will be used by vp9 decoder, so I packed it into
single object to make it "friendly".
I will move it back to Vp9Parser since the parser will maintain it.

+  // Updates the reference slots according to |fhdr|.
+  void Update(const Vp9FrameHeader& fhdr);

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

You're not using Vp9ReferenceSlots::Update currently. This gets filled up by the
Vp9Parser directly. Is there planned use for the function or is it just
leftover?

[kcwu, 2015/08/03 10:05:52]

I was misunderstood it will be used by vp9 decoder. Revised.

+
+  gfx::Size slot[kVp9NumRefFrames];
+};
+
+// A parser for VP9 bitstream.
+class MEDIA_EXPORT Vp9Parser {
+ public:
+  Vp9Parser();
+
+  // Parses one frame and fill parsing result to |fhdr|.

[Pawel Osciak, 2015/08/01 15:22:49]

s/fill/fills/
Returns true on success, false otherwise.

[kcwu, 2015/08/03 10:05:52]

Done.

+  // |stream| is the address of VP9 bitstream with |size|.
+  // |ref_slots| is the VP9 reference slots maintained by the caller.

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

Explain also the return value.

[kcwu, 2015/08/03 10:05:52]

Done.

+  bool ParseFrame(const uint8_t* stream,
+                  size_t size,
+                  const Vp9ReferenceSlots& ref_slots,
+                  Vp9FrameHeader* fhdr);
+
+ private:
+  uint8_t ReadProfile();

[Ville-Mikko Rautio, 2015/08/02 23:43:49]

As far as I can tell all the private members are just implementation details for
the user of the class and their understanding are not essential for the use of
the class. Maybe hide them in a forward declared private class defined in the
implementation file (PIMPL)?

[kcwu, 2015/08/03 10:05:52]

Indeed they are implementation detail. However I'm not sure it worth to
implement PIMPL pattern here. (which makes it not straightforward)

+  bool VerifySyncCode();
+  bool ReadBitDepthColorSpaceSampling(Vp9FrameHeader* fhdr);
+  void ReadFrameSize(Vp9FrameHeader* fhdr);
+  bool ReadFrameSizeFromRefs(const Vp9ReferenceSlots& ref_slots,
+                             Vp9FrameHeader* fhdr);
+  void ReadDisplayFrameSize(Vp9FrameHeader* fhdr);
+  Vp9InterpFilter ReadInterpFilter();
+  void ReadLoopFilter(Vp9LoopFilter* loop_filter);
+  void ReadQuantization(Vp9QuantizationParams* quants);
+  void ReadSegmentationMap(Vp9Segmentation* segment);
+  void ReadSegmentationData(Vp9Segmentation* segment);
+  void ReadSegmentation(Vp9Segmentation* segment);
+  void ReadTiles(Vp9FrameHeader* fhdr);
+  bool ParseUncompressedHeader(const Vp9ReferenceSlots& ref_slots,
+                               Vp9FrameHeader* fhdr);
+
+  // Start address of VP9 bitstream buffer.
+  const uint8_t* stream_;
+
+  // Size of |stream_| in bytes.
+  size_t size_;
+
+  // Raw bits decoder for uncompressed frame header.
+  Vp9RawBitsReader reader_;
+
+  DISALLOW_COPY_AND_ASSIGN(Vp9Parser);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_FILTERS_VP9_PARSER_H_