Fix issue #5632, where missing AUD units for keyframes causes them to be merged with their preceding frame #5652
Conversation
src/demux/video/avc-video-parser.ts
Outdated
| break; | ||
| // IDR | ||
| } | ||
| case 5: | ||
| push = true; | ||
| // handle PES not starting with AUD | ||
| // if we have frame data already, that cannot belong to the same frame, so force a push | ||
| if (VideoSample && VideoSample.frame) { |
There was a problem hiding this comment.
For the most part these changes look good. There are however TS streams that hit this case which show decode artifacts after this change. Checking that the frame is not a key frame fixes #5632, while not introducing artifacts into other TS streams (consecutive IDR NAL units) that enter this path.
| if (VideoSample && VideoSample.frame) { | |
| if (VideoSample?.frame && !VideoSample.key) { |
There was a problem hiding this comment.
I think that actually makes sense in the NAL unit type 1 key frame case as well (or at least, shouldn't hurt anything to have), so added it there too.
Thulinma
force-pushed
the
frame_detection_fix_for_occasional_missing_aud
branch
2 times, most recently
from
eaf3c04
to
884c0b3
Compare
|
I have to set the milestone for this to v1.5.0 because it's made against changed in development recently merged with #5634. If you would like to see this fix sooner in a v1.4.9 patch (comings days vs weeks), please make another PR with these changes against patch/v1.4.x branch. |
|
Please run prettier for builds checks to pass https://github.com/video-dev/hls.js/actions/runs/5549533386/jobs/10133880265?pr=5652 |
Thulinma
force-pushed
the
frame_detection_fix_for_occasional_missing_aud
branch
from
884c0b3
to
5c90fe8
Compare
Done: #5660 (And also ran prettier on both this and that PR) |
This PR will...
Fix issue #5632, where missing AUD units for keyframes causes them to be merged with their preceding frame, rather than forcing two frames.
Are there any points in the code the reviewer needs to double check?
Logic, I guess..? It's a pretty complex case, but I think I covered all edge cases.
Resolves issues:
#5632
Checklist