Repository navigation
Fix struct layout for C 'unsigned long' on Windows - #359
Conversation
C 'unsigned long' is 4 bytes on Windows (LLP64) and 8 bytes on Linux/macOS x64 (LP64). It was mapped to C# ulong (always 8 bytes), so every field after AVIOContext.checksum sat 8 bytes too far on Windows. Measured against MSVC x64 with the committed headers: field native before after checksum 100 104 100 checksum_ptr 104 112 104 seekable 136 144 136 protocol_whitelist 144 152 144 sizeof(AVIOContext) 200 208 200 Managed 'seekable' landed on native 'protocol_whitelist' (a char*), so pb->seekable = 1 wrote into that pointer and avio_close -> av_opt_free -> av_freep crashed on it. That is the AccessViolationException from #217. sizeof(_GUID) was 24 instead of 16 for the same reason. - Re-applies 9959a1c (reverted in 6f24260): CLong/CULong mapping in TypeHelper, conditional using aliases in the generators, and netstandard shims that keep the previous 8-byte behaviour behind an [Obsolete] warning - Adds net8.0 to all five packages, not just Abstractions and FFmpeg.AutoGen. This is what #217 was still missing: without a net* asset the BCL CLong/CULong never come into play - Fixes an empty '#if NET' branch in the ffmpeg static constructor that left EAGAIN at 0 as soon as a net* target framework exists, which made the decode loop throw on the first frame. This likely caused the original revert - Silences CS8981 for the deliberate lower-case public names netstandard2.0/2.1 consumers keep today's layout and behaviour unchanged. Verified: build clean, 22/22 tests, example decodes and re-encodes 1001 frames, managed offsets match the MSVC probe byte for byte. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- ENOMEM, EINVAL and EPIPE are the same on Windows, Linux and Darwin, so they become const and are now usable in case labels - EAGAIN moves to a field initializer, which removes the static constructor that existed only to compute it - Drops the '#else' branch: the package targets netstandard2.0 at the lowest, so NETSTANDARD2_0_OR_GREATER always holds there and the branch could never compile. It also tested PlatformID.MacOSX, which .NET Core never returns - The Darwin check moves behind a single IsDarwin property Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The example decoded and encoded but never muxed: out.h264 was an elementary stream written through a plain FileStream, so AVIOContext never appeared in the demo at all. It now wraps that stream into a real container without re-encoding, which covers avformat_alloc_output_context2, avio_open2, avformat_write_header, av_interleaved_write_frame and av_write_trailer. Raw H.264 carries no timestamps, so presentation times come from the packet index at a fixed rate and are rescaled into the time base the muxer settles on after write_header. The muxer also reports AVIOContext.seekable. That is not decoration: the mov muxer rewinds at trailer time to patch the sizes in moov and mdat, and a non-seekable sink would have to be muxed as fragmented mp4. It doubles as a canary for the struct layout this branch fixes, since seekable sits after the checksum field that was misaligned on Windows. Produces frames/out.mp4: h264 854x480, 25 fps, 1001 frames, 40.04 s, with ftyp/free/mdat/moov all present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The shims still told people to upgrade to .NET 6+, which is what the earlier attempt targeted. Following that literally now makes things worse: the package ships net8.0, netstandard2.1 and netstandard2.0, so a net6.0 project resolves the netstandard2.1 asset and keeps the broken layout. The text also said the size was "a known issue on older frameworks", which reads like an oversight. It is a constraint: a netstandard assembly has one fixed layout and cannot vary per platform, so the shim preserves the historical 8-byte behaviour by design. The '#if NET6_0_OR_GREATER' guards stay as they are. They describe a capability - the BCL having CLong and CULong - rather than the shipped target framework, so they remain correct if net6.0 or net7.0 are ever added. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The frames directory was spelled three ways across six sites - "frames", "./frames" and "frames/..." with hardcoded separators - and the frame file name existed twice: once as the search pattern the encoder reads back and once as the format string the decoder writes. Those two could drift apart without anything failing to compile. All of it now sits in one block at the top of Program, and the paths go through Path.Combine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two text-only leftovers from the previous pull request:
- README said "Nuget" where the project and the ecosystem write "NuGet"
- the comment above the anonymous-member naming did not say what the
synthesized names look like, so a reader had to unpick the ternary. It now
names them: union0, union1, struct0, numbered per declaring struct in
declaration order
Copilot reported the second as a mismatch between the comment and the code,
which was not quite it - the union{N} phrasing it quoted lives in the commit
message, not in the file. The underlying point stood, so the comment now
spells the scheme out.
No generated output changes; both edits are prose.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the generated bindings to correctly model C long / unsigned long on Windows (LLP64) by introducing CLong/CULong and adding a net8.0 target so the runtime’s platform-sized interop types can be used, fixing struct layout issues that can lead to crashes (e.g., AVIOContext.seekable misalignment on Windows). It also cleans up platform errno handling and extends the example to mux H.264 into MP4 to validate seekability behavior.
Changes:
- Map C
long/unsigned longtoCLong/CULongin the generator and regenerate affected structs/delegates. - Add
net8.0to all packages and include conditionalusingaliases for runtimeCLong/CULongon modern TFMs, while keeping netstandard shims for compatibility. - Fix
EAGAINinitialization logic and simplify related errno constants; expand the example app with an MP4 muxing step.
Reviewed changes
Copilot reviewed 16 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| FFmpeg.AutoGen/generated/Structs.g.cs | Regenerated structs to use CULong for affected fields (_GUID, DXVA2, AVIOContext.checksum). |
| FFmpeg.AutoGen/generated/Delegates.g.cs | Regenerated delegate signature to use CULong for AVIOContext_update_checksum. |
| FFmpeg.AutoGen/FFmpeg.AutoGen.csproj | Add net8.0 TFM and suppress CS8981. |
| FFmpeg.AutoGen/CLong.cs | Add netstandard CLong/CULong shim types (Obsolete guidance). |
| FFmpeg.AutoGen.Example/Program.cs | Use constants for output paths and add muxing step invocation. |
| FFmpeg.AutoGen.Example/Mp4Muxer.cs | New example helper that muxes raw H.264 into MP4 without re-encoding. |
| FFmpeg.AutoGen.CppSharpUnsafeGenerator/Processing/TypeHelper.cs | Remap primitive Long/ULong to CLong/CULong in generated output. |
| FFmpeg.AutoGen.CppSharpUnsafeGenerator/Generation/StructuresGenerator.cs | Emit conditional using aliases for runtime CLong/CULong. |
| FFmpeg.AutoGen.CppSharpUnsafeGenerator/Generation/GeneratorBase.cs | Add hook to emit conditional usings before namespace declaration. |
| FFmpeg.AutoGen.CppSharpUnsafeGenerator/Generation/DelegatesGenerator.cs | Emit conditional using aliases for runtime CLong/CULong. |
| FFmpeg.AutoGen.Bindings.StaticallyLinked/FFmpeg.AutoGen.Bindings.StaticallyLinked.csproj | Add net8.0 TFM and suppress CS8981. |
| FFmpeg.AutoGen.Bindings.DynamicallyLoaded/FFmpeg.AutoGen.Bindings.DynamicallyLoaded.csproj | Add net8.0 TFM and suppress CS8981. |
| FFmpeg.AutoGen.Bindings.DynamicallyLinked/FFmpeg.AutoGen.Bindings.DynamicallyLinked.csproj | Add net8.0 TFM and suppress CS8981. |
| FFmpeg.AutoGen.Abstractions/generated/Structs.g.cs | Mirror regenerated structs using CULong where appropriate. |
| FFmpeg.AutoGen.Abstractions/generated/Delegates.g.cs | Mirror regenerated delegate signature using CULong. |
| FFmpeg.AutoGen.Abstractions/FFmpeg.cs | Fix EAGAIN initialization for NET TFMs; make other errno values const. |
| FFmpeg.AutoGen.Abstractions/FFmpeg.AutoGen.Abstractions.csproj | Add net8.0 TFM and suppress CS8981. |
| FFmpeg.AutoGen.Abstractions/CLong.cs | Add netstandard CLong/CULong shim types (Obsolete guidance). |
Suppressed comments (2)
FFmpeg.AutoGen/CLong.cs:33
- Same as above for
CULong: the shim is controlled by!NET6_0_OR_GREATER, but the docs / Obsolete message mention net8.0. Align the wording with the actual compile-time condition to avoid misleading guidance.
/// fixing it. Target net8.0 or newer to get the runtime's own CULong, sized per platform.
/// </summary>
[Obsolete("CULong is 8 bytes here, which is wrong on Windows and shifts every field after it. Target net8.0 or newer for a correct struct layout.")]
FFmpeg.AutoGen.Abstractions/CLong.cs:33
- Same doc/Obsolete mismatch for
CULongin Abstractions: the shim is gated by!NET6_0_OR_GREATER, but the guidance references net8.0. Align these so consumers get accurate upgrade guidance.
/// fixing it. Target net8.0 or newer to get the runtime's own CULong, sized per platform.
/// </summary>
[Obsolete("CULong is 8 bytes here, which is wrong on Windows and shifts every field after it. Target net8.0 or newer for a correct struct layout.")]
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// Target net8.0 or newer to get the runtime's own CLong, which is sized per platform. | ||
| /// </summary> | ||
| [Obsolete("CLong is 8 bytes here, which is wrong on Windows and shifts every field after it. Target net8.0 or newer for a correct struct layout.")] |
| var pPacket = ffmpeg.av_packet_alloc(); | ||
| var packetNumber = 0; |
| AVFormatContext* pInputContext = null; | ||
| ffmpeg.avformat_open_input(&pInputContext, inputPath, null, null).ThrowExceptionIfError(); | ||
| _pInputContext = pInputContext; | ||
| ffmpeg.avformat_find_stream_info(_pInputContext, null).ThrowExceptionIfError(); | ||
|
|
| /// Target net8.0 or newer to get the runtime's own CLong, which is sized per platform. | ||
| /// </summary> | ||
| [Obsolete("CLong is 8 bytes here, which is wrong on Windows and shifts every field after it. Target net8.0 or newer for a correct struct layout.")] |
The serious half is the same trap this branch already fixed once. Adding a net8.0 target framework flips every '#if NETSTANDARD..' to its '#else', and FFmpeg.cs was only one of five such gates: - FunctionResolverFactory fell back to Environment.OSVersion.Platform, which never reports PlatformID.MacOSX on .NET Core. macOS would have resolved LinuxFunctionResolver and asked for libavcodec.so.63 instead of libavcodec.63.dylib, so Initialize() would throw DllNotFoundException - FunctionResolverBase lost its MarshalDirectiveException catch, so throwOnError: false stopped degrading gracefully - string marshalling was gated on NETSTANDARD2_1_OR_GREATER, so net8.0 fell back to ICustomMarshaler instead of LPUTF8Str across 140 entry points per file. NativeAOT does not support that, which would have hit exactly the MAUI and iOS consumers these packages advertise The resolver gates are now gone: every shipped target framework has RuntimeInformation and the generic GetDelegateForFunctionPointer, so the '#else' branches were dead. The marshalling condition became 'NETSTANDARD2_1_OR_GREATER || NET'. Verified in the built assemblies: the net8.0 output no longer references get_OSVersion and no longer embeds the custom marshal descriptor, while netstandard2.0 still does. Reverts ENOMEM/EINVAL/EPIPE to static readonly. Turning them into const was binary-breaking: the fields stop existing, so a consumer upgraded without a recompile throws MissingFieldException. Usable case labels were not worth it. In the example: - the encoder disables B-frames. The elementary stream carries no timestamps at all - the demuxer reports AV_NOPTS_VALUE even when given a framerate - so the muxer can only number packets as they arrive, and with B-frames the mp4 came out with no ctts table and played in decode order. Output is now I P P P rather than I B B B P - pts was frameNumber * fps in a 1/fps time base, so every timestamp was 25x too large. It is the frame index - the frame rate joins the paths as a shared constant; both halves of the pipeline have to agree on it - Mp4Muxer disposes on a failed construction instead of leaking two format contexts, checks nb_streams before indexing streams[0], null-checks av_packet_alloc, and treats a read error as an error rather than as clean end of input Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot flagged that the shim compiles under '!NET6_0_OR_GREATER' while the docs say to target net8.0. Both are right and they answer different questions, so the text now says so instead of leaving the reader to work it out. The guard is about capability: the runtime supplies CLong and CULong from net6.0 onward, so the shim steps aside there, and it keeps doing the right thing if net6.0 or net7.0 are ever added as targets. The advice is about what a consumer can actually do: the package ships net8.0, netstandard2.1 and netstandard2.0, so targeting net6.0 resolves the netstandard2.1 asset and keeps the broken layout. net8.0 is the lowest target that gets the fix. Changing the guard to NET8_0_OR_GREATER instead would be the wrong half to move: adding a net6.0 target later would then compile the shim alongside the runtime's own type in the same namespace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same FFmpeg release, a bindings-level revision on top of it: the Windows struct layout fix, the net8.0 assets and the preprocessor gates that came with them. The fourth segment is non-zero, so NuGet keeps it and the packages ship as 9.0.1.1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Draft — addresses #217, open since 2022. Posting for discussion rather than merge; see Open questions at the end.
The measurement
C
unsigned longis 4 bytes on Windows (LLP64) and 8 on Linux/macOS x64 (LP64). It was mapped to C#ulong, which is always 8. I compiled a probe with MSVC x64 against the committed headers and compared it toAVIOContextas C# lays it out:max_packet_sizemin_packet_sizechecksumchecksum_ptrupdate_checksumread_pauseread_seekseekableprotocol_whitelistsizeof(AVIOContext)sizeof(_GUID)was 24 where the native Windows GUID is 16; on net8.0 it is 16 now, on netstandard it stays 24.This also confirms the crash mechanism hglee worked out in #217 by inspection. Managed
seekablesat at 144, which natively isprotocol_whitelist, achar*. SoformatContext->pb->seekable = 1— the exact line from the original report — wrote 1 into the low half of that pointer, andavio_close→av_opt_free→av_freepthen dereferenced it. Hence the AccessViolationException.Linux is unaffected, and that is measured rather than assumed. The same probe built with gcc 11.4 on x86_64 Linux reports:
ulongchecksumchecksum_ptrread_seekseekableprotocol_whitelistsizeof(AVIOContext)Identical throughout, which explains the thirteen years in production in one line: the generated struct has always had the LP64 shape, because C#
ulongis 8 bytes on every platform. On Linux that happened to be the truth. On Windows it never was.It also means the change is a no-op on Linux x64:
CULongisnuintoutside Windows, so it is the same 8 bytes the field already had.Affected surface is exactly four declarations
TypeHelper.GetTypeNameis shared byStructureProcessor,FunctionProcessorandEnumerationProcessor, so the remapping reaches struct fields, function signatures, delegate signatures, fixed-array element types and enum underlying types alike. After regeneration the onlyCLong/CULongoccurrences in the whole output are:_GUID.Data1AVDXVA2FramesContext.surface_typeAVIOContext.checksumAVIOContext_update_checksum— return type and first parameterNo exported or inline function in FFmpeg 9.0.1 takes or returns a C
long, which confirms SuRGeoNix's 2022 sweep still holds three major versions later.What changed
Re-applies 9959a1c (reverted in 6f24260) —
CLong/CULonginTypeHelper, conditionalusingaliases emitted by the generators, and netstandard shims that preserve the previous 8-byte behaviour behind an[Obsolete]warning. That design was right and is used as-is.Adds
net8.0to all five packages. The original attempt addednet6.0toAbstractionsandFFmpeg.AutoGenonly. This is what Erik-White identified as still missing in #217: without anet*asset the BCLCLong/CULongnever enter the picture, because onnetstandardthe shim is deliberately 8 bytes.Fixes an empty
#if NETbranch.FFmpeg.AutoGen.Abstractions/FFmpeg.cshad:NETis defined for net5.0+, so the moment anet*target framework exists the static constructor does nothing andEAGAINstays 0.AVERROR(EAGAIN)then returns 0, the decode loop exits on the first-11, and every consumer seesResource temporarily unavailableon frame one. The example reproduced this immediately after the target framework was added. This is very likely what caused the original revert — the symptom looks like CLong broke everything, but the two are unrelated.Tidies the errno block in the same file:
ENOMEM,EINVALandEPIPEare identical on Windows, Linux and Darwin, so they becomeconstand can now be used incaselabels;EAGAINmoves to a field initializer, which removes the static constructor that existed only to compute it; the#elsebranch is dropped, since the package targets netstandard2.0 at the lowest and that branch could never compile — it also testedPlatformID.MacOSX, which .NET Core never returns.Silences CS8981 for
ffmpeg,vectorsandfunc, which the modern target framework started warning about.Compatibility
Both halves are measured, not assumed:
main(right-hand column above). No regression for net48, Unity or Mono consumers. The field type changes fromulongto the shimCULong, which is source-breaking but carries an[Obsolete]message pointing at the upgrade.AVIOContext.checksum,_GUID.Data1,AVDXVA2FramesContext.surface_typeand theupdate_checksumdelegate signature are nowCULong. All are internal machinery:checksum/checksum_ptr/update_checksumare driven byffio_*inside libavformat, and the header statessizeof(AVIOContext)must not be used outside libav*.Verification
dotnet build -c Release— 0 errors, 0 warningsdotnet test -c Release— 22/22out.h264byte-identical in size to the run onmainmainexactly on netstandard2.0Open questions
net8.0ornet6.0? The original used net6.0, which is out of support.NET6_0_OR_GREATERstill guards correctly either way.CULongis 8 bytes there follows from CoreLib's#if TARGET_WINDOWS uint #else nuintrather than from a run. Both x64 Apple silicon and Intel are LP64, so the expectation is that macOS behaves like Linux. Longer term the expected offsets can come from the generator itself — clang already computes them,Field.OffsetandClass.Layoutare right there — so the check needs no C sources in the tree at all.ptrdiff_t/size_ton x86 are still wrong (AVFrame crop and duration fields, reported by SuRGeoNix in AccessViolationException calling avio_close #217). The generator runs under the x64 triple and bakes 8 bytes in. Separate issue.EAGAINto 11;RuntimeInformation.IsOSPlatform(OSPlatform.OSX)is false there. Pre-existing, untouched here. The new#if NETbranch does cover Darwin properly.🤖 Generated with Claude Code