Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ffmpeg 7.0 Update: Use new channel layout #1626

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

hashworks
Copy link
Contributor

@hashworks hashworks commented May 25, 2024

Context: At Arch Linux I build OvenMediaEngine against the system libraries. Since we currently rebuild stuff for ffmpeg 7.0 this patch is required.

Please bear with me here, I don't really code in C++.

Note: There was a flag FF_API_OLD_CHANNEL_LAYOUT, which has been dropped in v7.

@hashworks hashworks marked this pull request as draft May 25, 2024 17:27
@hashworks hashworks changed the title Draft: ffmpeg 7.0 Update ffmpeg 7.0 Update May 25, 2024
@hashworks hashworks force-pushed the chore/ffmpeg7 branch 5 times, most recently from 25075c5 to 5eb40d5 Compare May 25, 2024 18:32
@hashworks hashworks changed the title ffmpeg 7.0 Update ffmpeg 7.0 Update: Use new channel layout May 25, 2024
@hashworks hashworks force-pushed the chore/ffmpeg7 branch 2 times, most recently from 42cfe85 to e135e5b Compare May 25, 2024 18:54
@hashworks hashworks force-pushed the chore/ffmpeg7 branch 4 times, most recently from d04bd4b to 9b8956a Compare May 25, 2024 20:09
@hashworks
Copy link
Contributor Author

This compiles with ffmpeg 7.0 and is currently used in the Arch Linux extra-staging repository: https://gitlab.archlinux.org/archlinux/packaging/packages/ovenmediaengine/-/commit/3a029703f7f9ce35d31487848efd9524516bf220

It is however entirely untested and will remain a draft until then.

@hashworks
Copy link
Contributor Author

hashworks commented Jul 18, 2024

I'm marking this as ready - I've been using this for a few weeks now, including transcoding with ffmpeg v7 openh264.

The Arch Linux package is now using this patch directly.

```
[ 17/505|  3%] [static] libapi_server.a: C++ projects/api_server/controllers/v1/vhosts/apps/output_profiles/output_profiles_controller.cpp => intermediates/RELEASE/objs/api_server/controllers/v1/vhosts/apps/output_profiles/output_profiles_controller.o
In file included from projects/providers/multiplex/multiplex_stream.h:11,
                 from projects/providers/multiplex/multiplex_application.h:16,
                 from projects/api_server/controllers/v1/vhosts/apps/multiplex_channels/multiplex_channels_controller.cpp:13:
projects/modules/ffmpeg/ffmpeg_conv.h: In static member function ‘static bool ffmpeg::Conv::ToMediaTrack(AVStream*, std::shared_ptr<MediaTrack>)’:
projects/modules/ffmpeg/ffmpeg_conv.h:375:130: error: ‘AVCodecParameters’ {aka ‘struct AVCodecParameters’} has no member named ‘channel_layout’; did you mean ‘ch_layout’?
  375 |                                         media_track->GetChannel().SetLayout(ffmpeg::Conv::ToAudioChannelLayout(stream->codecpar->channel_layout));
      |                                                                                                                                  ^~~~~~~~~~~~~~
      |                                                                                                                                  ch_layout
```
```
    /**
     * Audio only. The channel layout bitmask. May be 0 if the channel layout is
     * unknown or unspecified, otherwise the number of bits set must be equal to
     * the channels field.
     * @deprecated use ch_layout
     */
    attribute_deprecated
    uint64_t channel_layout;
```
```
    /**
     * duration of the corresponding packet, expressed in
     * AVStream->time_base units, 0 if unknown.
     * - encoding: unused
     * - decoding: Read by user.
     *
     * @deprecated use duration instead
     */
    attribute_deprecated
    int64_t pkt_duration;
```
…v_get_channel_layout_describe

```
/**
 * Return a description of a channel layout.
 * If nb_channels is <= 0, it is guessed from the channel_layout.
 *
 * @param buf put here the string containing the channel layout
 * @param buf_size size in bytes of the buffer
 * @param nb_channels number of channels
 * @param channel_layout channel layout bitset
 * @deprecated use av_channel_layout_describe()
 */
attribute_deprecated
void av_get_channel_layout_string(char *buf, int buf_size, int nb_channels, uint64_t channel_layout);
```
```
    /**
     * Audio only. The number of audio channels.
     * @deprecated use ch_layout.nb_channels
     */
    attribute_deprecated
    int      channels;
```
Disabling lzo isn't allowed anymore FFmpeg/FFmpeg@df27292

Couldn't find commits for the other two. But I suspect they are now
enabled by default.
Otherwise this fails with `Cannot change ownership to uid 66878, gid
100: Invalid argument`, at least with podman.
Copy link
Contributor

@basisbit basisbit left a comment

Choose a reason for hiding this comment

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

looks good to me. Hope this gets merged into main, because getting ffmpeg 5 to work gets more and more difficult on more current OS versions

@hashworks
Copy link
Contributor Author

hashworks commented Oct 5, 2024

ffmpeg 7.1 changed the API again: FFmpeg/FFmpeg@7f17e0e

AVFilterLink no longer has access to hw_frames_ctx, which moved to the internal FilterLink. I'm not sure what I should do with SetHWFramesCtxOfAVFilterLink (ffmpeg_conv.h#L958):

context->hw_frames_ctx = av_buffer_ref(hw_frames_ref);

av_buffer_unref(&hw_frames_ref);

Any ideas?

@hashworks
Copy link
Contributor Author

(ffmpeg_conv.h#L958)

@Keukhan you added this in 1de467e, can you take a look?

@Keukhan
Copy link
Member

Keukhan commented Oct 6, 2024

(ffmpeg_conv.h#L958)

@Keukhan you added this in 1de467e, can you take a look?

First of all, thank you for the PR. I will internally discuss changing FFmpeg to 7.x, and after that, I will modify the code related to hw_frames_ctx of AVFilterList.

@hashworks
Copy link
Contributor Author

hashworks commented Nov 6, 2024

Since this blocks the ffmpeg 7.1 release on Arch I've moved the package to our ffmpeg4.4 dependency: https://gitlab.archlinux.org/archlinux/packaging/packages/ovenmediaengine/-/commit/d58322c814222a09ceb6d731905efe5bc3b632f5

@dx314
Copy link

dx314 commented Nov 27, 2024

Is this still going to be merged in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants