Commit bdf97d44 authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

Log bitstream conversion failures

Currently, when a H264 chunk cannot be converted to a DecodeBuffer, we
return a nullptr. This means that conversion errors are reported as
"null or empty" decoder buffer errors.

This CL fixes the issue by surfacing a media::Status along with the
nullptr.

Bug: 1139089
Change-Id: I018715b56f65b96e843e6af19c9b8300b73b1f9e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523750
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826021}
parent f715b16c
...@@ -119,8 +119,8 @@ CodecConfigEval AudioDecoder::MakeMediaConfig(const ConfigType& config, ...@@ -119,8 +119,8 @@ CodecConfigEval AudioDecoder::MakeMediaConfig(const ConfigType& config,
return CodecConfigEval::kSupported; return CodecConfigEval::kSupported;
} }
scoped_refptr<media::DecoderBuffer> AudioDecoder::MakeDecoderBuffer( media::StatusOr<scoped_refptr<media::DecoderBuffer>>
const InputType& chunk) { AudioDecoder::MakeDecoderBuffer(const InputType& chunk) {
auto decoder_buffer = media::DecoderBuffer::CopyFrom( auto decoder_buffer = media::DecoderBuffer::CopyFrom(
static_cast<uint8_t*>(chunk.data()->Data()), static_cast<uint8_t*>(chunk.data()->Data()),
chunk.data()->ByteLengthAsSizeT()); chunk.data()->ByteLengthAsSizeT());
......
...@@ -77,7 +77,7 @@ class MODULES_EXPORT AudioDecoder : public DecoderTemplate<AudioDecoderTraits> { ...@@ -77,7 +77,7 @@ class MODULES_EXPORT AudioDecoder : public DecoderTemplate<AudioDecoderTraits> {
CodecConfigEval MakeMediaConfig(const ConfigType& config, CodecConfigEval MakeMediaConfig(const ConfigType& config,
MediaConfigType* out_media_config, MediaConfigType* out_media_config,
String* out_console_message) override; String* out_console_message) override;
scoped_refptr<media::DecoderBuffer> MakeDecoderBuffer( media::StatusOr<scoped_refptr<media::DecoderBuffer>> MakeDecoderBuffer(
const InputType& chunk) override; const InputType& chunk) override;
}; };
......
...@@ -132,7 +132,14 @@ void DecoderTemplate<Traits>::decode(const InputType* chunk, ...@@ -132,7 +132,14 @@ void DecoderTemplate<Traits>::decode(const InputType* chunk,
Request* request = MakeGarbageCollected<Request>(); Request* request = MakeGarbageCollected<Request>();
request->type = Request::Type::kDecode; request->type = Request::Type::kDecode;
request->decoder_buffer = MakeDecoderBuffer(*chunk); auto status_or_buffer = MakeDecoderBuffer(*chunk);
if (status_or_buffer.has_value()) {
request->decoder_buffer = std::move(status_or_buffer.value());
} else {
request->status = std::move(status_or_buffer.error());
}
requests_.push_back(request); requests_.push_back(request);
++requested_decodes_; ++requested_decodes_;
ProcessRequests(); ProcessRequests();
...@@ -291,9 +298,13 @@ bool DecoderTemplate<Traits>::ProcessDecodeRequest(Request* request) { ...@@ -291,9 +298,13 @@ bool DecoderTemplate<Traits>::ProcessDecodeRequest(Request* request) {
// The request may be invalid, if so report that now. // The request may be invalid, if so report that now.
if (!request->decoder_buffer || request->decoder_buffer->data_size() == 0) { if (!request->decoder_buffer || request->decoder_buffer->data_size() == 0) {
HandleError("Decoding error", media::Status error =
media::Status(media::StatusCode::kDecoderFailedDecode, !request->status.is_ok()
"Null or empty decoder buffer.")); ? request->status
: media::Status(media::StatusCode::kDecoderFailedDecode,
"Null or empty decoder buffer.");
HandleError("Decoding error", error);
return false; return false;
} }
......
...@@ -68,9 +68,11 @@ class MODULES_EXPORT DecoderTemplate ...@@ -68,9 +68,11 @@ class MODULES_EXPORT DecoderTemplate
// Convert a chunk to a DecoderBuffer. You can assume that the last // Convert a chunk to a DecoderBuffer. You can assume that the last
// configuration sent to MakeMediaConfig() is the active configuration for // configuration sent to MakeMediaConfig() is the active configuration for
// |chunk|. // |chunk|. If there is an error in the conversion process, the resulting
virtual scoped_refptr<media::DecoderBuffer> MakeDecoderBuffer( // DecoderBuffer will be null, and |out_status| will contain a description of
const InputType& chunk) = 0; // the error.
virtual media::StatusOr<scoped_refptr<media::DecoderBuffer>>
MakeDecoderBuffer(const InputType& chunk) = 0;
private: private:
struct Request final : public GarbageCollected<Request> { struct Request final : public GarbageCollected<Request> {
...@@ -93,6 +95,9 @@ class MODULES_EXPORT DecoderTemplate ...@@ -93,6 +95,9 @@ class MODULES_EXPORT DecoderTemplate
// For kFlush Requests. // For kFlush Requests.
Member<ScriptPromiseResolver> resolver; Member<ScriptPromiseResolver> resolver;
// For reporting an error at the time when a request is processed.
media::Status status;
}; };
void ProcessRequests(); void ProcessRequests();
......
...@@ -168,8 +168,8 @@ CodecConfigEval VideoDecoder::MakeMediaConfig(const ConfigType& config, ...@@ -168,8 +168,8 @@ CodecConfigEval VideoDecoder::MakeMediaConfig(const ConfigType& config,
return CodecConfigEval::kSupported; return CodecConfigEval::kSupported;
} }
scoped_refptr<media::DecoderBuffer> VideoDecoder::MakeDecoderBuffer( media::StatusOr<scoped_refptr<media::DecoderBuffer>>
const InputType& chunk) { VideoDecoder::MakeDecoderBuffer(const InputType& chunk) {
uint8_t* src = static_cast<uint8_t*>(chunk.data()->Data()); uint8_t* src = static_cast<uint8_t*>(chunk.data()->Data());
size_t src_size = chunk.data()->ByteLengthAsSizeT(); size_t src_size = chunk.data()->ByteLengthAsSizeT();
...@@ -180,16 +180,16 @@ scoped_refptr<media::DecoderBuffer> VideoDecoder::MakeDecoderBuffer( ...@@ -180,16 +180,16 @@ scoped_refptr<media::DecoderBuffer> VideoDecoder::MakeDecoderBuffer(
uint32_t output_size = h264_converter_->CalculateNeededOutputBufferSize( uint32_t output_size = h264_converter_->CalculateNeededOutputBufferSize(
src, static_cast<uint32_t>(src_size), h264_avcc_.get()); src, static_cast<uint32_t>(src_size), h264_avcc_.get());
if (!output_size) { if (!output_size) {
// TODO(sandersd): Provide an error message. return media::Status(media::StatusCode::kH264ParsingError,
return nullptr; "Unable to determine size of bitstream buffer.");
} }
std::vector<uint8_t> buf(output_size); std::vector<uint8_t> buf(output_size);
if (!h264_converter_->ConvertNalUnitStreamToByteStream( if (!h264_converter_->ConvertNalUnitStreamToByteStream(
src, static_cast<uint32_t>(src_size), h264_avcc_.get(), buf.data(), src, static_cast<uint32_t>(src_size), h264_avcc_.get(), buf.data(),
&output_size)) { &output_size)) {
// TODO(sandersd): Provide an error message. return media::Status(media::StatusCode::kH264ParsingError,
return nullptr; "Unable to convert NALU to byte stream.");
} }
decoder_buffer = media::DecoderBuffer::CopyFrom(buf.data(), output_size); decoder_buffer = media::DecoderBuffer::CopyFrom(buf.data(), output_size);
......
...@@ -86,7 +86,7 @@ class MODULES_EXPORT VideoDecoder : public DecoderTemplate<VideoDecoderTraits> { ...@@ -86,7 +86,7 @@ class MODULES_EXPORT VideoDecoder : public DecoderTemplate<VideoDecoderTraits> {
CodecConfigEval MakeMediaConfig(const ConfigType& config, CodecConfigEval MakeMediaConfig(const ConfigType& config,
MediaConfigType* out_media_config, MediaConfigType* out_media_config,
String* out_console_message) override; String* out_console_message) override;
scoped_refptr<media::DecoderBuffer> MakeDecoderBuffer( media::StatusOr<scoped_refptr<media::DecoderBuffer>> MakeDecoderBuffer(
const InputType& input) override; const InputType& input) override;
#if BUILDFLAG(USE_PROPRIETARY_CODECS) #if BUILDFLAG(USE_PROPRIETARY_CODECS)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment