1958036 - Regression in 1850615 breaks V4L2 h264 decoding

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[David Turner]

Steps to reproduce:

I've found that h264 hw hardware decode (using v4l2 stateful via ffmpeg) is broken on Raspberry Pi 4 with the firefox 137 release. I've bisected the breakage down to https://phabricator.services.mozilla.com/D239857 / bug 1850615.

I haven't had a chance to debug further yet what exactly is going wrong, but please could I be given permission to see bug 1850615 so I can find out the logic behind that patch?

Testing on Raspberry Pi OS bookworm, 64-bit, Raspberry Pi 4 (aarch64)

Actual results:

VP9 video plays correctly. If I try to play an h264 video it just shows black and never starts playing. After attempting to play h264 video, I can't even play vp9 videos any more until I restart the browser.

Expected results:

h264 video playback should work, and should not break playback of all other videos.

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Core::Audio/Video' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[BugBot [:suhaib / :marco]]

:karlt, since you are the author of the regressor, bug 1850615, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[David Turner]

With MOZ_LOG=FFmpegVideo:4, in the working case I see:

[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG: FFmpegVideoDecoder::FFmpegVideoDecoder MIME video/avc Codec ID 27
[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG: Initialising VA-API FFmpeg decoder
[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG:   codec h264 : H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10
libva info: VA-API version 1.17.0
libva info: Trying to open /usr/lib/aarch64-linux-gnu/dri/v3d_drv_video.so
libva info: va_openDriver() returns -1
[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG:   vaInitialize failed.
[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG:   Failed to create VA-API device context
[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG: Initialising V4L2-DRM FFmpeg decoder
[RDD 46514: MediaSupervisor #1]: D/FFmpegVideo FFMPEG:   V4L2 codec h264_v4l2m2m : V4L2 mem2mem H.264 decoder wrapper
<snip>

Whereas in the non-working case, it just stops after the va_openDriver() returns -1 line, I don't see vaInitialize failed. So it seems like what is broken is specifically vaapi failing and falling back to v4l2 (and thence software). (Does this mean it's broken video on all systems without working vaapi?)

Comment 4

[David Turner]

Done some more digging. There's something odd going on in VADisplayHolder::GetSingleton() - I see the call to vaInitialize fails, and we get into the if (status != VA_STATUS_SUCCESS) { branch, but it doesn't seem to ever return to CreateVAAPIDeviceContext().

If I throw in a return nullptr; in GetSingleton() before the new VADisplayHolder line then everything works fine, if I put the GetSingleton() after the new VADisplayHolder line then it fails with the original issue. So either there's something weird happening with the ~VADisplayHolder destructor or the stack is getting screwed up. I put some logs in MaybeDestroy() and ~VADisplayHolder() and we never get there.

Will investigate further next week.

Comment 5

[Karl Tomlinson (:karlt)]

Thank you for the analysis.
VADisplayHolder::MaybeDestroy() would be waiting to take the lock already held by GetSingleton().

Comment 7

[Karl Tomlinson (:karlt)]

Attachment: Bug 1958036 use UniqueFileHandle to close DRM file descriptor r?#media-playback-reviewers

This now closes even when vaGetDisplayDRM() fails.

Comment 8

[Karl Tomlinson (:karlt)]

Attachment: Bug 1958036 delay VADisplayHolder construction until it is known usable r?#media-playback-reviewers

This avoids a deadlock attempting to acquire sDisplayHolderMutex in
MaybeDestroy() on releasing the last reference during GetSingleton() after
vaInitialize() failure.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c27e68d60e42
use UniqueFileHandle to close DRM file descriptor r=media-playback-reviewers,stransky

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c50f774fe5d3
delay VADisplayHolder construction until it is known usable r=media-playback-reviewers,stransky

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/c27e68d60e42

Comment 12

[Goloman Adrian[:agoloman]]

https://hg.mozilla.org/mozilla-central/rev/c50f774fe5d3

Comment 13

[David Turner]

I've confirmed that this patch fixes V4L2 playback on latest nightlies, thanks for the speedy fix!

I think this would be worth uplift to 138 beta. I don't mind for Raspberry Pi OS as I can backport the fix downstream, but I'd expect this issue to affect all other aarch64/SBC users.

Comment 14

[Karl Tomlinson (:karlt)]

Attachment: Bug 1958036 use UniqueFileHandle to close DRM file descriptor r?#media-playback-reviewers

This now closes even when vaGetDisplayDRM() fails.

Original Revision: https://phabricator.services.mozilla.com/D244463

Comment 15

[Karl Tomlinson (:karlt)]

Attachment: Bug 1958036 delay VADisplayHolder construction until it is known usable r?#media-playback-reviewers

This avoids a deadlock attempting to acquire sDisplayHolderMutex in
MaybeDestroy() on releasing the last reference during GetSingleton() after
vaInitialize() failure.

Original Revision: https://phabricator.services.mozilla.com/D244464

Comment 16

[Phabricator Automation]

beta Uplift Approval Request

• User impact if declined: Video playback stops for all videos in some Linux system configurations.
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: Lower.
• Explanation of risk level: Smaller patches affecting Linux-only video playback.
• String changes made/needed: None.
• Is Android affected?: no

Comment 17

[Karl Tomlinson (:karlt)]

A "workaround" for those affected could be to set "media.hardware-video-decoding.enabled" to false, but that would of course mean software decoding.

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/6665340824f8
https://hg.mozilla.org/releases/mozilla-beta/rev/3626edb1965c

Comment 20

[Karl Tomlinson (:karlt)]

Attachment: Bug 1958036 use UniqueFileHandle to close DRM file descriptor r?#media-playback-reviewers

This now closes even when vaGetDisplayDRM() fails.

Original Revision: https://phabricator.services.mozilla.com/D244463

Comment 21

[Karl Tomlinson (:karlt)]

Attachment: Bug 1958036 delay VADisplayHolder construction until it is known usable r?#media-playback-reviewers

This avoids a deadlock attempting to acquire sDisplayHolderMutex in
MaybeDestroy() on releasing the last reference during GetSingleton() after
vaInitialize() failure.

Original Revision: https://phabricator.services.mozilla.com/D244464

Comment 22

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Playback stops for all videos and audio elements in some Linux system configurations.
• Code covered by automated testing: no
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: n/a
• Risk associated with taking this patch: Lower
• Explanation of risk level: Smaller patches affecting Linux-only video playback
• String changes made/needed: none
• Is Android affected?: no

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/e462e8a44d38
https://hg.mozilla.org/releases/mozilla-esr128/rev/ff10a64936bb