GSoC 2024: libvmaf: Propagate metadata

[yigithanyigit]

Part of the GSoC 2024 project.

Supports propagating metadata with modified feature_vector_append.

[kylophone]

Thanks @yigithanyigit, will add my review tomorrow morning.

[nilfm99]

I'd like to understand this better - are there plans to make more changes to this function based on this flag, or will it just be used to suppress logs in case of error?

[yigithanyigit]

In my case I used to suppress the logs because when calculating the vmaf score, each feature appends to the collector and attempts to access non-initialized features and is causing numerous errors. However, I am aware that this flag prevents visibility of 'real' errors, I am open to discuss that.

[kylophone]

In general we should avoid introducing unrelated changes into the diff, makes it much harder to read.

[kylophone]

Left a few small comments. I still need to spend some time with your tests to see how this all works.

Also, in general, I think the propagate_metadata naming of this api is not the greatest since it is flexible enough to have other uses outside of ffmpeg AVFrame metadata propagation. Don't worry about changing this now, we can save that for the end.

[kylophone]

@yigithanyigit Looks like there is a use after free in test_model_mount.

You can enable address sanitizer via meson like this: LDFLAGS=-fsanitize=address meson build --buildtype debug -Db_sanitize=address

test_feature_vector_init_append_and_destroy: libvmaf WARNING feature "psnr_y" cannot be overwritten at index 8
�[32mpass�[0m
test_feature_collector_init_append_get_and_destroy: �[32mpass�[0m
test_aggregate_vector_init_append_and_destroy: �[32mpass�[0m
test_model_mount: =================================================================
==7613==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000000ba0 at pc 0x00010d57ed3a bp 0x7ff7b298e1f0 sp 0x7ff7b298e1e8
READ of size 8 at 0x60c000000ba0 thread T0
    #0 0x10d57ed39 in test_model_mount test_feature_collector.c:76
    #1 0x10d57c322 in run_tests test_feature_collector.c:251
    #2 0x10d572753 in main test.c:26
    #3 0x7ff8144f1365 in start+0x795 (dyld:x86_64+0xfffffffffff5c365)

0x60c000000ba0 is located 32 bytes inside of 128-byte region [0x60c000000b80,0x60c000000c00)
freed by thread T0 here:
    #0 0x10e218b69 in wrap_free+0xa9 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0xdcb69)
    #1 0x10d57620e in vmaf_feature_collector_destroy feature_collector.c:443
    #2 0x10d57ecde in test_model_mount test_feature_collector.c:75
    #3 0x10d57c322 in run_tests test_feature_collector.c:251
    #4 0x10d572753 in main test.c:26
    #5 0x7ff8144f1365 in start+0x795 (dyld:x86_64+0xfffffffffff5c365)

previously allocated by thread T0 here:
    #0 0x10e218a20 in wrap_malloc+0xa0 (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0xdca20)
    #1 0x10d5733a6 in vmaf_feature_collector_init feature_collector.c:210
    #2 0x10d57e968 in test_model_mount test_feature_collector.c:57
    #3 0x10d57c322 in run_tests test_feature_collector.c:251
    #4 0x10d572753 in main test.c:26
    #5 0x7ff8144f1365 in start+0x795 (dyld:x86_64+0xfffffffffff5c365)

SUMMARY: AddressSanitizer: heap-use-after-free test_feature_collector.c:76 in test_model_mount
Shadow bytes around the buggy address:
  0x60c000000900: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x60c000000980: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x60c000000a00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x60c000000a80: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x60c000000b00: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
=>0x60c000000b80: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd
  0x60c000000c00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x60c000000c80: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x60c000000d00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x60c000000d80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x60c000000e00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==7613==ABORTING

[nilfm99]

FYI: The build failures are due to cjlin1/libsvm#218 and they are fixed now after #1384. I'll rebase.

[nilfm99]

This new failure seems legitimate.

[yigithanyigit]

This new failure seems legitimate.

Issue looks like related our temporary vmaf.c changes, I am working on it.

Thanks