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.

Sign up for GitHub

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

using LGPL compiled ffmpeg shared for FFmpeg n5.0 from [here](https:/ #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

using LGPL compiled ffmpeg shared for FFmpeg n5.0 from [here](https:/ #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,5 @@ UpgradeLog*.XML
*.stackdump

# Local
/FFmpeg.AutoGen.Example/frame.*.jpg
/FFmpeg.AutoGen.Example/frame.*.jpg
/video-test*
15 changes: 15 additions & 0 deletions FFmpeg.AutoGen.Example.Transcoding/FFmpeg.AutoGen.Example.Transcoding.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net6.0</TargetFramework>
<ImplicitUsings>disable</ImplicitUsings>
<Nullable>enable</Nullable>
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
</PropertyGroup>

<ItemGroup>
<ProjectReference Include="..\FFmpeg.AutoGen\FFmpeg.AutoGen.csproj" />
</ItemGroup>

</Project>
36 changes: 36 additions & 0 deletions FFmpeg.AutoGen.Example.Transcoding/FFmpegBinariesHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
using System;
using System.IO;
using System.Runtime.InteropServices;

namespace FFmpeg.AutoGen.Example
{
public class FFmpegBinariesHelper
{
internal static void RegisterFFmpegBinaries()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
var current = Environment.CurrentDirectory;
var probe = Path.Combine("FFmpeg", "bin", Environment.Is64BitProcess ? "x64" : "x86");

while (current != null)
{
var ffmpegBinaryPath = Path.Combine(current, probe);

if (Directory.Exists(ffmpegBinaryPath))
{
Console.WriteLine($"FFmpeg binaries found in: {ffmpegBinaryPath}");
ffmpeg.RootPath = ffmpegBinaryPath;
return;
}

current = Directory.GetParent(current)?.FullName;
}
Comment on lines +16 to +28
Copy link

Choose a reason for hiding this comment

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

The search for FFmpeg binaries on Windows could potentially traverse up to the root of the file system. Consider adding a limit to the number of directory levels to traverse to prevent excessive searching and to fail early if the binaries are not found in an expected location.

}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
ffmpeg.RootPath = "/lib/x86_64-linux-gnu/";
Comment on lines +30 to +31
Copy link

Choose a reason for hiding this comment

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

The FFmpeg root path for Linux is hardcoded to "/lib/x86_64-linux-gnu/". This may not be accurate for all Linux distributions. Consider making this path configurable or verifying that the directory exists before setting it.

else
throw new NotSupportedException(); // fell free add support for platform of your choose
Copy link

Choose a reason for hiding this comment

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

The NotSupportedException thrown does not include an error message. Consider adding a message to inform the user which platform is not supported and possibly suggest actions to take or where to add support.

- throw new NotSupportedException();
+ throw new NotSupportedException("The current platform is not supported. Please add support for it or use a supported platform.");
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
throw new NotSupportedException(); // fell free add support for platform of your choose
throw new NotSupportedException("The current platform is not supported. Please add support for it or use a supported platform."); // fell free add support for platform of your choose

}
}
}
23 changes: 23 additions & 0 deletions FFmpeg.AutoGen.Example.Transcoding/FFmpegHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using System;
using System.Runtime.InteropServices;

namespace FFmpeg.AutoGen.Example
{
internal static class FFmpegHelper
{
public static unsafe string av_strerror(int error)
{
var bufferSize = 1024;
Copy link

Choose a reason for hiding this comment

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

Consider defining the buffer size as a constant to improve code readability and maintainability.

- var bufferSize = 1024;
+ const int ErrorBufferSize = 1024;
+ var bufferSize = ErrorBufferSize;
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
var bufferSize = 1024;
const int ErrorBufferSize = 1024;
var bufferSize = ErrorBufferSize;

var buffer = stackalloc byte[bufferSize];
ffmpeg.av_strerror(error, buffer, (ulong) bufferSize);
var message = Marshal.PtrToStringAnsi((IntPtr) buffer);
return message;
}

public static int ThrowExceptionIfError(this int error)
{
if (error < 0) throw new ApplicationException(av_strerror(error));
return error;
}
Comment on lines +17 to +21
Copy link

Choose a reason for hiding this comment

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

The method name ThrowExceptionIfError could be improved to follow .NET naming conventions for methods that can throw exceptions. Additionally, consider using a more specific exception type than ApplicationException.

- public static int ThrowExceptionIfError(this int error)
+ public static int EnsureSuccessOrThrow(this int error)
{
    if (error < 0) throw new FFmpegException(av_strerror(error));
    return error;
}
Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
public static int ThrowExceptionIfError(this int error)
{
if (error < 0) throw new ApplicationException(av_strerror(error));
return error;
}
public static int EnsureSuccessOrThrow(this int error)
{
if (error < 0) throw new FFmpegException(av_strerror(error));
return error;
}

}
}
12 changes: 12 additions & 0 deletions FFmpeg.AutoGen.Example.Transcoding/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace FFmpeg.AutoGen.Example
{
public class Program
{
private static void Main(string[] args)
{
FFmpegBinariesHelper.RegisterFFmpegBinaries();
var transcoding = new Transcoding();
transcoding.main(args.Length, args);
Copy link

Choose a reason for hiding this comment

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

The method name main does not follow C# naming conventions. Consider renaming it to Main or another appropriate PascalCase name to align with best practices.

Comment on lines +7 to +9
Copy link

Choose a reason for hiding this comment

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

There is no error handling for exceptions that might be thrown by RegisterFFmpegBinaries or transcoding.main. Consider adding try-catch blocks to handle potential exceptions gracefully.

}
}
}
Loading