ffmpeg API compatibility

[Paweł Hajdan, Jr.]

This may be a controversial topic. My goal is not to stir controversy, and I'm going to listen to technical feedback posted here, just as I had for similar efforts before. Please keep that in mind before replying. :)

ffmpeg API changes quite quickly before releases, and for Chrome we seem to be using recent code from git. When using system ffmpeg for Linux distros, there are challenges with making that actually work.

I was thinking about creating some sort of compatibility layer that would allow the codebase to build against API-incompatible versions of ffmpeg. That may include extracting some ffmpeg-specific code, and putting it behind an interface specifying more what it should do, rather than how to do that (i.e. what ffmpeg functions should be called, headers included etc). I can have some better examples after actual experiments, but before doing the experiments I wanted to check here for any requirements I may be not aware of. Please keep in mind I want to keep it leightweight and simple.

What do you think?

Paweł

[Alexei Svitkine]

What's wrong with requiring a specific minimum version of ffmpeg? If the distro doesn't have a system-level one of that version, then  don't use system ffmpeg and build our own as we normally do via DEPS.

Paweł

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Chris Palmer]

Is it ever the case that the underlying system has a newer ffmpeg than
what we ship with?

From both the reliability/stability and security standpoints, I'd want
to always run the most recent ffmpeg. I suspect that will be Chrome's;
Linux distros seem to be slow to upgrade packages ("conservative", if
you prefer).

In general, Chrome moves faster than almost everyone else. For Chrome
to isolate itself from underlying platform dependencies is a feature,
not a bug. I don't understand the "use system libs" fixation.

[Matt Giuca]

On 15 January 2013 10:07, Chris Palmer <pal...@google.com> wrote:

Is it ever the case that the underlying system has a newer ffmpeg than
what we ship with?

Certainly if there is a security fix for ffmpeg (or anything), the Linux distros will coordinate and push out a fix within 24 hours of the patch becoming public. I don't know a lot about the Chrome release cycle -- if a security vulnerability is fixed in Chrome, does an update get pushed immediately, or does it usually wait until the next release? Does this also extend to patches in third-party software?

Linux distros seem to be slow to upgrade packages ("conservative", if
you prefer).

That's true for general updates. However, for security fixes, they do special cherry-picks and push them out immediately.

Even if it Chrome integrates security fixes from upstream and pushes them out quickly, I would expect Linux distro package managers (whose primary job is to push out security fixes in an immediate and coordinated fashion) would push them out faster than Chrome can integrate the patch, and release a new version. But as I said, I don't know how Chrome deals with these situations.

[Dale Curtis]

tl;dr: I don't think this is worth worrying and may even be problematic from a security perspective. 

1. Chrome uses a small subset of the ffmpeg API, small enough to be whitelisted:

http://git.chromium.org/gitweb/?p=chromium/third_party/ffmpeg.git;a=blob;f=chromium/ffmpegsumo.sigs

2. FFmpeg security issues are found and fixed on a regular basis in just the codecs we use. I'm doubtful that the distro patch cycle is even close to Chrome's turn around time (~days). I certainly do not see my local ffmpeg package updated as frequently as I merge security fixes from upstream (in fact I've never seen it updated...). The usage and security model for the ffmpeg or libav package in a distro is very different than Chrome's. Compare the cost of drive-by attacks in a browser with those of playing a video locally.

To put some numbers to this question: We sync to ToT FFmpeg every milestone (~6 weeks), giving a distro based on stable channel ~12 weeks to update. I think that's a pretty big window for security fixes and even major version updates.

- dale

--

[Paweł Hajdan, Jr.]

Sorry, the issue of how patching works on distros and who is faster and how that affects what we do wrt system libraries has been discussed to death. We still provide an option to use system libraries for Linux distros. We continue to use bundled libraries for Google Chrome, even on Linux. If you want to know more, feel free to check out http://goto.google.com/xpovo (sorry, Googlers-only).

Now why don't we just state a minimum version of ffmpeg we require to work? Chrome moves fast and seems to use unreleased versions of ffmpeg (code just pulled from git). Even without that, all the ffmpeg-dependent apps do not always update as fast as we can.

Please try to avoid discussing whether or not this effort is worth it. For me it is. :) The question is: provided I can make it lightweight and simple enough (which should be made easier because we only use a small subset of ffmpeg API), would you consider having such a compatibility layer? Alternatively, what would your requirements for such a layer be? Note that I'd do any necessary work there, and also design it in a way to make it easier to use more ffmpeg functions in the future.

Paweł

On Mon, Jan 14, 2013 at 3:33 PM, Matt Giuca <mgi...@chromium.org> wrote:

[John Abd-El-Malek]

On Mon, Jan 14, 2013 at 3:49 PM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

Sorry, the issue of how patching works on distros and who is faster and how that affects what we do wrt system libraries has been discussed to death. We still provide an option to use system libraries for Linux distros. We continue to use bundled libraries for Google Chrome, even on Linux. If you want to know more, feel free to check out http://goto.google.com/xpovo (sorry, Googlers-only).

Now why don't we just state a minimum version of ffmpeg we require to work? Chrome moves fast and seems to use unreleased versions of ffmpeg (code just pulled from git). Even without that, all the ffmpeg-dependent apps do not always update as fast as we can.

Please try to avoid discussing whether or not this effort is worth it. For me it is. :) The question is: provided I can make it lightweight and simple enough (which should be made easier because we only use a small subset of ffmpeg API), would you consider having such a compatibility layer? Alternatively, what would your requirements for such a layer be? Note that I'd do any necessary work there, and also design it in a way to make it easier to use more ffmpeg functions in the future.

My understanding is that the consensus on the team has been that we don't want to spend effort to support system libraries. This seems to go against that. The cost will be paid by not just you when adding this, but for everyone else who needs to use a new method or change how it's used, since they have to update this extra layer (and in doing so, will break the system-library build anyways).

So, definitely I think we should be discussing whether this effort is worth it or not, since it's not a one-time thing.

Paweł

On Mon, Jan 14, 2013 at 3:33 PM, Matt Giuca <mgi...@chromium.org> wrote:

On 15 January 2013 10:07, Chris Palmer <pal...@google.com> wrote:

Is it ever the case that the underlying system has a newer ffmpeg than
what we ship with?

Certainly if there is a security fix for ffmpeg (or anything), the Linux distros will coordinate and push out a fix within 24 hours of the patch becoming public. I don't know a lot about the Chrome release cycle -- if a security vulnerability is fixed in Chrome, does an update get pushed immediately, or does it usually wait until the next release? Does this also extend to patches in third-party software?

Linux distros seem to be slow to upgrade packages ("conservative", if
you prefer).

That's true for general updates. However, for security fixes, they do special cherry-picks and push them out immediately.

Even if it Chrome integrates security fixes from upstream and pushes them out quickly, I would expect Linux distro package managers (whose primary job is to push out security fixes in an immediate and coordinated fashion) would push them out faster than Chrome can integrate the patch, and release a new version. But as I said, I don't know how Chrome deals with these situations.

[Chris Palmer]

On Mon, Jan 14, 2013 at 3:44 PM, Dale Curtis <dalec...@chromium.org> wrote:

> 2. FFmpeg security issues are found and fixed on a regular basis in just the
> codecs we use. I'm doubtful that the distro patch cycle is even close to
> Chrome's turn around time (~days). I certainly do not see my local ffmpeg
> package updated as frequently as I merge security fixes from upstream (in
> fact I've never seen it updated...).

Exactly.

On top of that, the job of back-porting fixes is (a) not fun; (b) not
always possible; and (c) a lot of work for an open source distro that
gets a lot of its engineering resources from volunteers. And security
fixes are not always clearly labeled as such — how often do distro
patch back-porters ignore patches whose commit log message is "fixed
crash"? Sometimes the commit message isn't even that suggestive.

> The usage and security model for the
> ffmpeg or libav package in a distro is very different than Chrome's. Compare
> the cost of drive-by attacks in a browser with those of playing a video
> locally.

Not *that* different... after all, where did those local video files
come from? The internet, I bet. :) I.e. although VLC is not as
vulnerable to rapid-fire attacks (as from a crazy web page), distro
maintainers should be more worried than they apparently are about the
up-to-datedness of their complex data parsers/interpreters.

> To put some numbers to this question: We sync to ToT FFmpeg every milestone
> (~6 weeks), giving a distro based on stable channel ~12 weeks to update. I
> think that's a pretty big window for security fixes and even major version
> updates.

And Debian, for example, wants to back-port modern patches to code
that is up to 18 months old. (At least, that's what they tell me.)
Even if it were possible, it would not be wise.

The One True Version of any software package is the latest stable
release — especially for resource-constrained projects like many/most
open source distributions.

[Paweł Hajdan, Jr.]

I talked to John and Chris about issues they raised and I think the conclusion is (feel free to correct me):

1. This is not going to increase ongoing maintenance cost for Chrome codebase, and any one-time work will be done by me.

2. Distros shipping outdated ffmpeg are a security concern (same with other raised concerns), but from a distro perspective hunting down each package shipping ffmpeg and making sure it is patched is much worse than making sure the one true (system) copy of ffmpeg is secure. The latter is faster and more reliable, so it is also more secure. Even if some distros today don't do a good job, providing working options to use system ffmpeg can enable them to do a better job. For a distro shipping old ffmpeg it's recommended not to use the option.

Now the compatibility patch (not to take as-is for Chromium codebase) is http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/www-client/chromium/files/chromium-system-ffmpeg-r1.patch?revision=1.1&view=markup .

While thinking about ways to improve things Chrome-side, the switch statements to convert e.g. between CODEC_ID_OPUS and kCodecOpus etc. come to mind: how does it sound to you to have a macro like say:

#define CHROMIUM_TO_FFMPEG_CODEC(chromium_id, ffmpeg_id) \

  #ifdef ffmpeg_id \
  case chromium_id: \

    return ffmpeg_id; \

  #endif

WDYT? It turned out that the resulting patch is not too big/complicated anyway so this is optional even for me, but maybe if you think it's fine it could simplify things even more.

Paweł

[Dale Curtis]

Have you seen media/ffmpeg/ffmpeg_common.{cc,h} ? It does a lot of this stuff already. We've also got some glue in media/filters/ffmpeg_glue.cc.

- dale

[Paweł Hajdan, Jr.]

On Thu, Jan 17, 2013 at 3:48 PM, Dale Curtis <dalec...@chromium.org> wrote:

Have you seen media/ffmpeg/ffmpeg_common.{cc,h} ? It does a lot of this stuff already.

Have you seen the patch I linked to (http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/www-client/chromium/files/chromium-system-ffmpeg-r1.patch?revision=1.1&view=markup)? It modifies ffmpeg_common.cc, and I was wondering whether we can sort of get these #ifdefs by default behind a "nice" macro. Still, it's not strictly necessary but IMHO still possibly worth it.

[John Abd-El-Malek]

On Thu, Jan 17, 2013 at 3:24 PM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:

I talked to John and Chris about issues they raised and I think the conclusion is (feel free to correct me):

1. This is not going to increase ongoing maintenance cost for Chrome codebase, and any one-time work will be done by me.

To be clear, what I'm opposed to is committing to ongoing work because of the creation of an extra layer. I'm not sure how you will add a compatibility layer that doesn't require ongoing maintenance (i.e. as we use more of ffmpeg)?

[Chris Palmer]

On Fri, Jan 18, 2013 at 9:45 AM, John Abd-El-Malek <j...@chromium.org> wrote:

> To be clear, what I'm opposed to is committing to ongoing work because of
> the creation of an extra layer. I'm not sure how you will add a
> compatibility layer that doesn't require ongoing maintenance (i.e. as we use
> more of ffmpeg)?

That is my primary concern, too.

[Ami Fischman]

#define CHROMIUM_TO_FFMPEG_CODEC(chromium_id, ffmpeg_id) \

  #ifdef ffmpeg_id \
  case chromium_id: \

    return ffmpeg_id; \

  #endif

WDYT?

CODEC_ID_<FOO> are enum values, not preprocessor macros, so this doesn't work.

Also, re: the gentoo patch you pointed at, it conditionally elides functionality based on the ffmpeg version constants.  I'd be loathe to have such conditional sections in chromium code because it implies a backwards-compatibility promise that nobody is prepared to back up.

Cheers,

-a

[Paweł Hajdan, Jr.]

On Fri, Jan 18, 2013 at 9:59 AM, Ami Fischman <fisc...@chromium.org> wrote:

#define CHROMIUM_TO_FFMPEG_CODEC(chromium_id, ffmpeg_id) \

  #ifdef ffmpeg_id \
  case chromium_id: \

    return ffmpeg_id; \

  #endif

WDYT?

CODEC_ID_<FOO> are enum values, not preprocessor macros, so this doesn't work.

Oops indeed. Thank you for catching this. How can I detect the availability of CODEC_ID_OPUS at compile time then, other than checking version numbers explicitly? Is there an easy way?

Would you accept a script that would detect that by calling the compiler (only executed for -Duse_system_ffmpeg=1 case, so would not affect Chrome nor most developers)?

 

Also, re: the gentoo patch you pointed at, it conditionally elides functionality based on the ffmpeg version constants.  I'd be loathe to have such conditional sections in chromium code because it implies a backwards-compatibility promise that nobody is prepared to back up.

I know. That's one of the reasons I've not submitted it for review. :) I consider this something to be done at the distro-level. 

[Ami Fischman]

Oops indeed. Thank you for catching this. How can I detect the availability of CODEC_ID_OPUS at compile time then, other than checking version numbers explicitly?

Why would you want to not use version numbers?  That's what they're for.

 

Would you accept a script that would detect that by calling the compiler (only executed for -Duse_system_ffmpeg=1 case, so would not affect Chrome nor most developers)?

I wouldn't have a problem with the presence of a script; the real question is what the script would do (see below).

  

Also, re: the gentoo patch you pointed at, it conditionally elides functionality based on the ffmpeg version constants.  I'd be loathe to have such conditional sections in chromium code because it implies a backwards-compatibility promise that nobody is prepared to back up.

I know. That's one of the reasons I've not submitted it for review. :) I consider this something to be done at the distro-level. 

If you're prepared to add conditional compilation at the distro level, then what are you proposing to upstream into the chromium codebase?

I think this conversation would be more productive if we had a specific proposal to review (I believe the policy question part of this thread has been answered, but of course feel free to clarify if that's not the case).  

In case it's useful, here's an observation: ffmpeg changes quickly (in terms of API changes), and chromium wants to use ffmpeg at the HEAD that it is rolled to, without being constrained to any other version of ffmpeg that it might be built against.  That means any compatibility layer interposed between chromium and a system ffmpeg will by definition be brittle to chromium/ffmpeg changes.  Since it's brittle, it probably doesn't belong in the non-system-ffmpeg codepath.  Since it will be specific to the version of ffmpeg it's pinned to, you can imagine one of these per distro (in case non-gentoo distros get a phajdan.jr of their own ;)).  All this points pretty strongly towards maintaining this layer in the distro, not in chromium.  

Cheers,

-a

[Paweł Hajdan, Jr.]

On Tue, Jan 22, 2013 at 4:01 PM, Ami Fischman <fisc...@chromium.org> wrote:

Oops indeed. Thank you for catching this. How can I detect the availability of CODEC_ID_OPUS at compile time then, other than checking version numbers explicitly?

Why would you want to not use version numbers?  That's what they're for.

They require more manual lookup (I've mentioned that earlier), and it seems version numbers used by ffmpeg vs. libav are not always in sync.

 

Would you accept a script that would detect that by calling the compiler (only executed for -Duse_system_ffmpeg=1 case, so would not affect Chrome nor most developers)?

I wouldn't have a problem with the presence of a script; the real question is what the script would do (see below).

  

Also, re: the gentoo patch you pointed at, it conditionally elides functionality based on the ffmpeg version constants.  I'd be loathe to have such conditional sections in chromium code because it implies a backwards-compatibility promise that nobody is prepared to back up.

I know. That's one of the reasons I've not submitted it for review. :) I consider this something to be done at the distro-level. 

If you're prepared to add conditional compilation at the distro level, then what are you proposing to upstream into the chromium codebase?

Well, we have an existing example of generate_shim_headers.py that is used only for distro, but lives in chromium repo. Why? Because any non-trivial patching distro-side is not maintainable, especially if it's perpetual by design.

 

I think this conversation would be more productive if we had a specific proposal to review (I believe the policy question part of this thread has been answered, but of course feel free to clarify if that's not the case).

Agreed. I'm also asking for advice though. I think what I would like to do now is to add a script (only used for distro builds, never chrome builds, gated on use_system_ffmpeg=1) that would launch a compiler to see if enum values are defined, similar to what ./configure might do. I'd like to add some #ifdefs / macros as mentioned earlier to ffmpeg_common.cc (under Chrome they would be always enabled).

[Ami Fischman]

They require more manual lookup (I've mentioned that earlier), and it seems version numbers used by ffmpeg vs. libav are not always in sync.

This is true, but OTOH it's unclear to me that there's any combination of other macros that you could test for that would give you a better picture of the presence/absence of features and bugs in a given checkout.

 

I think this conversation would be more productive if we had a specific proposal to review (I believe the policy question part of this thread has been answered, but of course feel free to clarify if that's not the case).

Agreed. I'm also asking for advice though. I think what I would like to do now is to add a script (only used for distro builds, never chrome builds, gated on use_system_ffmpeg=1) that would launch a compiler to see if enum values are defined, similar to what ./configure might do. I'd like to add some #ifdefs / macros as mentioned earlier to ffmpeg_common.cc (under Chrome they would be always enabled).

If the change could be contained to ffmpeg_common.cc and wasn't invasive, I'd say it might be ok.  But it's not clear to me that that's the case, because the ffmpeg API surface that chromium uses is not constrained to ffmpeg_common.h.  This is, for example, why the gentoo patch you pointed at also needs to patch ffmpeg_audio_decoder.cc.  I'm having a hard time envisioning a compatibility shim that wouldn't be invasive.

[nit: I prefer (default-disabled) negative macros to (default-enabled) positive ones; so instead of #ifdef HAS_OPUS_SUPPORT, use #ifdef OMIT_OPUS_SUPPORT in an attempt to imply the default to readers.]

Cheers,

-a