New issue
Advanced search Search tips

Issue 778369 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

IvfParser is compiled for all builds but used only for (unit)tests

Project Member Reported by mcasas@chromium.org, Oct 25 2017

Issue description

media/filters/ivfparser.{cc,h} is compiled for all builds:
https://cs.chromium.org/chromium/src/media/filters/BUILD.gn?type=cs&q=ivf_parser.h&sq=package:chromium&l=45

but is only used for unittests/tests
https://cs.chromium.org/search/?q=IvfParser&sq=package:chromium&type=cs


$ grep -rn IvfParser media/
media/filters/ivf_parser.cc:28:IvfParser::IvfParser() : ptr_(nullptr), end_(nullptr) {}
media/filters/ivf_parser.cc:30:bool IvfParser::Initialize(const uint8_t* stream,
media/filters/ivf_parser.cc:64:bool IvfParser::ParseNextFrame(IvfFrameHeader* frame_header,
media/filters/vp9_parser_unittest.cc:87:  IvfParser ivf_parser_;
media/filters/vp8_parser_fuzzertest.cc:15:  media::IvfParser ivf_parser;
media/filters/vp8_parser_unittest.cc:23:  IvfParser ivf_parser;
media/filters/vp9_parser_fuzzertest.cc:28:  media::IvfParser ivf_parser;
media/filters/ivf_parser.h:56:class MEDIA_EXPORT IvfParser {
media/filters/ivf_parser.h:58:  IvfParser();
media/filters/ivf_parser.h:81:  DISALLOW_COPY_AND_ASSIGN(IvfParser);
media/filters/ivf_parser_unittest.cc:15:TEST(IvfParserTest, StreamFileParsing) {
media/filters/ivf_parser_unittest.cc:22:  IvfParser parser;


Consider compiling only for unittests/fuzzertests etc.
 
Should we just delete this then? Why does it even exist?
Owner: dalecur...@chromium.org
Status: Started (was: Untriaged)
https://chromium-review.googlesource.com/c/chromium/src/+/738638

Comment 3 by mcasas@chromium.org, Oct 25 2017

#1: it's used in the parsers to read the input .ivf file(s).
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 2 2017

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

commit 08268de146718a0d96c0eaf79adf0dd3999ecf28
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Thu Nov 02 03:18:18 2017

IVFParser is only used by unit tests.

So don't build it for all platforms and non-test targets.

BUG= 778369 
TEST=none

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: I106f2a909b27028fbc88a2fdf9dd850f97d5eb73
Reviewed-on: https://chromium-review.googlesource.com/738638
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513391}
[modify] https://crrev.com/08268de146718a0d96c0eaf79adf0dd3999ecf28/media/BUILD.gn
[modify] https://crrev.com/08268de146718a0d96c0eaf79adf0dd3999ecf28/media/base/BUILD.gn
[modify] https://crrev.com/08268de146718a0d96c0eaf79adf0dd3999ecf28/media/filters/BUILD.gn
[modify] https://crrev.com/08268de146718a0d96c0eaf79adf0dd3999ecf28/media/filters/ivf_parser.h

Status: Fixed (was: Started)

Sign in to add a comment