Add JPEG parser for JPEG decode acceleration

content/common/gpu/media/vaapi_jpeg_parser.h

+// Copyright 2014 The Chromium Authors. All rights reserved.

[Pawel Osciak, 2014/12/26 04:47:40]

This class could be in media/filters.

[kcwu, 2014/12/27 13:27:35]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+#ifndef CONTENT_COMMON_GPU_MEDIA_VAAPI_JPEG_PARSER_H_
+#define CONTENT_COMMON_GPU_MEDIA_VAAPI_JPEG_PARSER_H_
+
+#include "base/big_endian.h"
+#include "base/macros.h"
+#include "content/common/content_export.h"
+
+namespace content {
+
+static const int kJpegHuffmanTableNum_baseline = 2;

[Pawel Osciak, 2014/12/26 04:47:40]

all size_t please.

[kcwu, 2014/12/27 13:27:35]

Done.

+static const int kJpegComponentNum = 4;
+static const int kJpegQuantizationTableNum = 4;
+
+enum JpegMarker {

[Owen Lin, 2014/12/26 07:13:39]

Do we need to export this ? Or it should be hide in the c++ file.

[kcwu, 2014/12/27 13:27:35]

Done.

+  SOF0 = 0xC0,     // start of frame (baseline)
+  DHT = 0xC4,      // define huffman table
+  SOI = 0xD8,      // start of image
+  SOS = 0xDA,      // start of scan
+  DQT = 0xDB,      // define quantization table
+  DRI = 0xDD,      // define restart internal
+  MARKER1 = 0xFF,  // jpeg marker prefix
+};
+
+// Parsing result of JPEG DHT marker

[Pawel Osciak, 2014/12/26 04:47:40]

Dots at end of sentences everywhere please.

[kcwu, 2014/12/27 13:27:35]

Done.

+struct JpegHuffmanTable {
+  bool valid;
+  uint8_t code_length[16];
+  uint8_t code_value[256];
+};
+
+// Parsing result of JPEG DQT marker
+struct JpegQuantizationTable {
+  bool valid;
+  uint8_t value[64];  // only support 8 bits quantization table

[Pawel Osciak, 2014/12/26 04:47:40]

Is this a limitation of this parser, or is this according to spec?

[kcwu, 2014/12/27 13:27:35]

Done.

+};
+
+// Parsing result of a JPEG component
+struct JpegComponent {
+  uint8_t id;
+  uint8_t horizontal_sampling_factor;
+  uint8_t vertical_sampling_factor;
+  uint8_t quantization_table_selector;
+};
+
+// Parsing result of JPEG SOS marker
+struct JpegScan {
+  uint8_t num_component;
+  struct Component {
+    uint8_t component_selector;
+    uint8_t dc_selector;
+    uint8_t ac_selector;
+  } components[kJpegComponentNum];
+  const char* data;

[Pawel Osciak, 2014/12/26 04:47:40]

const uint8_t*

[kcwu, 2014/12/27 13:27:35]

Done.

[kcwu, 2015/01/07 11:07:31]

I decided go back to const char*. Otherwise reinterpret_cast from "const char*"
to "const uint8_t*" and later reinterpret_cast+const_cast to "char*" for use, is
ugly.

+  size_t data_size;
+};
+
+struct JpegParseResult {
+  uint16_t visible_width;

[Pawel Osciak, 2014/12/26 04:47:40]

gfx::Size ?

[kcwu, 2014/12/27 13:27:35]

Done.
The drawback is needing ctor since it is no longer plain data.

+  uint16_t visible_height;
+  uint8_t num_component;

[Pawel Osciak, 2014/12/26 04:47:40]

num_components. Please describe how this differs from kJpegComponentNum. I'm
guessing you probably want to s/kJpegComponentNum/kJpegMaxComponents/. Not sure
about other constants?

[kcwu, 2014/12/27 13:27:35]

Done.

+  JpegComponent components[kJpegComponentNum];
+  JpegHuffmanTable dc_table[kJpegHuffmanTableNum_baseline];
+  JpegHuffmanTable ac_table[kJpegHuffmanTableNum_baseline];
+  JpegQuantizationTable q_table[kJpegQuantizationTableNum];
+  uint16_t restart_interval;
+  JpegScan scan;
+};
+
+// VaapiJpegParser parses JPEG header. It's not a full feature JPEG parser

[Owen Lin, 2014/12/26 07:13:39]

full featured ?

[kcwu, 2014/12/27 13:27:35]

Done.

+// implememtation. It only parses subset of JPEG baseline mode needed by
+// VAAPI JPEG decoder. For explanations of each struct and its members,
+// see JPEG specification at http://www.w3.org/Graphics/JPEG/itu-t81.pdf.
+class CONTENT_EXPORT VaapiJpegParser {
+ public:
+  // JPEG buffer and its size. The parser doesn't own the memory.

[Pawel Osciak, 2014/12/26 04:47:40]

|buffer| |sizes|
What does the buffer have to contain, a JPEG picture? Can it take any JPEG
picture? What happens if the picture is not supported?

[kcwu, 2014/12/27 13:27:35]

Acknowledged.

+  VaapiJpegParser(const uint8_t* buffer, size_t length);

[Pawel Osciak, 2014/12/26 04:47:40]

Feels to me that we could easily make this not Vaapi-specific.

[kcwu, 2014/12/27 13:27:35]

Done.

+  ~VaapiJpegParser();
+
+  // Parse JPEG header. Returns the parsed result iff header is valid and
+  // supported by VAAPI. Otherwise returns NULL. The parser still owns
+  // the returned result memory.

[Owen Lin, 2014/12/26 07:13:39]

It is not convincing that the life time of the parsed result should be same as
the parser. How about passing the result as an output argument to this function.
Return true iff the result is successfully parsed.

bool Parse(JpegParseResult* result);

or make this class can be reused:

bool Parse(const uint8_t* buffer, size_t length, JpegParseResult *result)

[kcwu, 2014/12/27 13:27:35]

Done.

+  const JpegParseResult* Parse();
+
+ private:
+  bool ParseSOF(base::BigEndianReader reader);

[Owen Lin, 2014/12/26 07:13:39]

Why |reader| is not base::BigEndianReader* or const base::BigEndianReader& ?

Or just ParseSOF(const uint8_t* data, size_t size)
and you can create the BigEndianReaders inside those functions.

[kcwu, 2014/12/27 13:27:35]

Done.

+  bool ParseDQT(base::BigEndianReader reader);
+  bool ParseDHT(base::BigEndianReader reader);
+  bool ParseDRI(base::BigEndianReader reader);
+  bool ParseSOS(base::BigEndianReader reader);
+  bool ParseSOI(base::BigEndianReader reader);
+
+  // JPEG buffer. The parser doesn't own the memory.
+  const uint8_t* buffer_;
+  // Length of data in buffer
+  size_t length_;
+
+  // Parsed result
+  JpegParseResult result_;
+
+  DISALLOW_COPY_AND_ASSIGN(VaapiJpegParser);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_COMMON_GPU_MEDIA_VAAPI_JPEG_PARSER_H_