Skip to content

Commit

Permalink
Bug 1898773 - Ensure we also check for ffmpeg + OpenH264 when determi…
Browse files Browse the repository at this point in the history
…ning codec support. r=media-playback-reviewers,padenot a=RyanVM

When we create a video decoder with ffmpeg, we check if the underlying
decoder implementation is supplied by OpenH264. If so, and the
media.ffmpeg.allow-openh264 is set to false (the default), we refuse to
create the video decoder. This is a problem because the decoder module
for ffmpeg declared support for H264, so we did not try falling back to
our own OpenH264 plugin exposed via GMP.

This patch duplicates the logic check for ffmpeg + OpenH264 in
FFmpegDecoderModule::Supports to ensure we fallback properly.

Differential Revision: https://phabricator.services.mozilla.com/D223843
  • Loading branch information
aosmond committed Sep 26, 2024
1 parent 18e907e commit 5759400
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 10 deletions.
6 changes: 4 additions & 2 deletions dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,10 @@ MediaResult FFmpegDataDecoder<LIBAV_VER>::InitDecoder(AVDictionary** aOptions) {
return MediaResult(NS_ERROR_DOM_MEDIA_FATAL_ERR,
RESULT_DETAIL("unable to find codec"));
}
// openh264 has broken decoding of some h264 videos so
// don't use it unless explicitly allowed for now.
// This logic is mirrored in FFmpegDecoderModule::Supports. We prefer to use
// our own OpenH264 decoder through the plugin over ffmpeg by default due to
// broken decoding with some versions. openh264 has broken decoding of some
// h264 videos so don't use it unless explicitly allowed for now.
if (!strcmp(codec->name, "libopenh264") &&
!StaticPrefs::media_ffmpeg_allow_openh264()) {
FFMPEG_LOG(" unable to find codec (openh264 disabled by pref)");
Expand Down
27 changes: 19 additions & 8 deletions dom/media/platforms/ffmpeg/FFmpegDecoderModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,28 @@ class FFmpegDecoderModule : public PlatformDecoderModule {
mimeType.BeginReading()));
return media::DecodeSupportSet{};
}
AVCodecID codec = audioCodec != AV_CODEC_ID_NONE ? audioCodec : videoCodec;
bool supports = !!FFmpegDataDecoder<V>::FindAVCodec(mLib, codec);
AVCodecID codecId =
audioCodec != AV_CODEC_ID_NONE ? audioCodec : videoCodec;
AVCodec* codec = FFmpegDataDecoder<V>::FindAVCodec(mLib, codecId);
MOZ_LOG(sPDMLog, LogLevel::Debug,
("FFmpeg decoder %s requested type '%s'",
supports ? "supports" : "rejects", mimeType.BeginReading()));
if (supports) {
// TODO: Note that we do not yet distinguish between SW/HW decode support.
// Will be done in bug 1754239.
return media::DecodeSupport::SoftwareDecode;
!!codec ? "supports" : "rejects", mimeType.BeginReading()));
if (!codec) {
return media::DecodeSupportSet{};
}
// This logic is mirrored in FFmpegDataDecoder<LIBAV_VER>::InitDecoder and
// FFmpegVideoDecoder<LIBAV_VER>::InitVAAPIDecoder. We prefer to use our own
// OpenH264 decoder through the plugin over ffmpeg by default due to broken
// decoding with some versions.
if (!strcmp(codec->name, "libopenh264") &&
!StaticPrefs::media_ffmpeg_allow_openh264()) {
MOZ_LOG(sPDMLog, LogLevel::Debug,
("FFmpeg decoder rejects as openh264 disabled by pref"));
return media::DecodeSupportSet{};
}
return media::DecodeSupportSet{};
// TODO: Note that we do not yet distinguish between SW/HW decode support.
// Will be done in bug 1754239.
return media::DecodeSupport::SoftwareDecode;
}

protected:
Expand Down
11 changes: 11 additions & 0 deletions dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,17 @@ MediaResult FFmpegVideoDecoder<LIBAV_VER>::InitVAAPIDecoder() {
FFMPEG_LOG(" couldn't find ffmpeg VA-API decoder");
return NS_ERROR_DOM_MEDIA_FATAL_ERR;
}
// This logic is mirrored in FFmpegDecoderModule::Supports. We prefer to use
// our own OpenH264 decoder through the plugin over ffmpeg by default due to
// broken decoding with some versions. openh264 has broken decoding of some
// h264 videos so don't use it unless explicitly allowed for now.
if (!strcmp(codec->name, "libopenh264") &&
!StaticPrefs::media_ffmpeg_allow_openh264()) {
FFMPEG_LOG(" unable to find codec (openh264 disabled by pref)");
return MediaResult(
NS_ERROR_DOM_MEDIA_FATAL_ERR,
RESULT_DETAIL("unable to find codec (openh264 disabled by pref)"));
}
FFMPEG_LOG(" codec %s : %s", codec->name, codec->long_name);

if (!(mCodecContext = mLib->avcodec_alloc_context3(codec))) {
Expand Down

0 comments on commit 5759400

Please sign in to comment.