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

Corruption on seek with 444 h264 sample #59

Closed
arch1t3cht opened this issue Mar 14, 2024 · 28 comments
Closed

Corruption on seek with 444 h264 sample #59

arch1t3cht opened this issue Mar 14, 2024 · 28 comments

Comments

@arch1t3cht
Copy link

arch1t3cht commented Mar 14, 2024

Hello,

loading the file Nagi no Asukara 2013 - EP01 [BD 1920x1080 23.976fps AVC-yuv444p10 FLACx3 Chap] - mawen1250.mkv from the classic doom9 collection of test files using core.lsmas.LWLibavSource() (with no other arguments except for a cachefile) in VapourSynth with release 1170 causes major corruption in frames after a few seeks. I wasn't yet able to fully narrow down what property of this file causes this behavior, but I'd guess that it has to do with the 444 subsampling (but reencoding the file to another 444 file using ffmpeg/x264 doesn't cause the same corruption so this seems to not be the only reason). Interestingly enough, AkarinVS/L-SMASH-Works has no issues with this file.

image

This is the only file in this doom9 collection that has seeking errors or corruption with this source filter (except for a couple ones which crash or return errors), so thanks a lot for your work on this source filter.

@Asd-g
Copy link
Contributor

Asd-g commented Mar 14, 2024

AkarinVS/L-SMASH-Works has addition changes (info) that iirc cause other issues while this specific file is ok. I have to check it again.

@arch1t3cht
Copy link
Author

A bit more investigation shows what exactly is happening here:

MediaInfo shows that this file has been encoded with x264 core 142. Early x264 versions had a bug that made them output broken x264 bitstreams for 444 footage. FFmpeg's h264 decoder has specific code that checks for old x264 versions and compensates for this, for example here. And, indeed, editing or removing the SEI specifying the x264 version from the file makes it corrupt in mpv/ffplay.

lwlibav_flush_buffers reopens the decoder, so when the file is seeked past the first keyframe the x264 version SEI won't be read, so the new decoder instance won't know the x264 version and can't compensate for the broken bitstream.

Asd-g added a commit that referenced this issue Mar 17, 2024
Opening and closing the codec in order to flush the buffer could lead
to corrupted decoded frames (x264 version < 151).
On the other side just flushing the buffer could lead to garbage
decoded frames when SPS/PPS is available only in the header.

This is based on AkarinVS/L-SMASH-Works@3d2f02e.
@Asd-g
Copy link
Contributor

Asd-g commented Mar 17, 2024

Hey, you can try the new version r1183.

@arch1t3cht
Copy link
Author

I ran another full seektest with this, the results are below.

So, while this fixes the Nagi no Asukara sample, it breaks a few other ones. However, these other samples are VP9 or MPEG-4 video. So a simple stopgap solution would be to only allow flushing the decoder for H264 video and always reopening it for everything else.

Another much more hacky solution I can think of is manually parsing the first packet of H264 streams and looking for type 5 SEI messages, which are then manually added to the first packed fed to reopened decoders. (Or, even worse, parsing the actual SEI and setting the found x264 version as an x264_version AVOption). But, yes, very hacky.

Seektest results (format is <filename>,<number of errors>):

BoatsAtLahaina_too_few_timecodes.mp4,Crashes
mpeg1.mov,Error
mpeg1.mpg,Failed to output a video frame.
00006.mkv,0
132518447-659caa63-ce2d-4120-9f76-5700cfc7fcb6.mov,0
3d-parrot.mkv,0
3d-parrot.MTS,0
amarec(20200806-1406)_sample.avi,0
apple_trailers.mov,0
AV1 Summer.webm,0
AVC ES.264,0
avidv.avi,0
a_test.mkv,0
bars601.mkv,0
bars601.ts,0
batman.v.superman.mkv,0
BD 21 grammi.mkv,0
camcorder_25i_4-3.mkv,0
CANON.MXF,0
Chimera-AV1-10bit-1280x720-2380kbps.mp4,0
crowd-x265.mp4,0
dvd.mkv,0
dvd.mpeg,0
ffms2_seeking_issue.mkv,0
ffms2_seeking_issue.mp4,0
flpyoj.mkv,0
h263.3gp,0
interlaced_h264.mkv,0
interlaced_h264.mp4,0
Letter - SHE'S.mkv,9087
MainconceptLogo_Blu-ray_MPEG2_1920x1080_LPCM.mkv,0
MainconceptLogo_Blu-ray_MPEG2_1920x1080_LPCM.mpg,0
MainconceptLogo_MPEG2_DVD_720x576.mkv,0
MainconceptLogo_MPEG2_DVD_720x576.mpg,0
MC TMB.mov,0
MPEGSolution_stuart.mkv,0
MPEGSolution_stuart.mp4,8628
Nagi no Asukara 2013 - EP01 [BD 1920x1080 23.976fps AVC-yuv444p10 FLACx3 Chap] - mawen1250.mkv,0
NTSC_720p_MPEG_XDCAM-EX_colorbar.mxf,0
parkrun1280_12mbps.mkv,0
parkrun1280_12mbps.ts,0
rav1e_b_4600.ivf,0
rav1e_q_150.mkv,0
seek-test.mkv,2
snow.MTS,2
Sunset.m2ts,0
Sunset.mkv,0
VC1.mpg,0
vc1_sample.mkv,0
VP9.mkv,1130
VTS_01_1.demuxed.m2v,0
yVY.webm,14732
Zenit.mkv,0

@Asd-g
Copy link
Contributor

Asd-g commented Mar 17, 2024

Did you use the release binary?

I ran that seek test (1000 frames for every video) and the attached file is the result.

Log_2024-03-17__13-59__(0 1000 ['lsmas']).csv

@arch1t3cht
Copy link
Author

Yes, I did use that binary. And when checking one of the files (yVY.webm) manually I do see random minor corruption (small black or green dots on the frame, for example) on seeks.

@Asd-g
Copy link
Contributor

Asd-g commented Mar 17, 2024

I used libvpx for VP9 in the latest release. So the issues you have are caused by libvpx. Interestingly I cannot see any issues with that yVY.webm when jumping on random frames. I manually compared this clip with the previous release by randomly jumping on different frames forward/backward and still no issues - New File (4)002613

b=LWLibavVideoSource("D:\L-SMASH-Works-r1170.0.0.0\yVY.webm", cachefile="D:\L-SMASH-Works-r1170.0.0.0\fdgd.lwi")
LoadPlugin("D:\L-SMASH-Works-r1170.0.0.0\x64\LSMASHSource.dll")
a=LWLibavVideoSource("D:\L-SMASH-Works-r1170.0.0.0\yVY.webm")
Compare(a, b)

Also another run for Letter - SHE'S.mkv and seek-test.mkv:
Untitled

How did you run the test so I can try to reproduce the result?

@arch1t3cht
Copy link
Author

I use this VapourSynth script:

# Originally from https://gist.github.com/HolyWu/c73b296c802f657be7b4b8f8a90b4cf2
# 
# Usage:
# python3 seek-test.py "test files/*"
#
# Change the source filter as needed.

# Note: this script is slow because making the source filter decode frames randomly is slow.

import hashlib
import random
import sys
import os
import itertools
import glob

import vapoursynth as vs
from vapoursynth import core


def hash_frame(frame):
    md5 = hashlib.md5()
    for line in frame.readchunks():
        md5.update(line)
    return md5.hexdigest()


seen_files = []

for filename in itertools.chain(*(glob.glob(p) for p in sys.argv[1:])):
    try:
        with open("seektest.csv") as f:
            seen_files = [l.split(",")[0] for l in f.read().splitlines()]
    except:
        pass

    if filename in seen_files:
        continue

    print(f"Testing {filename}")
    # clip = core.bs.VideoSource(filename, cachesize=0)
    # clip = core.d2v.Source(filename, rff=False)
    # clip = core.dgdecode.MPEG2Source(filename)
    # clip = core.dgdecodenv.DGSource(filename)

    cachefilename = "{}.lwindex".format(filename.replace("/", "_").replace("\\", "_"))

    try:
        os.remove(cachefilename)
    except:
        pass

    try:
        # clip = core.ffms2.Source(filename, cachefile="cache.ffindex")
        clip = core.lsmas.LWLibavSource(filename, cachefile=cachefilename)
    except vs.Error:
        print("Failed to open file.")
        with open("seektest.csv", "a") as logfile:
            logfile.write(f"{filename},Error\n")

    print(f"Clip has {clip.num_frames} frames.")

    reference = []
    for i in range(clip.num_frames):
        reference.append(hash_frame(clip.get_frame(i)))

        if i % 10 == 0:
            print(f"Hashing: {i * 100 // (clip.num_frames - 1)}%", end="\r")

    print("\nClip hashed.\n")

    seek_tests = list(range(clip.num_frames))
    random.shuffle(seek_tests)
    seek_tests += list(range(clip.num_frames))[::-1]

    failures = 0

    for n, i in enumerate(seek_tests):
        try:
            result = hash_frame(clip.get_frame(i))
            if result != reference[i]:
                print(f"Requested frame {i}, ", end="")
                failures += 1
                try:
                    print(f"got frame {reference.index(result)}.")
                except ValueError:
                    print(f"got new frame with hash {result}.")

            if n % 10 == 0:
                print(f"Seeking: {n * 100 // len(seek_tests)}%", end="\r")
        except vs.Error as e:
            failures += 1
            print(f"requesting frame {i} broke source filter.")
            raise e

    print(f"Test complete. {failures} failures in total.")
    print()

    with open("seektest.csv", "a") as logfile:
        logfile.write(f"{filename},{failures}\n")

Here's an example of the corruption, the little yellow line on his forehead is not supposed to be there.
image

@Asd-g
Copy link
Contributor

Asd-g commented Mar 17, 2024

Thanks.

Can you share the same frame with the previous release 1170?

@arch1t3cht
Copy link
Author

Here. Note that the corruption on r1183 is not deterministic. When seeking back and forth a few times it goes away or appears at a different location.

image

@L4cache
Copy link

L4cache commented Mar 19, 2024

\>python seek-test.py yVY.webm
Testing yVY.webm
Creating lwi index file 100%
Clip has 10800 frames.
Hashing: 99%
Clip hashed.

Test complete. 0 failures in total.

I don't see corruption when using the script you provided

@Asd-g
Copy link
Contributor

Asd-g commented Mar 20, 2024

@L4cache, can you test also Letter - SHE'S.mkv?

@L4cache
Copy link

L4cache commented Mar 20, 2024

I tested a few problematic files shown above.
Still no "vp9 specific" problem found.

seek-test.mkv,2
Letter - SHE'S.mkv,0
VP9.mkv,0
MPEGSolution_stuart.mp4,8599
snow.MTS,2

Could it be hardware related issue(s)?

P.S. snow.MTS and seek-test.mkv can pass the test after change to single thread. But AkarinVS/L-SMASH-Works don't need single thread for seek-test.mkv to pass the test (but not for snow.MTS).
snow.MTS is probably just so esoteric so I guess it's OK to just leave it with single thread, but seek-test.mkv seems more important...

@L4cache
Copy link

L4cache commented Mar 21, 2024

New File (4)002613

Just noticed some vertical black lines in your screenshot, is this normal? I don't see them in mine.

@Asd-g
Copy link
Contributor

Asd-g commented Mar 22, 2024

Thanks for testing.

Those vertical black lines are from Compare(a, b) (show_graph=true) (info).

seek-test.mkv failures can be easily fixed by increasing the decoder latency.

@arch1t3cht, how many CPU threads do you have? If more than 16, can you test Letter - SHE'S.mkv and yVY.webm with 16 threads or less?

@L4cache
Copy link

L4cache commented Mar 22, 2024

Thanks for testing.

Those vertical black lines are from Compare(a, b) (show_graph=true) (info).

seek-test.mkv failures can be easily fixed by increasing the decoder latency.

@arch1t3cht, how many CPU threads do you have? If more than 16, can you test Letter - SHE'S.mkv and yVY.webm with 16 threads or less?

Thanks.
Is same thing applies to HEVC? I don't see HEVC codes nearby (besides the cuvid thing).
HEVC has max_latency_increase_plus1 though...

I have 16 threads too.

@arch1t3cht
Copy link
Author

My CPU has 8 cores and 16 logical processors. I tried yVY.webm with threads=0, threads=1, and threads=16 and get corruption in all cases.

@L4cache
Copy link

L4cache commented Mar 22, 2024

My CPU has 8 cores and 16 logical processors. I tried yVY.webm with threads=0, threads=1, and threads=16 and get corruption in all cases.

Instruction sets?

@arch1t3cht
Copy link
Author

It's a Ryzen 7 3700X, so:

MMX instructions
Extensions to MMX
SSE / Streaming SIMD Extensions
SSE2 / Streaming SIMD Extensions 2
SSE3 / Streaming SIMD Extensions 3
SSSE3 / Supplemental Streaming SIMD Extensions 3
SSE4 / SSE4.1 + SSE4.2 / Streaming SIMD Extensions 4
SSE4a
AES / Advanced Encryption Standard instructions
AVX / Advanced Vector Extensions
AVX2 / Advanced Vector Extensions 2.0
BMI / BMI1 + BMI2 / Bit Manipulation instructions
F16C / 16-bit Floating-Point conversion instructions
FMA3 / 3-operand Fused Multiply-Add instructions
SHA / Secure Hash Algorithm extensions
AMD64 / AMD 64-bit technology
SMT / Simultaneous MultiThreading
Precision Boost 2
AMD-V / AMD Virtualization technology

@Asd-g
Copy link
Contributor

Asd-g commented Mar 23, 2024

@arch1t3cht, can you test this yVY.webm file with the attached version?

LSMASHSource.zip

@arch1t3cht
Copy link
Author

Still corrupts, I'm afraid, with the same issues as before.

@L4cache
Copy link

L4cache commented Mar 24, 2024

This could be somthing interesting, I made the script output the reference hashes, and some frames in the last seconds have same hash.
I've changed hash algorithm to sha1 because on modern cpus with sha extensions the speed of sha1 (and sha256 and sha3-256) is significantly faster tham md5.
yVY.webm.hashes.txt

But the sequential decode result of 1183 and 1170 is the same, so this is not defect.

And if the topic is now about VP9 decoding:
After 1183 the _PictType is all ? for VP9 files, not that it's super important but...
I checked the ffms2 I built with libvpx and it's the same.
I checked ffprobe -show_frames results, when using libvpx to decode vp9 the pict_type is actually ?, so this is a FFmpeg's problem I guess.
But the Index file seems to have Key=1,Pic=1 fields, can we make use of them?

@Asd-g
Copy link
Contributor

Asd-g commented Mar 29, 2024

The default VP9 decoder is again the ffmpeg one (capped to 2 threads) and optionally decoder="libvpx-vp9" can be used. Frame props are fixed when libvpx-vp9 is used.

You can test the attached version.

LSMASHSource_r1185.zip

@arch1t3cht
Copy link
Author

Yes, with this version the vp9 files work for me again.

@L4cache
Copy link

L4cache commented Mar 31, 2024

What is the code that controls default decoder selection?

@Asd-g
Copy link
Contributor

Asd-g commented Apr 1, 2024

LWLibavVideoSource(decoder="libvpx-vp9") in the case of VP9.

@L4cache
Copy link

L4cache commented Apr 1, 2024

LWLibavVideoSource(decoder="libvpx-vp9") in the case of VP9.

You've said it.
I'm not asking the obvious, I mean, the code that controls the filter's internal default decoder decision without user intervention, or is it done in ffmpeg libraries?

@Asd-g
Copy link
Contributor

Asd-g commented Apr 1, 2024

One way is to replace the latest line here with

else if (codec_id == AV_CODEC_ID_VP9)
    codec = avcodec_find_decoder_by_name("libvpx-vp9");
return codec;

Other way is to build ffmpeg with --enable-libvpx --disable-decoder=vp9.

@Asd-g Asd-g closed this as completed Apr 2, 2024
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