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

Access to [AVOption.default_var.str] causes AccessViolationException #278

Closed
jcyuan opened this issue Sep 6, 2023 · 11 comments
Closed

Access to [AVOption.default_var.str] causes AccessViolationException #278

jcyuan opened this issue Sep 6, 2023 · 11 comments

Comments

@jcyuan
Copy link

jcyuan commented Sep 6, 2023

ENV:
OS: Win10 x64
FFmpeg: 60
FFAutoGen: 6.0

Screenshots:
image

image

@jcyuan
Copy link
Author

jcyuan commented Sep 6, 2023

test case:

var avClass = ffmpeg.avformat_get_class();

var result = new List<RawAVOption>(128);
if (avClass == null)
    return result;

var option = avClass->option;
while (option != null)
{
    if (option->type != AVOptionType.AV_OPT_TYPE_CONST)
        result.Add(new RawAVOption(option));          // crash
    option = ffmpeg.av_opt_next(avClass, option);
}

class RawAVOption {
    internal unsafe RawAVOption(AVOption* option)
    {
        DefaultString = PtrToUTF8String(option->default_val.str);  // if (*ptr == 0) crash
    }
}

@hglee
Copy link

hglee commented Sep 21, 2023

Would you check more on types?

https://ffmpeg.org/doxygen/trunk/opt_8c_source.html#l01422

For example,

            var avClass = ffmpeg.avformat_get_class(); 
            var opt = avClass->option;

            while (opt != null)
            {
                if (opt->type == AVOptionType.AV_OPT_TYPE_COLOR ||
                    opt->type == AVOptionType.AV_OPT_TYPE_IMAGE_SIZE ||
                    opt->type == AVOptionType.AV_OPT_TYPE_STRING ||
                    opt->type == AVOptionType.AV_OPT_TYPE_DICT ||
                    opt->type == AVOptionType.AV_OPT_TYPE_VIDEO_RATE ||
                    opt->type == AVOptionType.AV_OPT_TYPE_CHLAYOUT)
                {
                    string optName;
                    string optVal;

                    if (opt->name != null)
                    {
                        var str = Marshal.PtrToStringAnsi(new IntPtr(opt->name));
                        if (!string.IsNullOrWhiteSpace(str))
                        {
                            optName = str;
                        }
                        else
                        {
                            optName = "(error)";
                        }
                    }
                    else
                    {
                        optName = "(null)";
                    }

                    if (opt->default_val.str != null)
                    {
                        var str = Marshal.PtrToStringAnsi(new IntPtr(opt->default_val.str));
                        if (!string.IsNullOrWhiteSpace(str))
                        {
                            optVal = str;
                        }
                        else
                        {
                            optVal = "(error)";
                        }
                    }
                    else
                    {
                        optVal = "(null)";
                    }

                    var res = $"Type: {opt->type}, Name:{optName}, Val:{optVal}";

                    Console.WriteLine(res);
                }

                opt = ffmpeg.av_opt_next(avClass, opt);
            }

@jcyuan
Copy link
Author

jcyuan commented Sep 21, 2023

Would you check more on types?

https://ffmpeg.org/doxygen/trunk/opt_8c_source.html#l01422

For example,

i didn't test according to types, i have just tested with all options to see which will cause crash, and here is the result:

Passed options:

Type: AV_OPT_TYPE_FLAGS, Name:avioflags, Val:(null)
Type: AV_OPT_TYPE_CONST, Name:direct, Val:(error)
Type: AV_OPT_TYPE_INT, Name:packetsize, Val:(null)
Type: AV_OPT_TYPE_CONST, Name:ignidx, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:genpts, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:nofillin, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:noparse, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:igndts, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:discardcorrupt, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:nobuffer, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:bitexact, Val:(error)
Type: AV_OPT_TYPE_BOOL, Name:seek2any, Val:(null)
Type: AV_OPT_TYPE_INT64, Name:analyzeduration, Val:(null)
Type: AV_OPT_TYPE_BINARY, Name:cryptokey, Val:(null)
Type: AV_OPT_TYPE_FLAGS, Name:fdebug, Val:(null)
Type: AV_OPT_TYPE_CONST, Name:ts, Val:(error)
Type: AV_OPT_TYPE_INT, Name:audio_preload, Val:(null)
Type: AV_OPT_TYPE_INT, Name:chunk_duration, Val:(null)
Type: AV_OPT_TYPE_INT, Name:chunk_size, Val:(null)
Type: AV_OPT_TYPE_FLAGS, Name:f_err_detect, Val:(error)
Type: AV_OPT_TYPE_FLAGS, Name:err_detect, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:crccheck, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:bitstream, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:buffer, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:explode, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:ignore_err, Val:(error)
Type: AV_OPT_TYPE_BOOL, Name:use_wallclock_as_timestamps, Val:(null)
Type: AV_OPT_TYPE_INT64, Name:skip_initial_bytes, Val:(null)
Type: AV_OPT_TYPE_BOOL, Name:correct_ts_overflow, Val:(error)
Type: AV_OPT_TYPE_DURATION, Name:output_ts_offset, Val:(null)
Type: AV_OPT_TYPE_INT, Name:f_strict, Val:(null)
Type: AV_OPT_TYPE_INT, Name:strict, Val:(null)
Type: AV_OPT_TYPE_CONST, Name:very, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:strict, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:normal, Val:(null)
Type: AV_OPT_TYPE_INT, Name:max_ts_probe, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:disabled, Val:(null)
Type: AV_OPT_TYPE_CONST, Name:make_non_negative, Val:(error)
Type: AV_OPT_TYPE_CONST, Name:make_zero, Val:(error)
Type: AV_OPT_TYPE_STRING, Name:dump_separator, Val:, 
Type: AV_OPT_TYPE_STRING, Name:codec_whitelist, Val:(null)
Type: AV_OPT_TYPE_STRING, Name:format_whitelist, Val:(null)
Type: AV_OPT_TYPE_STRING, Name:protocol_whitelist, Val:(null)
Type: AV_OPT_TYPE_STRING, Name:protocol_blacklist, Val:(null)
Type: AV_OPT_TYPE_INT, Name:max_streams, Val:(error)
Type: AV_OPT_TYPE_BOOL, Name:skip_estimate_duration_from_pts, Val:(null)
Type: AV_OPT_TYPE_INT, Name:max_probe_packets, Val:(error)

Crashed options

probesize                INT64
formatprobesize     INT
fflags                       FLAGS
sortdts                    CONST
fastseek                  CONST
shortest                  CONST
autobsf                   CONST
indexmem              INT
rtbufsize                 INT
max_delay              INT
start_time_realtime  INT64
fpsprobesize            INT
careful                     CONST
compliant                CONST
aggressive               CONST
flush_packets          INT
metadata_header_padding   INT
max_interleave_delta            INT64
unofficial                              CONST
experimental                        CONST
avoid_negative_ts                 INT
auto                                     CONST

obviously if an option has value, it will cause crash. and those which does not have (null / string.Empty) won't.

@Ruslan-B
Copy link
Owner

it really depends from the way you build ffmpeg - you can do worse in normal c lang - congratulations you just tested your ffmpeg assembly )

@Ruslan-B
Copy link
Owner

Ruslan-B commented Sep 21, 2023

binaries in repo not redistributable they needed for generation only

@Ruslan-B
Copy link
Owner

Ruslan-B commented Sep 21, 2023

again if you step in file witch has ".c" on end it all unofficial and not testable so welcome to test and trail, you have access to API there few complex issues which cannot be fix easily and conveniently as marshaling falls and the ways C lang and LLVM reads code - please be respectful to work I did. I do appreciate you made but if you have an Idea how to improve it from the official API ".h" I really take it seriously. I start adding ".c" entire project will be unsable :)

@Ruslan-B
Copy link
Owner

if you mid take a look here #271 most probably same issue

@hglee
Copy link

hglee commented Sep 22, 2023

@jcyuan Because default_val is union, you need to check type carefully. You should not access str field if type is not intend to as string.

@Ruslan-B
Copy link
Owner

Ruslan-B commented Sep 22, 2023

@hglee thanks - and it usually the problem when union appear - I think who did this in C and C++ later than made life of novice developer as way complex than it should be - but rationale clear to me - lack of memory back than.

@jcyuan
Copy link
Author

jcyuan commented Sep 22, 2023

image

Guys, after a thorough comparsion, from clang result it shows the C# binding has no problem, and i have tested with the C code side too, yes as @hglee said that was some usage problem in the logic, some fields don't have string value in it just on purpose, but i'm not sure why they have wild pointer instead of NULL. (on C side)

both side on C# and C has the same result: only 'dump_separator' option has default str value: ", "

but as i tested the code on C side, here i report a problem i encountered although it's off-topic here, so i'm sorry @Ruslan-B
the code will crash at if (av_opt_query_ranges(&r, obj, keyStr, AV_OPT_SEARCH_FAKE_OBJ) >= 0), not sure why, didn't dig into it. @hglee
(obj = avformat_get_class())

without range output, all work fine.

this can be closed yes, and thanks for your effort on this brilliant lib, @Ruslan-B

@Ruslan-B
Copy link
Owner

Ruslan-B commented Sep 22, 2023

but i'm not sure why they have wild pointer instead of NULL. (on C side)

we usually don't care but it is most probably compiler optimization on C side - code base of ffmpeg and amount of contributors... well hardy control abled

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

No branches or pull requests

3 participants