Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: Support building without FFmpeg #3

Open
kkofler opened this issue Aug 7, 2015 · 9 comments
Open

RFE: Support building without FFmpeg #3

kkofler opened this issue Aug 7, 2015 · 9 comments

Comments

@kkofler
Copy link

kkofler commented Aug 7, 2015

It should be possible to compile Chromium entirely without the FFmpeg backend (only with GStreamer), not only either with both (runtime-selectable) or only with FFmpeg. This is especially important for us in Fedora because FFmpeg is not allowed in Fedora.

@aobzhirov
Copy link

Hi, good point. FFmpeg backend is integral part of Chromium and it will be challenging to disable it compile time, but we should definitely consider it. May be it could be done as a part of the refactoring once we introduce media process in upstream Chromium.

@kkofler
Copy link
Author

kkofler commented Aug 7, 2015

I suppose the first step would be to set media_use_ffmpeg=0 and comment out the runtime backend selection, and see what breaks, right?

@kkofler
Copy link
Author

kkofler commented Aug 7, 2015

(But that would still try to build a lot of code that isn't actually going to be used, wouldn't it? As far as I can see, your backend plugs in at a much higher level than where the MEDIA_DISABLE_FFMPEG defines are.)

@aobzhirov
Copy link

Yes, you are right. If you use MEDIA_DISABLE_FFMPEG it will disable ffmpeg decoder (FFmpegVideoDecoder) for the renderer and it shouldn't compile FFmpeg license code. But it can still use default media player (WebMediaPlayerImpl) with other supported decoders like gpu accelerated (GpuVideoDecoder). And as you said we plug in few levels up and provide our own implementation of blink::WebMediaPlayer that why I was talking about possible future refactoring which would split WebMediaPlayerImpl code into render and media process parts similar to ours. But it looks like you can already disable ffmpeg and use our backend.

aobzhirov pushed a commit that referenced this issue Aug 7, 2015
This CL parses the report-uri attribute on HPKP headers and stores them
in TransportSecurityPersister.

This is CL #1.
CL #2: crrev.com/1212973002 (add net::CertificateReportSender)
CL #3: crrev.com/1212613004 (build and send HPKP reports)

BUG=445793

Committed: https://crrev.com/1320e36d908427d615357df1630348bfb38cb5c4
Cr-Commit-Position: refs/heads/master@{#339667}

Review URL: https://codereview.chromium.org/1211363005

Cr-Commit-Position: refs/heads/master@{#340490}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
Summary of changes available at:
https://chromium.googlesource.com/skia/+log/9a5d1ab..a3a9ebc

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=reed@google.com

Commits in this roll:
a3a9ebc halcanary@google.com documentation: more API details and examples.
6e9aed9 junov@chromium.org Fixing src rect constraint support for drawImage with SkPicture
bca1400 reed@google.com remove pixel assert from ctable validator
e9d6095 joshualitt@chromium.org Cleanup Default Geo Proc API
2c32342 mtklein@chromium.org No one calls SkXfermode::GetProc16
56b78a7 mtklein@chromium.org Revert of Lay groundwork for SkOpts. (patchset #3 id:40001 of https://codereview.chromium.org/1255193002/)
513f265 joshualitt@chromium.org fix up GrImmediateDrawTarget.cpp
58fd2c8 mtklein@chromium.org Remove sk_memcpy32
ce2c505 mtklein@chromium.org Lay groundwork for SkOpts.

Review URL: https://codereview.chromium.org/1258073004

Cr-Commit-Position: refs/heads/master@{#340579}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
…40001 of https://codereview.chromium.org/1220223002/)

Reason for revert:
Looks like this doesn't quite work on Windows:

http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/builds/21206/steps/compile%20%28with%20patch%29/logs/stdio

Original issue's description:
> Add output reference to gen written files.
>
> By adding the outputs written at gn gen time to the build.ninja representation the build tool (ninja) can know how to recreate them if and when necessary.
>
> For example, if one or more of the <blah>.tmp files is removed, even as part of an entire hierarchy/subtree currently the build fails because ninja does not know how to re-make it.
>
> BUG=469621
> TEST=Build time only. See https://code.google.com/p/chromium/issues/detail?id=469621#c3
>
> Committed: https://crrev.com/517ac52f86903beb206b19e6ddeedbc709b253a1
> Cr-Commit-Position: refs/heads/master@{#338929}

TBR=brettw@chromium.org,petermayo@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=469621

Review URL: https://codereview.chromium.org/1255113004

Cr-Commit-Position: refs/heads/master@{#340596}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
net::CertificateReportSender contains code factored out of
CertificateErrorReporter in //chrome. net::CertificateReportSender will
be used for both HPKP violation reports and the Safe Browsing Extended
Reporting cert reports that CertificateErrorReporter handles.

CL #1: crrev.com/1211363005 (parse report-uri)
This is CL #2.
CL #3: crrev.com/1212613004 (build and send HPKP reports)

BUG=445793

Review URL: https://codereview.chromium.org/1212973002

Cr-Commit-Position: refs/heads/master@{#340641}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
This CL adds code to TransportSecurityState to build HPKP reports, and
sends them with a CertificateReportSender constructed by
ProfileIOData. Calls to CheckPublicKeyPins() indicate whether a report
should be sent and pass necessary reporting information as arguments.

CL #1: crrev.com/1211363005 (parse report-uri)
CL #2: crrev.com/1212973002 (add net::CertificateReportSender)
This is CL #3.

BUG=445793

Review URL: https://codereview.chromium.org/1212613004

Cr-Commit-Position: refs/heads/master@{#340687}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
…d:60001 of https://codereview.chromium.org/1239363002/)

Reason for revert:
Breaks various fast/events and fast/scrolling layout tests on Mac >= 10.8.

https://code.google.com/p/chromium/issues/detail?id=514704

Original issue's description:
> Pass NSUserDefaults over IPC to the renderer
>
> Blink can now accept the NSUserDefaults values via IPC. This change causes the main process to pass the default values over IPC using the new API.
>
> BUG=392141
>
> Committed: https://crrev.com/54113566ce626a9fcfe5fd1dd46f0362d01ed8ed
> Cr-Commit-Position: refs/heads/master@{#340695}

TBR=palmer@chromium.org,avi@chromium.org,rsesek@chromium.org,kerrnel@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=392141

Review URL: https://codereview.chromium.org/1261933003

Cr-Commit-Position: refs/heads/master@{#340714}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
  In order to roll GN ec34450ee9..2f763b5fef (r338735:r340609) and pick up
  the following changes:

  61fb682 Revert of Add output reference to gen written files. (patchset #3 id:40001 of https://codereview.chromium.org/1220223002/)
  177c1e2 Add GN isolate support for a bunch of unittests.
  295e072 Count data deps even if it was already a regular dep
  45884a2 Make sure GN deps rolls include the optional GN bots.
  97ec9ac Add a GN auto-roller script.
  9550931 Remove legacy StartsWithASCII function.
  925b216 Fix broken link and add cross-compiling to quick start
  a7ff1b2 Remove some legacy versions of StartsWith and EndsWith.
  ddbde8f Revert of Remove some legacy versions of StartsWith and EndsWith. (patchset #6 id:100001 of https://codereview.chromium.org/1239493005/)
  edce9a3 Remove some legacy versions of StartsWith and EndsWith.
  517ac52 Add output reference to gen written files.
  d673f55 Add missing source to gn bootstrap.

TBR=brettw@chromium.org
CQ_EXTRA_TRYBOTS=tryserver.chromium.mac:mac_chromium_gn_rel,mac_chromium_gn_dbg;tryserver.chromium.win:win8_chromium_gn_dbg,win_chromium_gn_x64_rel

Review URL: https://codereview.chromium.org/1260153002

Cr-Commit-Position: refs/heads/master@{#340781}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
…s (patchset #1 id:1 of https://codereview.chromium.org/1244243003/)

Reason for revert:
It turns out this CL isn't the culprit of crashes. Let's reland.

Original issue's description:
> Revert of Call EnsureWebKitInitialized() before registering extensions (patchset #3 id:40001 of https://codereview.chromium.org/1182083006/)
>
> Reason for revert:
> This seems the cause of many crashes
>
> Original issue's description:
> > Call EnsureWebKitInitialized() before registering extensions
> >
> > Blink needs to be initialized before registering an extension because:
> > - WebScriptController::registerExtension() allocates an WTF::Vector on the first
> >   call.
> > - WTF::Vector uses PartitionAlloc.
> > - PartitionAlloc's partitions needs to be initialized before allocating memory.
> >   Blink initialization does the job.
> >
> > Before this CL, partitions are initialized lazily but lazy initialization
> > doesn't provide proper histogram function, which might be the cause of missing
> > report for PartitionAlloc.CommittedSize UMA.
> >
> > BUG=501171
> >
> > Committed: https://crrev.com/0d244fa5e106b3d146655a2c623539a3e1900831
> > Cr-Commit-Position: refs/heads/master@{#339200}
>
> TBR=jochen@chromium.org,haraken@chromium.org,kalman@chromium.org,kinuko@chromium.org,paulmeyer@chromium.org,sievers@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=501171
>
> Committed: https://crrev.com/2b94b05fc252c9a006ae924d0fe75ed3abecbeb2
> Cr-Commit-Position: refs/heads/master@{#339777}

TBR=jochen@chromium.org,haraken@chromium.org,kalman@chromium.org,kinuko@chromium.org,paulmeyer@chromium.org,sievers@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=501171

Review URL: https://codereview.chromium.org/1258103005

Cr-Commit-Position: refs/heads/master@{#340793}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
It's currently possible to "lose" a tab-modal dialog (e.g. print
preview) when dragging multiple tabs off a browser.

Mac's ConstrainedWindowManager doesn't support reparenting tab-modal
dialogs, so dragging of tabs with modal dialogs must be prevented.

BUG=514937 TEST=Have 3 tabs, press Cmd+P in #3 get a print preview,
Shift+Click tab #2 (release mouse button), now drag tab #2 off the
window. The whole window should drag (tabs can not be detached with
dialogs). Before this change, the tabs would come off and the dialog
would be lost.

Review URL: https://codereview.chromium.org/1261513005

Cr-Commit-Position: refs/heads/master@{#341239}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
…es. (patchset #3 id:40001 of https://codereview.chromium.org/1254423006/)

Reason for revert:
Speculative revert.
This seems to have broken
BrandColorTest.testBrandColorInterstitial
BrandColorTest.testNavigatingToBrandColorAndBack
BrandColorTest.testNoBrandColor
on Lollipop in Document Mde.

Original issue's description:
> Add getThemeColor to Tab and add plumbing for ChromeActivites.
>
> Moves all theme color computing logic to tab. Adds the necessary
> update signals to ChromeActivity and adds an onUpdate call to be
> overridden by extending classes.
>
> The main outcome of all this is, all ChromeActivities become color change aware. Doesn't add any visual changes. Yet.
>
> BUG=507340
>
> Committed: https://crrev.com/25d23c547cd7661fe2e679d74e4272917dae2985
> Cr-Commit-Position: refs/heads/master@{#341226}

TBR=dtrainor@chromium.org,yusufo@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=507340

Review URL: https://codereview.chromium.org/1260523004

Cr-Commit-Position: refs/heads/master@{#341332}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
Summary of changes available at:
https://chromium.googlesource.com/skia/+log/0aff2fa..8db6fdc

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=reed@google.com

Commits in this roll:
8db6fdc joshualitt@chromium.org bump the size of the atlas id to 64 bits
1421aee bsalomon@google.com Make SkIsPow2 templated
bdc91a7 joshualitt@google.com Revert of Modifying TextBlobCacheTest to use SkRandomScalerContext (patchset #3 id:40001 of https://codereview.chromium.org/1266003002/)
adcdca8 joshualitt@chromium.org Modifying TextBlobCacheTest to use SkRandomScalerContext

Review URL: https://codereview.chromium.org/1256703005

Cr-Commit-Position: refs/heads/master@{#341356}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
Summary of changes available at:
https://chromium.googlesource.com/skia/+log/8db6fdc..f96bee3

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=reed@google.com

Commits in this roll:
f96bee3 mtklein@chromium.org disable SkOpts on x86 iOS (simulator)
853336c joshualitt@chromium.org adding gm to use random scaler context
aa80e42 scroggo@chromium.org Fix straggling SK_VIRTUAL_CONSTRAINT_TYPEs
da9ccf1 joshualitt@google.com Revert of Move strike to subrun in GrAtlasTextContext (patchset #3 id:40001 of https://codereview.chromium.org/1257253005/)
bdb34d0 mtklein@chromium.org Move SkOpts.h back to src/core.
eae6200 bsalomon@google.com Some cleanup in GrTextureProvider and GrResourceProvider.
77d89f7 joshualitt@chromium.org Move strike to subrun in GrAtlasTextContext
7568d0b halcanary@google.com C API: add sk_xfermode.h, impl, test
85cd78d bsalomon@google.com Speculative fix for http://crbug.com/515966
ec994b6 halcanary@google.com Documentation spelling error
82314e9 halcanary@google.com Documentation: SkXfermode::Mode
d1ebe06 joshualitt@chromium.org Another small fix to GrFontScaler
383ff10 bsalomon@google.com Remove unnecessary virtual destructor on SkTArray
490b615 mtklein@chromium.org Port SkXfermode opts to SkOpts.h
685f277 reed@google.com use SkNextID::ImageID for android's stable id
65e96b4 joshualitt@chromium.org Modifying TextBlobCacheTest to use SkRandomScalerContext
88c7b98 bsalomon@google.com Make ANGLE perf decisions be runtime rather than compile time
7eb0945 mtklein@chromium.org Port SkUtils opts to SkOpts.
5119ac0 djsollen@google.com Update Android Testing apps to support release mode
b411b3b bsalomon@google.com Restore read pixels perf on ANGLE
96a52c8 reed@google.com remove drawimagerect flags, as skia no longer respects them

Review URL: https://codereview.chromium.org/1260793009

Cr-Commit-Position: refs/heads/master@{#341448}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
Summary of changes available at:
https://chromium.googlesource.com/skia/+log/e003680..1d183b4

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
TBR=mtklein@google.com

Commits in this roll:
1d183b4 mtklein@google.com Revert of SkCanvas::onDrawPicture() quick-reject (patchset #3 id:40001 of https://codereview.chromium.org/1264133003/ )
48ed62b fmalita@chromium.org SkCanvas::onDrawPicture() quick-reject

Review URL: https://codereview.chromium.org/1268983004

Cr-Commit-Position: refs/heads/master@{#341682}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
…app is already installed. (patchset #3 id:40001 of https://codereview.chromium.org/1259543002/ )

Reason for revert:
breaks browser_tests on Win7: https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20(dbg)(1)/builds/40480/steps/browser_tests/logs/AppBannerDataFetcherBrowserTest.WebAppBannerCreatedDirect

Original issue's description:
> Make desktop banner prompts respect bypass flag when a web app is already installed.
>
> Desktop app banners are currently not shown if the web app has already
> been added to the shelf. This should be overruled by
> --bypass-app-banner-engagement-checks
>
> BUG=513582
> R=benwells@chromium.org
>
> Committed: https://crrev.com/98d5901a6cd71bfdca36e92db2f51015df000ba8
> Cr-Commit-Position: refs/heads/master@{#341671}

TBR=benwells@chromium.org,dominickn@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=513582

Review URL: https://codereview.chromium.org/1265233002

Cr-Commit-Position: refs/heads/master@{#341693}
aobzhirov pushed a commit that referenced this issue Aug 7, 2015
…(patchset #3 id:40001 of https://codereview.chromium.org/1267243003/ )

Reason for revert:
Test flaking on dowsntream ToT due to deferred startup not finishing.

Original issue's description:
> Add custom tabs tests using intents with non-null sessions
>
> All current tests either test the service side session creation without
> launching intents or the UI customization with no sessions. This change
> add a trivial test that uses a valid session to launch an intent and also
> another more complex test that reloads a new URL in the same tab when the
> same session is used inside the intent.
>
> Note that the service side is interfaced directly through CustomTabConnection
> and this doesn't exactly replicate the actual scenario since all calls come
> from the same process.
>
> Committed: https://crrev.com/a69c3f7ba69a30aef2143945b8b2bbf8e6421080
> Cr-Commit-Position: refs/heads/master@{#342131}

TBR=newt@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1276993002

Cr-Commit-Position: refs/heads/master@{#342169}
@CapOM
Copy link

CapOM commented Sep 22, 2015

Simply add: media_use_ffmpeg=0 media_use_libvpx=0 media_use_libwebm=0 to the GYP_DEFINES when executing the gyp configuration command. (I need to mention it in our README.md)

Btw why FFmpeg is not allowed on Fedora ?

@mariospr
Copy link

Btw why FFmpeg is not allowed on Fedora ?

FFmpeg contains certain algorithms (decoders, encoders) that can be patent encumbered which can't be packaged in Fedora either due to the licensing guidelines: https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Patented_Software (yet you can get them installed via other repositories, such as RPMFusion)

Additionally, another good reason to push for being able to build chromium without the internal FFmpeg it bundles is that Fedora has a strict policy when it comes to bundled libraries which makes it really hard to package Chromium, and not just because of FFmpeg, but also for the rest of libraries it bundles: https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

@kkofler
Copy link
Author

kkofler commented Sep 22, 2015

Btw why FFmpeg is not allowed on Fedora ?

Because it is a patent minefield. In principle, the patent-encumbered stuff can be ripped out, but in practice, shipping that would be a disservice to our users because its monolithic structure does not allow plugging in additional codecs from third-party repositories, you have to recompile the whole library. Sadly, neither FFmpeg upstream nor Libav upstream are in any way interested in making codecs pluggable. There were even proof of concept patches for codec plugins that got submitted, they all got rejected because they don't like the idea as a principle.

There are Chromium packages in Copr repositories (our version of PPAs) for Fedora, but those ship a bundled FFmpeg with almost all codecs ripped out. Bundled libraries are also not allowed in Fedora in most cases, which is why those packages are only in Copr repositories, and they also suffer from the "can't add more codecs" problem I described above.

@CapOM
Copy link

CapOM commented Sep 22, 2015

Thx for the detailed explanations. Make perfectly sense. I also like the argument about "Bundled libraries are also not allowed in Fedora in most cases"

Then for chromium it is not only ffmpeg you should disable but at least also libvpx and libwebm.

Theoretically it should be enough to add "media_use_ffmpeg=0 media_use_libvpx=0 media_use_libwebm=0" to the GYP_DEFINES build command.

I tried but I got a few build failures which I fixed: 94e7010
It is even a candidate patch for upstream chromium.

So now you should be able to completely disable ffmpeg.

(A side note about HW decoders in chromium, they are not available on desktop linux, only on some Embedded configurations, iOS, Android and ChromeOS. With our on-going zero-copy work our GStreamer backend will be able to use HW decoders that come from gstreamer plugins packages, like gstvaapidecode, gstomxvideodecoder and gstv4l2videodecoder, and having zero-copy working)

@JulienIsorce
Copy link
Contributor

Worth to mention that the patch mentioned in previous comment in under review here: https://codereview.chromium.org/1415793003

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants