Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 747337

Summary: media-video/ffmpeg-4.3.1: Unbreak av_malloc_max(0) API/ABI
Product: Gentoo Linux Reporter: Joakim Tjernlund <joakim.tjernlund>
Component: Current packagesAssignee: Gentoo Media-video project <media-video>
Status: RESOLVED WONTFIX    
Severity: normal CC: chewi, jstein, marcel.schilling, paolo.pedroni, sam, sultan, whissi
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Attachments: Unbreak av_malloc_max(0) API/ABI

Description Joakim Tjernlund 2020-10-08 15:09:06 UTC
Created attachment 664390 [details, diff]
Unbreak av_malloc_max(0) API/ABI

From https://bugs.chromium.org/p/chromium/issues/detail?id=1095962
----------------------------
This seems to be caused by the custom handling of "av_max_alloc(0)" in
Chromium's ffmpeg fork to mean unlimited (added in [1]).

Upstream ffmpeg doesn't treat 0 as a special value; versions before 4.3 seemingly worked
because 32 was subtracted from max_alloc_size (set to 0 by Chromium) resulting in an
integer underflow, making the effective limit be SIZE_MAX - 31.

Now that the above underflow doesn't happen, the tab just crashes. The upstream change
for no longer subtracting 32 from max_alloc_size was included in ffmpeg 4.3. [2]

[1] https://chromium-review.googlesource.com/c/chromium/third_party/ffmpeg/+/73563
[2] https://github.com/FFmpeg/FFmpeg/commit/731c77589841
---------------------------

Restore av_malloc_max(0) to MAX_INT fixing MS Teams, Discord older chromium etc.
Comment 1 Alexis Ballier gentoo-dev 2020-10-09 16:45:49 UTC
I'm not fond of that hack and I'd rather see the consumers use INT_MAX instead of 0...

However, if this patch gets accepted upstream, then I'm fine with having it backported.
Comment 2 Joakim Tjernlund 2020-10-10 09:15:25 UTC
(In reply to Alexis Ballier from comment #1)
> I'm not fond of that hack and I'd rather see the consumers use INT_MAX
> instead of 0...

Sure, but that will take time and we need to still be able to use these apps
which are binary only so we cannot fix then ourself.

> 
> However, if this patch gets accepted upstream, then I'm fine with having it
> backported.

If upstream does not take it I think Gentoo should until apps has been fixed
Comment 3 Alexis Ballier gentoo-dev 2020-10-10 13:23:20 UTC
(In reply to Joakim Tjernlund from comment #2)

> > 
> > However, if this patch gets accepted upstream, then I'm fine with having it
> > backported.
> 
> If upstream does not take it I think Gentoo should until apps has been fixed

It is not really our role to define the ABI & API of upstream libraries :/
Plus, if they do not take it, they would have good reasons that I would like to hear.
Comment 4 Joakim Tjernlund 2020-10-11 09:34:03 UTC
Speaking of upstream, I posted the patch to their dev mail list but it it
seems stuck.
Anyone here close to upstream that can bring this patch to their attention?
Comment 5 Alexis Ballier gentoo-dev 2020-10-11 12:36:18 UTC
(In reply to Joakim Tjernlund from comment #4)
> Speaking of upstream, I posted the patch to their dev mail list but it it
> seems stuck.
> Anyone here close to upstream that can bring this patch to their attention?

Do you have a link ?
I can't seem to find it.

Usually it's ok to send a friendly ping if no answer for >1 week.
Comment 6 Joakim Tjernlund 2020-10-11 14:42:13 UTC
(In reply to Alexis Ballier from comment #5)
> (In reply to Joakim Tjernlund from comment #4)
> > Speaking of upstream, I posted the patch to their dev mail list but it it
> > seems stuck.
> > Anyone here close to upstream that can bring this patch to their attention?
> 
> Do you have a link ?
> I can't seem to find it.

No I do not. Seems like it never got though the mail list. I guess
they don't let non subscribers into the list but I did not get any rejection reply  either.
Comment 7 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2020-10-11 15:06:45 UTC
(In reply to Joakim Tjernlund from comment #6)
> (In reply to Alexis Ballier from comment #5)
> > (In reply to Joakim Tjernlund from comment #4)
> > > Speaking of upstream, I posted the patch to their dev mail list but it it
> > > seems stuck.
> > > Anyone here close to upstream that can bring this patch to their attention?
> > 
> > Do you have a link ?
> > I can't seem to find it.
> 
> No I do not. Seems like it never got though the mail list. I guess
> they don't let non subscribers into the list but I did not get any rejection
> reply  either.

Subscribe and try again?
Comment 8 Joakim Tjernlund 2020-10-11 15:22:33 UTC
(In reply to Sam James from comment #7)
> (In reply to Joakim Tjernlund from comment #6)
> > (In reply to Alexis Ballier from comment #5)
> > > (In reply to Joakim Tjernlund from comment #4)
> > > > Speaking of upstream, I posted the patch to their dev mail list but it it
> > > > seems stuck.
> > > > Anyone here close to upstream that can bring this patch to their attention?
> > > 
> > > Do you have a link ?
> > > I can't seem to find it.
> > 
> > No I do not. Seems like it never got though the mail list. I guess
> > they don't let non subscribers into the list but I did not get any rejection
> > reply  either.
> 
> Subscribe and try again?

I am afraid I have reached my limit here, so no.
Got my users covered, if Gentoo won't for policy reasons only there is
not much I can do.
Comment 9 Joakim Tjernlund 2020-10-13 10:33:30 UTC
(In reply to Sam James from comment #7)
> (In reply to Joakim Tjernlund from comment #6)
> > (In reply to Alexis Ballier from comment #5)
> > > (In reply to Joakim Tjernlund from comment #4)
> > > > Speaking of upstream, I posted the patch to their dev mail list but it it
> > > > seems stuck.
> > > > Anyone here close to upstream that can bring this patch to their attention?
> > > 
> > > Do you have a link ?
> > > I can't seem to find it.
> > 
> > No I do not. Seems like it never got though the mail list. I guess
> > they don't let non subscribers into the list but I did not get any rejection
> > reply  either.
> 
> Subscribe and try again?

OK, so I caved and subscribed just to post a simple patch:
http://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/270977.html
Comment 10 Martin Cihlář 2020-10-14 06:51:00 UTC
(In reply to Joakim Tjernlund from comment #9)
> OK, so I caved and subscribed just to post a simple patch:
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/270977.html

Oh, fun thread. On one hand, I understand the ffmpeg devs hesitant to restore undocumented behaviour, but on the other, breaking applications (wrongly) using this behaviour is not a commendable decision.

This unfortunately seems to be common behaviour in the FOSS camp and the exact opposite of the proprietary, enterprise software camp - take MS, they will carry all the legacy hacks and cruft just to make sure that all the customers' applications still work.

Either way, thank you Joakim for doing the legwork on this one.
Comment 11 Joakim Tjernlund 2020-10-14 16:03:01 UTC
(In reply to Martin Cihlář from comment #10)
> (In reply to Joakim Tjernlund from comment #9)
> > OK, so I caved and subscribed just to post a simple patch:
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/270977.html
> 
> Oh, fun thread. On one hand, I understand the ffmpeg devs hesitant to
> restore undocumented behaviour, but on the other, breaking applications
> (wrongly) using this behaviour is not a commendable decision.
> 
> This unfortunately seems to be common behaviour in the FOSS camp and the
> exact opposite of the proprietary, enterprise software camp - take MS, they
> will carry all the legacy hacks and cruft just to make sure that all the
> customers' applications still work.
> 
> Either way, thank you Joakim for doing the legwork on this one.

Thanks, not sure they are that much opposed, more that then want a more complete
patch. We will see ...
Comment 12 Sam James archtester Gentoo Infrastructure gentoo-dev Security 2020-10-14 16:09:52 UTC
(In reply to Joakim Tjernlund from comment #11)
> (In reply to Martin Cihlář from comment #10)
> > (In reply to Joakim Tjernlund from comment #9)
> > > OK, so I caved and subscribed just to post a simple patch:
> > > http://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/270977.html
> > 
> > Oh, fun thread. On one hand, I understand the ffmpeg devs hesitant to
> > restore undocumented behaviour, but on the other, breaking applications
> > (wrongly) using this behaviour is not a commendable decision.
> > 
> > This unfortunately seems to be common behaviour in the FOSS camp and the
> > exact opposite of the proprietary, enterprise software camp - take MS, they
> > will carry all the legacy hacks and cruft just to make sure that all the
> > customers' applications still work.
> > 
> > Either way, thank you Joakim for doing the legwork on this one.
> 
> Thanks, not sure they are that much opposed, more that then want a more
> complete
> patch. We will see ...

I do genuinely appreciate the effort here, by the way. I'm just wary about modifying a core application without upstream consent. Thank you.
Comment 13 Joakim Tjernlund 2020-10-18 18:25:45 UTC
James, since you now work for MS I figure you might be interested in this bug
Comment 14 Joakim Tjernlund 2020-11-25 09:03:11 UTC
1.3.00.30857 came a few days ago but the problem persists.

I have given up on upstream ffmpeg too as then cannot make a decision.
Comment 15 Marcel Schilling 2022-04-26 05:30:42 UTC
Is this actually still an issue with net-im/teams-1.5.00.10453? According to http://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/270984.html, chromium got patched to support ffmpeg >= 4.3 a year and a half ago. If Teams still is using and older version than that, maybe we should just consider it dead upstream, purge it from the portage tree and recommend using the web version via a safe browser instead? ;-)
I'd be happy to do some testing but not unless I know there is an actual chance of fixing this old issue and finally allowing to upgrade the system ffmpeg to 4.3 while uisng teams without the bundled version.
Comment 16 Joakim Tjernlund 2022-04-26 07:51:35 UTC
(In reply to Marcel Schilling from comment #15)
> Is this actually still an issue with net-im/teams-1.5.00.10453? According to
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-October/270984.html, chromium
> got patched to support ffmpeg >= 4.3 a year and a half ago. If Teams still
> is using and older version than that, maybe we should just consider it dead
> upstream, purge it from the portage tree and recommend using the web version
> via a safe browser instead? ;-)
> I'd be happy to do some testing but not unless I know there is an actual
> chance of fixing this old issue and finally allowing to upgrade the system
> ffmpeg to 4.3 while uisng teams without the bundled version.

Still using the av_malloc_max(0) patch with ffmpeg-4.4.1-r5
Don't know if it is needed still though.
I think Teams uses Electron 10 now but that is still old.
Only way to find out is to test, it breaks quickly if I recall correctly
Comment 17 Marcel Schilling 2022-04-26 13:10:46 UTC
So I'd just need to copy teams-1.5.00.10453.ebuild to my local overlay, drop  the `system-ffmpeg? ( <media-video/ffmpeg-4.3[chromium]` line, then `emerge -1 media-video/ffmpeg net-im/teams` and check if I can start `teams` without segfaulting? If so, would a patch as teams-1.5.00.10453-r1.ebuild have a chance or would that require additional testing (if so: what)?
Comment 18 Joakim Tjernlund 2022-04-26 13:29:05 UTC
(In reply to Marcel Schilling from comment #17)
> So I'd just need to copy teams-1.5.00.10453.ebuild to my local overlay, drop
> the `system-ffmpeg? ( <media-video/ffmpeg-4.3[chromium]` line, then `emerge
> -1 media-video/ffmpeg net-im/teams` and check if I can start `teams` without
> segfaulting? If so, would a patch as teams-1.5.00.10453-r1.ebuild have a
> chance or would that require additional testing (if so: what)?

yes, and USE=system-ffmpeg too
Comment 19 Marcel Schilling 2022-04-26 13:40:46 UTC
FWIW, according to https://www.electronjs.org/blog/electron-10-0, Electron 10 bundles Chromium 85.0.4183.84, which was stabilized on 2022-09-01 (according to https://chromereleases.googleblog.com/2020/09/stable-channel-update-chrome-os.html). The fix was authored on 2022-08-11 (see https://chromium.googlesource.com/chromium/src/+/7f4c7ff6b0f0e74338c885b0d5e5ef80fed597c3) so I would hope it made it but https://github.com/chromium/chromium/tree/85.0.4183.84/media/base/media.cc#L44 suggest otherwise. :-(
Comment 20 Joakim Tjernlund 2022-04-28 19:50:05 UTC
(In reply to Marcel Schilling from comment #19)
> FWIW, according to https://www.electronjs.org/blog/electron-10-0, Electron
> 10 bundles Chromium 85.0.4183.84, which was stabilized on 2022-09-01
> (according to
> https://chromereleases.googleblog.com/2020/09/stable-channel-update-chrome-
> os.html). The fix was authored on 2022-08-11 (see
> https://chromium.googlesource.com/chromium/src/+/
> 7f4c7ff6b0f0e74338c885b0d5e5ef80fed597c3) so I would hope it made it but
> https://github.com/chromium/chromium/tree/85.0.4183.84/media/base/media.
> cc#L44 suggest otherwise. :-(

Not sure if this helps but teams-insiders 1.5.00.10453 has:
electron
10.4.7 <https://github.com/electron/electron/archive/v10.4.7.zip>
Copyright (c) 2013-2020 GitHub Inc.

Did you try?
Comment 21 Marcel Schilling 2022-04-28 20:34:43 UTC
(In reply to Joakim Tjernlund from comment #20)
> (In reply to Marcel Schilling from comment #19)
> Did you try?

I can build an launch teams and even log in. But it keeps restarting over and over and appears to be unusable. It ends with the following error message:

> There is a glitch. Sorry for the inconvenience. Please sign in again.

Not sure if this is caused by the issue at hand or this is an unrelated problem. I'd need to downgrade my system-ffmpeg or at least rebuild teams with `USE=-system-ffmpeg` to see if it works then. If I find the time, I'll post the result here.
Comment 22 Marcel Schilling 2022-04-29 13:55:45 UTC
(In reply to Joakim Tjernlund from comment #20)

Re-emerging `net-im/teams` with `USE=-system-ffmpeg` fixes the issue. So I guess Electron 10.4.7 doesn't contain the 20 months old Chromium fix yet.