AccessViolationException calling avio_close

[ispysoftware]

Just updated to the latest version and using the latest build from ffmpeg and my code is throwing AccessViolationException wherever i'm calling avio_close. Are you seeing the same thing?
Not sure if this is a problem with ffmpeg or the wrapper

[Ruslan-B]

I don't see any difference in bindings for avio_close. The main difference between 5.0 and 5.1 is in macros part.
Can you create a minimal test?

[ispysoftware]

thanks for getting back to me.

I found the issue, setting formatContext->pb->seekable = 1 breaks it now for some reason

           var outputFormat = av_guess_format("mp4", @"D:\test.mp4", null);
            var formatContext = avformat_alloc_context();
            formatContext->oformat = outputFormat;
            var videoStream = avformat_new_stream(formatContext, null);
            var videoCodec = avcodec_find_encoder(AVCodecID.AV_CODEC_ID_H264);
            var videoCodecContext = avcodec_alloc_context3(videoCodec);
            avcodec_open2(videoCodecContext, videoCodec, null);
            avio_open(&formatContext->pb, @"D:\test.mp4", AVIO_FLAG_WRITE);
            formatContext->pb->seekable = 1; //<-- comment out to fix
            avformat_write_header(formatContext, null);
            avio_close(formatContext->pb);  //<-- oopsie

[ispysoftware]

i logged a ticket with ffmpeg and they said it's working fine on their side so possibly some issue with the wrapper?

https://trac.ffmpeg.org/ticket/9871?cversion=1&cnum_hist=2

[Ruslan-B]

I see meanwhile can you try new packages #210 (comment)?

DynamicallyLoaded is the closest to what we have now.
StaticallyLinked - is mostly for iOS as ffmpeg is linked statically.
DynamicallyLinked - is a DllImport import version.

In the very beginning you have to initialize the API then the rest is similar to what you have already:
DynamicallyLoadedBindings.Initialize();

[Ruslan-B]

I'll test sniped you wrote.

[ispysoftware]

i can't test that update unfortunately as my project is .net standard 2.0 and i can't update that to 2.1 as that breaks compat with the 4.8 framework project that uses it.

Is there a reason you're using the 2.1 version of net standard? It limits the usability of the library quite a lot.

[Ruslan-B]

Well UnmanagedType.LPUTF8Str is in only in 2.1 standard - and this going to be newest version which supposed to support only net 6.0 as the rest is unsupported by MS any more. Custom marshaller - UTF8Marshaler can help here however in general I would prefer to drop anything lower than 2.1 standard.

[ispysoftware]

the full .net framework is still supported by MS

Support lifecycle
.NET Framework 4.8 is the latest version of .NET Framework and will continue to be distributed with future releases of Windows. As long as it is installed on a supported version of Windows, .NET Framework 4.8 will continue to also be supported.

https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-framework

[Ruslan-B]

Okay I'll add it to targets then.

[ispysoftware]

Hey @Ruslan-B did you manage to repro the original issue?

[Ruslan-B]

Confirming same exception while using .NET 6.0.400 and GPL FFmpeg 5.1.
Using new or old bindings.
Update:
It works for .NET 6.0.400 and GPL FFmpeg 4.4

Here is the log from 5.1

FFmpeg version info: 5.1-full_build-www.gyan.dev
[libx264 @ 00000148f57217c0] The encoder timebase is not set.
[file @ 00000148f5721bc0] Setting default whitelist 'file,crypto,data'
[mp4 @ 00000148f5720c80] Could not find tag for codec none in stream #0, codec not currently supported in container
[AVIOContext @ 00000148f5761dc0] Statistics: 0 bytes written, 0 seeks, 0 writeouts
Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at FFmpeg.AutoGen.Bindings.DynamicallyLoaded.DynamicallyLoadedBindings+<>c.<Initialize>b__6_583(FFmpeg.AutoGen.Abstractions.AVIOContext*)
   at FFmpeg.AutoGen.Abstractions.ffmpeg.avio_close(FFmpeg.AutoGen.Abstractions.AVIOContext*)
   at FFmpeg.AutoGen.Example.Program.Main(System.String[])

And working 4.4

FFmpeg version info: 4.4-full_build-www.gyan.dev
[libx264 @ 000001cdf5124200] The encoder timebase is not set.
[mp4 @ 000001cdf5122d00] Could not find tag for codec none in stream #0, codec not currently supported in container

It doesn't seems like bindings or marshaling - something different.

[hglee]

Hi,

I found different size of checksum in AVIOContext. So it has different offset after checksum.

FFmpeg.AutoGen/FFmpeg/include/libavformat/avio.h

Line 246 in 5194d11

unsigned long checksum;

FFmpeg.AutoGen/FFmpeg.AutoGen/FFmpeg.structs.g.cs

Line 2060 in d7f6e23

public ulong @checksum;

[Ruslan-B]

@hglee I didn't get could you please elaborate?

[hglee]

In FFmpeg.Autogen, size of checksum is 8 bytes (ulong = uint64).

But in native version, size of checksum is 4 bytes in common case (unsigned long = uint32).

So it has different offset in managed version and if you write seekable field in managed,

it corrupts memory around seekable field (maybe protocol_whitelist ?).

So it crashes in avio_close, actually, in av_freep (avio_close -> av_opt_free -> av_freep).

[Ruslan-B]

@hglee it is actually brilliant analysis - we have to re-test - this any way - but I am scratching my head - seems it has to be hinted to solve this - but how many this kind of thing we have at the moment- I think report back to FFmpeg - but again it has to be a collective claim as they wave it out otherwise.

[Ruslan-B]

To make it clear - it is Clang - without any standard so it will work like a magic as long you on platform which can compile it for years and then it breaks)))

[Ruslan-B]

I am not convinced - as double checked - as in 4.4 we have as well - it is possible to redefine ulong in Clang, but why you should - it is 64 bit world not 4 bytes - if it is - then is a bug in ffmpeg - it used to work in 4.4

        public ulong @checksum;

[hglee]

In 4.x, there was no critical fields around seekable in my guess.

FFmpeg.AutoGen/FFmpeg/include/libavformat/avio.h

Line 260 in e0d26f8

int seekable;

And you need to consider size of unsigned long by platforms.

In .NET, ulong is always 8 bytes.

In Windows x86/x64 native, unsigned long is 4 bytes in common case.

But other platforms like Linux, OSX x86/x64 have different size model.

[Ruslan-B]

I know -but I am don't buy it as it would be a change for everting - the rest is working - I don't see it in headers - don't say it isn't the case - but what wise to do here disable structure alignment first...

[FrostKiwi]

offtopic
I'm having a tough time debugging it, but certain C# packaging combinations have caused Windows Defender to trip, because FFmpeg.AutoGen works by calling into a .dll of unmanaged code, I guess. It depends on software layout, but I have Windows Defender sometimes refusing to give FFmpeg permission to write to the file it creates. It's bizzare and I'm not even sure what causes it. So if anyone sees Access Violations, this is one thing I encountered.
/offtopic

[SuRGeoNix]

@Ruslan-B I can confirm (for Windows) that by changing the checksum from ulong to UInt32 (uint) resolves the issue (all the next attributes read properly). I remember even with v4 having an issue when I was using HLSContext private structure which had AVIOContext and it was causing an issue with my alignment!

I will do more testing tomorrow and come back (AVCodecContext does not have this issue which makes things more complicated my mistake that was not long it was uint64_t so it's normal that it does not have an issue)

This is critical and should probably update all long/unsigned long (from C) in structs with Int32/UInt32 (int/uint).

Update: I've a created a sample C lib and confirmed that the size of long and unsigned long is 4 bytes for Windows x64 (imported to c# x64 with a testing struct to confirm it)

[SuRGeoNix]

The good news is that only AVIOContext (and _GUID?) struct is affected. I used your generator to see from the included c headers which have types long / unsigned long : -

DEBUGLONG: AVIOContext unsigned long checksum
DEBUGLONG: _GUID unsigned long  Data1

The bad news should also check functions for in/out params with longs (wasn't able to find any)

Opened also a 'wish' ticket in FFmpeg trac

Possible solutions:

• CppSharp will fix the issue (if they have any, might is just config) and FFmpeg.AutoGen will have different packages for each platform (so we loose the platform independency)
• FFmpeg replaces (the few) longs from public API
• FFmpeg.Autogen hardcodes this (int for windows, intptr for linux, long for mac) (so we loose the platform independency)

[Ruslan-B]

Seems I have free weekend so I'll try to retest this on linux and mac.
Right now it is passible to workaround this issue using type punning just to shadow incorrect structure.
Besides, as another option it is possible to create hidden flavored types to avoid loosing platform independency and do checks in runtime - cumbersome but API wise would be clean.

[Ruslan-B]

@iamcarbon need an assistance here as seems if I do exclusion to this structure all works well - from all test I maid it seems structure size is no different across intel platforms, cant verify it for arm based recent macs

[Ruslan-B]

it sounds funny but if have somewhere definition in preprocessor true = false I have to put more effort to to be handled

[Ruslan-B]

ghosting structures might play here if don't blow library size and do cross platform tricks. Do have definitive felling it has to be reported back to ffmpeg team

[SuRGeoNix]

Found a solution for this, tested and works as it should...
https://github.com/Ruslan-B/FFmpeg.AutoGen/blob/master/FFmpeg.AutoGen.CppSharpUnsafeGenerator/Processing/TypeHelper.cs#L46

// https://github.com/dotnet/runtime/issues/13788
PrimitiveType.Long => "CLong",
PrimitiveType.ULong => "CULong",

[Ruslan-B]

Looks like I have to create platform specific versions as looked in this:
https://github.com/dotnet/runtime/blob/5535e31a712343a63f5d7d796cd874e563e5ac14/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/CULong.cs

Boils down on windows C(U)Long is actually int.

And again we can forget about .net standard 2 it is dotnet 6 change.

May be forking this types from dot net core source code would be a solution.

[SuRGeoNix]

@Ruslan-B Found more issues (mainly for x86) with ptrdiff_t (which seems to be resolved with nint) / size_t data (which seems to be resolved with nuint). Tested with AVFrame that has crop variables as long in x86 it will have wrong values (eg duration field at latest master build).

[Ruslan-B]

great here we go again, means all longs are probably ints but only on windows. I'll check how to separate platforms in nuget - the silly things is it has to be per platform expect windows all the same.

[Ruslan-B]

none the less after 12 years we solving this "mystery"

[SuRGeoNix]

I don't think that you need to separate the nuget to platforms-OSes, using CULong / CLong and nuint / nint should be fine for both x86/x64 and for windows, linux, macos.

[Ruslan-B]

No wont work - given binaries as for .net has to have different structures. I'll look into this. Easy fix for windows - however, will break any *nix system. Will figure out. Besides can be solved with shadow structures. Maybe an option.

[SuRGeoNix]

@Ruslan-B I'm confused, you might be too. I create a NuGet package from Windows with AVIOContext struct (with CULong) and then use it from Ubuntu. They have different size when I run sizeof(AVIOContext) and they should work fine. So:-

#if TARGET_WINDOWS
using NativeType = System.UInt32;
#else
using NativeType = System.UIntPtr;
#endif

The confusing part for me is when the preprocessor directives will be calculated/executed. During Pack/Compile/Build of the package library, during the compilation of the parent project or during runtime. I'm still not sure to be honest but my testing shows that is not happening during the packing of the initial library which means that you don't need to create multiple platforms-os packages.

Update:
Based on above thoughts, I even think that there is no reason to check OS at runtime. For example for EAGAIN

FFmpeg.AutoGen/FFmpeg.AutoGen/FFmpeg.cs

Line 17 in e129a31

EAGAIN = FunctionResolverFactory.GetPlatformId() switch

This could be define as preprocessor directive https://blog.magnusmontin.net/2018/11/05/platform-conditional-compilation-in-net-core/

This is how donet actually does that for TARGET_WINDOWS https://github.com/dotnet/runtime/blob/703df3b3d7ef6c92ffdf3e9221d82539a9dd5fce/src/libraries/System.Net.Security/src/System.Net.Security.csproj#L16

So, my understanding is that all those will be calculated during the final binary which it will be OS specific (you cannot have the same binary for windows/linux/macos). This might not true only for AnyCpu but again we don't need to worry about this as far as we use the proper types (eg. nint).

[Ruslan-B]

I guess I will create another package which target windows OS. we already have an abstraction, kind of clutch but transparent.