-
Notifications
You must be signed in to change notification settings - Fork 85
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
New example add-sidx with advanced options. #325
Conversation
78b1ef3
to
eb221c3
Compare
actually I just noticed something. if you use dash instead:
FFmpeg reads 94.1% of the file. if you fix:
FFmpeg still reading 94.5% of the file. if I add global sidx with FFmpeg:
its only reading 0.6% of the file |
@3052 The ffmpeg dash option generates two sidx boxes per segment just to describe that segment. In my view, this is a waste and does not help with any search. The cmaf option does not generate any sidx boxes, but just does segmentations. The extra level of sidx boxes, and in particular having not one but two per segment, may not be properly handled, but it is not a high priority for me to fix that case. I'd suggest that you only use the cmaf option if that works in the way you want. |
using this file (69.3 MB): https://github.com/3052/sofia/releases/download/v1.1.4/Orphan.Black.1.2.-.Instinct.mp4 no global sidx, so its reading 20% of the file:
if I try to fix:
FFmpeg only reads 0.1% of the file:
but the file is no longer playable in Firefox or MPC-HC. |
There is a lot of things that can happen. I noticed that ffmpeg generates two global sidx boxes and two tfra boxes that point to the same ranges (one for each track), while the mp4ff code currently generates just one. I cannot see from the spec what is correct. In general, the recommendation is to use just a single track for fragmented files in all of DASH, Smooth Streaming and HLS. When I tested, I did not get the same issues with firefox as you. Starting from a short video bbb_1min.mp4 (attached) with the command
I first generated a fragmented file without sidx and mfra segment boxes.
Running
Both frag.mp4 and frag_w.mp4 play in Firefox 122 on Mac. If you think that two sidx boxes are needed you can extend this PR to do that. bbb_1min.mp4 |
I get your same results with your input file, but it seems you never tested MY INPUT file, so the conclusions you made aren't valid. the file I linked was NOT created with FFmpeg, it was just downloaded using other means, sorry if that was not clear. I DON'T care about dual SIDX, I only care about https://github.com/Eyevinn/mp4ff working with "real life" files, which currently it does not in regards to the scope of this issue. |
e64971b
to
b1b0dfb
Compare
@3052 I spent quite some time going to the bottom on what is special with your example file. The major issues were that
I think I've a solution to get passed all of these issues now. With these changes, you should be able to build add-side -removeEnc -startSegOnMoof Orphan.Black.1.2.-.Instinct.mp4 orphanWithSidx.mp4 to get a file with sidx and no encryption boxes. Please give it a try! PS. I've also make some unit tests based on a cropped version of your file with all video content removed. It thus not playable, but one can check the box structure of all the metadata. |
sorry for the trouble, if I had know you were doing extensive work I might have tried to get you a better test file. if I helps I can get you a "clean" version of the same file, with encryption enabled and provide a key. or if you are happy with the current result I can just test the current tool |
Well, there is always something to learn from looking at strange files. The encryption boxes are a bit special since they are not fully self-contained, but need information from other boxes as well. When not just presenting the content of the boxes, but trying to do something meaningful with them, things get complex. Anyway, I'm pretty satisfied with the changes to the core library and to the add-sidx example code, so please try it out. I haven't looked into files with audio and video multiplexed here, but I see that I wrote in #311 that they should be supported, so they may work. I don't use such files myself normally, so it is low priority to support them. |
I still dont think we are good. using this file (209 MB): https://github.com/3052/sofia/releases/download/v1.1.4/Donnie.Darko.-.2001.m4v no global sidx, so its reading 17% of the file:
if I try to fix:
FFmpeg only reads 65 kB:
and the file does play in MPC-HC, but the duration now says 6 seconds, and you cannot seek past that. original file has correct duration of 1h53m14s. looks like Firefox support is correct now for both files. |
I think you should check the flags I added for content that is not signalling the segments properly. Your content doesn't have styp boxes between the segments, and has the encoding boxes remaining. To get add-sidx to treat each moof box as a new segment (and not just a fragment inside a segment), you should use the flag Running Without these flags, I get the behaviour you describe. The duration shouldn't be that short of course, but what is happening is that you get one long segment for the whole asset. There may be something weird happening (an overflow?) for such a long segment (the printout of fragment vs segments is also not right in this case). So, I think your case is actually covered provided you specify the parameters for these types of files that don't have styp (and remaining encryption boxes). I'll try to have a look at the one-segment case as well, but it's not what you want as output for a fragmented file. |
looks like |
@3052 There was an issue with the case where only one segment was detected. It was only its duration that was added in the sidx box. This is now fixed, and without the I plan to merge this and close your issue. I hope you're fine with that? |
sounds good thanks for the hard work |
fix: GetTrex return values feat: File.UpdateSidx to update or add a top-level sidx box feat: Can remove unused enc boxes feat: New decoder option to segment on moof start feat: New moov.IsEncrypted() method fix: make add-sidx work with unused senc and piff boxes fix: GetTrex now returns proper OK values
With UpdateSidx() it is possible to update or add a top-level sidx box for a fragmented file.
There is also a new example program
examples/add-sidx
which shows how this can be applied to an existing file.This should help to improve the startup time for layout discussed in #311.