Find muxer options in VideoEncoder to test fragmented mp4#1216
Find muxer options in VideoEncoder to test fragmented mp4#1216Dan-Flores wants to merge 9 commits intometa-pytorch:mainfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/meta-pytorch/torchcodec/1216
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 50aae18 with merge base 6f10da0 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
…o fragmented_mp4_perhaps
This reverts commit c35d1e8.
| // https://github.com/FFmpeg/FFmpeg/blob/n8.0/libavformat/movenc.c#L8666 | ||
| if (status > 0) { | ||
| status = AVSUCCESS; | ||
| } |
There was a problem hiding this comment.
Without this conversion, we interpret the positive returned integer as a random low level error unrelated to the encoding process (ex. 24 raises EMFILE: too many open files). It might just be a bug in the FFmpeg implementation that we can work around.
Since FFmpeg errors are negative, I think its fine to set the status to successful here.
There was a problem hiding this comment.
Thanks for providing context! Would we want to also add a comment on how ffmpeg errors are negative and link to source code?
NicolasHug
left a comment
There was a problem hiding this comment.
A few Qs and nits but LGTM, thanks @Dan-Flores
| key.c_str(), | ||
| nullptr, | ||
| 0, | ||
| AV_OPT_SEARCH_FAKE_OBJ, |
There was a problem hiding this comment.
Any reason not to use AV_OPT_SEARCH_CHILDREN here? It's used above for fmtOpt
There was a problem hiding this comment.
We only use AV_OPT_SEARCH_CHILDREN when an AVClass has children to iterate over.
I checked that the AVFormatContext class defines child_next and child_class_iterate to iterate through child fields:
https://github.com/FFmpeg/FFmpeg/blob/release/8.0/libavformat/options.c#L128-L137
While the muxer class for MOV does not define these functions:
https://github.com/FFmpeg/FFmpeg/blob/release/8.0/libavformat/movenc.c#L132-L137
test/test_encoders.py
Outdated
|
|
||
| @pytest.mark.skipif( | ||
| ffmpeg_major_version == 4, | ||
| reason="On FFmpeg 4 we error on truncated packets", |
There was a problem hiding this comment.
Where do we error? By "we" you mean TC, or FFmpeg (i.e. is this limitation enforced by us, or by FFmpeg)?
There was a problem hiding this comment.
This is a torchcodec limitation, I was able to decode the truncated video with FFmpeg 4. I'll update the reason message.
The error we get is Invalid data found when processing input, which means we are hitting AVERROR_INVALIDDATA (see errors), which we do not handle, we only check for AVERROR_EOF:
torchcodec/src/torchcodec/_core/SingleStreamDecoder.cpp
Lines 265 to 272 in 6f10da0
Since this error only occurs on FFmpeg 4, I think it should be ok to leave as is.
test/test_encoders.py
Outdated
| assert len(truncated_decoder) >= 10 | ||
| for i in range(10): | ||
| truncated_frame = truncated_decoder.get_frame_at(i) | ||
| assert torch.equal(truncated_frame.data, reference_frames[i].data) |
There was a problem hiding this comment.
Use torch.testing.assert_equal(..., rtol=0, atol=0), it provides better error messages than torch,equal.
| # frag_duration creates fragments based on duration (in microseconds) | ||
| {"movflags": "+empty_moov", "frag_duration": "1000000"}, | ||
| ], | ||
| ) |
There was a problem hiding this comment.
This is a fun test!
Just to make sure, did you verify that it breaks if we don't pass the extra_option flags?
There was a problem hiding this comment.
Yes, it fails to open the file when using extra_options={}:
RuntimeError: Could not open input file: .../fragmented_output.mp4 Invalid data found when processing input
Hopefully this fun test does not introduce flakiness!
…o fragmented_mp4_perhaps
This PR adds logic to
sortCodecOptionsto find and set muxer options (check available options withffmpeg -h muxer=mp4)With these changes, we can now test that fragment related
movflagoptions are utilized correctly.But, without a 'streaming' video encoder, the benefits of fragmented mp4's are still not unlocked: