New issue
Advanced search Search tips

Issue 749178 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Consider expanding verification of FFmpegDemuxer MediaLog using StrictMock<MockMediaLog>

Project Member Reported by wolenetz@chromium.org, Jul 26 2017

Issue description

Many of our other unit tests (especially MSE parsing and demuxing) already strictly verify MediaLog expectations. FFmpegDemuxerTests don't yet.

This bug tracks potentially fixing that.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3b52cb6a2e29e9b1b9eb14d86535893f71edb43c

commit 3b52cb6a2e29e9b1b9eb14d86535893f71edb43c
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Thu Jul 27 01:01:38 2017

Clarify FFmpegDemuxer: failed creating ... stream log

Otherwise valid audio or video resources may contain streams that Chrome
does not support. When they are reported by FFmpeg as either an audio
for video stream, we try to create a valid decoder configuration for
them as part of constructing FFmpegDemuxerStreams for them.

For some types of such streams (e.g., poster image in a flac or mp3 file
is reported by FFmpeg as video), we previously logged an error message
into chrome://media-internals, followed by an INFO message describing
that we skipped the "invalid or unsupported {audio,video} track" (due to
the failure creating that decoder config/demuxerstream). This could be
confusing, since it's not necessarily an error.

This change clarifies the error log messages for audio/video, and
changes them to be DEBUG type. This also has the effect that
MediaError.message, which might occur if *all* streams are skipped due
to failures to create their FFmpegDemuxerStreams, won't be populated
with the "FFmpegDemuxer: failed creating {audio,video} stream" message
(or even the new DEBUG message). It should instead include a more
informative message "FFmpegDemuxer: no supported streams".

This change also adds references to  bug 749178  to expand strict MediaLog
verification in FFmpegDemuxerTests beyond the basics added here.

Example:
old:
info  | FFmpegDemuxer: created audio stream, config codec: mp3 [codec details...]
error | FFmpegDemuxer: failed creating video stream
info  | FFmpegDemuxer: skipping invalid or unsupported video track

new:
info  | FFmpegDemuxer: created audio stream, config codec: mp3 [codec details...]
debug | Warning, FFmpegDemuxer failed to create a valid video decoder configuration from muxed stream
info  | FFmpegDemuxer: skipping invalid or unsupported video track

BUG= 748965 , 749178 
TEST=Updated FFmpegDemuxerTests, and manual

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I64b59512ff73013f4307b5dda639e84cb015d69a
Reviewed-on: https://chromium-review.googlesource.com/587425
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489823}
[modify] https://crrev.com/3b52cb6a2e29e9b1b9eb14d86535893f71edb43c/media/base/test_helpers.h
[modify] https://crrev.com/3b52cb6a2e29e9b1b9eb14d86535893f71edb43c/media/filters/ffmpeg_demuxer.cc
[modify] https://crrev.com/3b52cb6a2e29e9b1b9eb14d86535893f71edb43c/media/filters/ffmpeg_demuxer_unittest.cc

Owner: wolenetz@chromium.org
Status: Fixed (was: Available)

Sign in to add a comment