Skip to content

Conversation

Dan-Flores
Copy link
Contributor

@Dan-Flores Dan-Flores commented Sep 12, 2025

This PR resolves the AVCodec fields deprecated in FFmpeg 7, described in #873.
This field is replaced by avcodec_get_supported_config, which supports retrieval of multiple AVCodecConfig

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 12, 2025
@Dan-Flores Dan-Flores force-pushed the fix_ch_layouts_ffmpeg7 branch from 479147c to a17b466 Compare September 15, 2025 14:50
@Dan-Flores Dan-Flores changed the title [WIP] Handle deprecated AVCodec fields Handle AVCodec fields deprecated in FFmpeg 7 Sep 16, 2025
@Dan-Flores Dan-Flores marked this pull request as ready for review September 16, 2025 14:36
supportedSampleRates = avCodec.supported_samplerates;
#endif

if (supportedSampleRates == nullptr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been trying to keep FFmpeg version ifdef-ing out of the mainline logic, and keep it all in functions defined in FFMPEGCommon. I think we should defined there as something like:

const int* getSupportedSampleRates(const AVCodec& avCodec);

#else
supportedSampleFormats = avCodec.sample_fmts;
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here about creating a function in FFMPEGCommon.

@Dan-Flores Dan-Flores marked this pull request as draft September 17, 2025 13:29
@Dan-Flores Dan-Flores marked this pull request as ready for review September 18, 2025 17:57
&avCodec,
AV_CODEC_CONFIG_SAMPLE_RATE,
0,
(const void**)&supportedSampleRates,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do a reinterpret_cast here; https://en.cppreference.com/w/cpp/language/reinterpret_cast.html. In general, we should never do a C-style cast, as it's not always obvious what kind of cast a C-style cast will actually become: https://en.cppreference.com/w/cpp/language/explicit_cast.html

&avCodec,
AV_CODEC_CONFIG_SAMPLE_FORMAT,
0,
(const void**)&supportedSampleFormats,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditt on reinterpret_cast.

&avCodec,
AV_CODEC_CONFIG_CHANNEL_LAYOUT,
0,
(const void**)&supported_layouts,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto on reinterpret_cast.

Copy link
Contributor

@scotts scotts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do the reinterpret_cast changes before merging. Otherwise, this is great! Thanks for fixing it.

@Dan-Flores Dan-Flores merged commit c2faa4c into meta-pytorch:main Sep 18, 2025
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants