Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Working FFmpeg support #31

Merged
merged 2 commits into from
Jul 28, 2022
Merged

Working FFmpeg support #31

merged 2 commits into from
Jul 28, 2022

Conversation

generic-pers0n
Copy link
Member

@generic-pers0n generic-pers0n commented Jul 15, 2022

This version of lib-ffmpeg-support from Audacity is the version that was first introduced. It does not necessarily come with FFmpeg support (although, according to the commit when this was first added, "versions 55 and 58 are supported now").

Unlike in Audacity, lib-ffmpeg-support is not built as a library. It is instead built into Saucedacity's core and the source is stored in src/ffmpeg.

There have also been some miscellaneous changes too. Some subdirectories in the Saucedacity source code: ffmpeg', 'shuttle', 'widgets', and 'xml' have been added as include directories. This is namely to make including some of these files easier all across Saucedacity's core.

This commit fixes #22, which involved avcodec_encode_audio2 returning -22. Funny coincidence indeed.

Resolves: (direct link to the issue)

(short description of the changes and the motivation to make the changes)

  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"

This version of lib-ffmpeg-support from Audacity is the version that was
first introduced. It does not necessarily come with FFmpeg support
(although, according to the commit when this was first added, "versions
55 and 58 are supported now").

Unlike in Audacity, lib-ffmpeg-support is not built as a library. It is
instead built into Saucedacity's core and the source is stored in
src/ffmpeg.

There have also been some miscellaneous changes too. Some subdirectories
in the Saucedacity source code: ffmpeg', 'shuttle', 'widgets', and 'xml'
have been added as include directories. This is namely to make including
some of these files easier all across Saucedacity's core.

This commit fixes #22, which involved avcodec_encode_audio2 returning
-22. Funny coincidence indeed.
@generic-pers0n
Copy link
Member Author

This is a draft for now because I am deciding of what else to do with FFmpeg support. It is certainly not ready merge yet as of this time of opening this PR.

int encodeResult = 0;

// Flush the audio FIFO first if necessary. It won't contain a _full_ audio frame because
// if it did we'd have pulled it from the FIFO during the last encodeAudioFrame() call
if (nFifoBytes > 0)
{
const int nAudioFrameSizeOut = default_frame_size * mEncAudioCodecCtx->channels * sizeof(int16_t);
const int nAudioFrameSizeOut = mDefaultFrameSize * mEncAudioCodecCtx->GetChannels() * sizeof(int16_t);

Check failure

Code scanning / CodeQL

Multiplication result converted to larger type

Multiplication result may overflow 'int' before it is converted to 'unsigned long'.
int nAudioFrameSizeOut = default_frame_size * mEncAudioCodecCtx->channels * sizeof(int16_t);
int ret;
uint8_t *pRawSamples = nullptr;
int nAudioFrameSizeOut = mDefaultFrameSize * mEncAudioCodecCtx->GetChannels() * sizeof(int16_t); int ret;

Check failure

Code scanning / CodeQL

Multiplication result converted to larger type

Multiplication result may overflow 'int' before it is converted to 'unsigned long'.
@generic-pers0n
Copy link
Member Author

generic-pers0n commented Jul 21, 2022

Huh...

So after doing some more research, I realized that we still have support for older FFmpeg versions, namely 2.3.6, after this library integration.

With that said, I now have plans to drop older versions of ffmpeg. FFmpeg 4.1 is going to be the minimum supported version for now. This might be upped in the future if I learn Saucedacity isn't compatible with FFmpeg 4.1 after further changes.

EDIT: added clarification.

This is mostly irrelevant due to the fact that we either load FFmpeg at
runtime or link against the system's FFmpeg at build time. This version
of FFmpeg might've been outdated too, given the outdated nature of some
of these libraries in lib-src.
@generic-pers0n generic-pers0n marked this pull request as ready for review July 25, 2022 03:53
@generic-pers0n generic-pers0n added this to the Saucedacity 1.2 milestone Jul 28, 2022
@generic-pers0n generic-pers0n self-assigned this Jul 28, 2022
@generic-pers0n generic-pers0n merged commit 507b7e6 into main Jul 28, 2022
@generic-pers0n generic-pers0n changed the title Updated FFmpeg support Working FFmpeg support Jul 28, 2022
@generic-pers0n
Copy link
Member Author

Umm...this should've been called "Working FFmpeg support" in the first place. I'll re-open a similar PR later.

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

Successfully merging this pull request may close these issues.

FFmpeg audio export fails with -22 (Invalid Argument)
1 participant