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

New example add-sidx with advanced options. #325

Merged
merged 2 commits into from
Apr 19, 2024
Merged

New example add-sidx with advanced options. #325

merged 2 commits into from
Apr 19, 2024

Conversation

tobbee
Copy link
Collaborator

@tobbee tobbee commented Feb 13, 2024

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.

@3052 3052 mentioned this pull request Feb 14, 2024
@3052
Copy link

3052 commented Feb 14, 2024

actually I just noticed something. if you use dash instead:

> ffmpeg -i in.mp4 -c copy -frag_size 9K -movflags dash dash.mp4
> ffmpeg -v verbose -i dash.mp4
[AVIOContext @ 0000021d07aa4880] Statistics: 38174724 bytes read, 54 seeks

FFmpeg reads 94.1% of the file. if you fix:

> add-sidx dash.mp4 fix.mp4
> ffmpeg -v verbose -i fix.mp4
[AVIOContext @ 0000012b85854880] Statistics: 38338564 bytes read, 51 seeks

FFmpeg still reading 94.5% of the file. if I add global sidx with FFmpeg:

> ffmpeg -i dash.mp4 -c copy -movflags global_sidx fix.mp4
> ffmpeg -v verbose -i fix.mp4
[AVIOContext @ 00000169363b4880] Statistics: 226878 bytes read, 2 seeks

its only reading 0.6% of the file

@tobbee
Copy link
Collaborator Author

tobbee commented Feb 15, 2024

@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.

@3052
Copy link

3052 commented Feb 15, 2024

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:

> ffmpeg -v verbose -i 'Orphan Black 1 2 - Instinct.mp4'
[AVIOContext @ 000002547f8eb740] Statistics: 14221312 bytes read, 429 seeks

if I try to fix:

add-sidx 'Orphan Black 1 2 - Instinct.mp4' fix.mp4

FFmpeg only reads 0.1% of the file:

> ffmpeg -v verbose -i fix.mp4
[AVIOContext @ 000001d2f8bf4880] Statistics: 65540 bytes read, 3 seeks

but the file is no longer playable in Firefox or MPC-HC.

@tobbee
Copy link
Collaborator Author

tobbee commented Feb 16, 2024

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

ffmpeg -i bbb_1min.mp4 -c copy -movflags +cmaf+frag_keyframe+skip_trailer frag.mp4

I first generated a fragmented file without sidx and mfra segment boxes.
I then get 27 seeks:

[AVIOContext @ 0x146605640] Statistics: 1048576 bytes read, 27 seeks

Running add-side frag.mp4 frag_w.mp4 I get a file with one top-level sidx, frag_w.mp4.
I then get one initial seek from ffmpeg -v verbose -i frag_w.mp4

[AVIOContext @ 0x12ee04e20] Statistics: 229376 bytes read, 1 seeks

Both frag.mp4 and frag_w.mp4 play in Firefox 122 on Mac.
So I cannot reproduce your problem.

If you think that two sidx boxes are needed you can extend this PR to do that.

bbb_1min.mp4

@3052
Copy link

3052 commented Feb 16, 2024

When I tested, I did not get the same issues with firefox as you.

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.

@tobbee tobbee force-pushed the feat-add-sidx branch 2 times, most recently from e64971b to b1b0dfb Compare April 14, 2024 12:44
@tobbee
Copy link
Collaborator Author

tobbee commented Apr 14, 2024

@3052 I spent quite some time going to the bottom on what is special with your example file.

The major issues were that

  • encryption information boxes like senc, and a uuid box for PIFF still present, although someone already removed the encryption.
  • The mp4ff code could not write, but only read the uuid box.
  • There is no segment start styp boxes indicating the start of segments in the file

I think I've a solution to get passed all of these issues now.
The code will now detect that the file is not encrypted, and not try to do as many things with the encryption information. It will also have the possibility to remove this information to save some space. Regarding the last option, the default of the add-sidx is to interpret your file as one segment with a a lot of fragments something that would only result in one entry in the sidx box. I therefore added an option to start a new segment at every new moof box.

With these changes, you should be able to build examples/add-sidx and run it as

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.

@tobbee tobbee changed the title New method File.UpdateSidx() New example add-side with advanced options. Apr 14, 2024
@3052
Copy link

3052 commented Apr 14, 2024

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

@tobbee
Copy link
Collaborator Author

tobbee commented Apr 16, 2024

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.

@3052
Copy link

3052 commented Apr 16, 2024

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:

> ffmpeg -v verbose -i 'Donnie Darko - 2001.m4v'
[AVIOContext @ 000001eea011b7c0] Statistics: 37269211 bytes read, 1112 seeks

if I try to fix:

add-sidx 'Donnie Darko - 2001.m4v' fix.mp4

FFmpeg only reads 65 kB:

> ffmpeg -v verbose -i fix.mp4
[AVIOContext @ 000002f54e264a80] Statistics: 65536 bytes read, 0 seeks

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.

@tobbee
Copy link
Collaborator Author

tobbee commented Apr 18, 2024

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 -startSegOnMoof.

Running go run . -startSegOnMoof -removeEnc Donnie.Darko.-.2001.m4v fix.mp4 I can play the sequence with ffplay and ffprobe says Duration 01:53:14.04.

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.

@3052
Copy link

3052 commented Apr 18, 2024

looks like -startSegOnMoof fixes it - great work!

@tobbee
Copy link
Collaborator Author

tobbee commented Apr 19, 2024

@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 -startSegOnMoof flag, add-sidx now reports one segment and the sidx box has one entry with a segment of duration 6794s and size 219MB.

I plan to merge this and close your issue. I hope you're fine with that?

@3052
Copy link

3052 commented Apr 19, 2024

sounds good thanks for the hard work

tobbee added 2 commits April 19, 2024 15:18
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
@tobbee tobbee changed the title New example add-side with advanced options. New example add-sidx with advanced options. Apr 19, 2024
@tobbee tobbee merged commit cfd8b12 into master Apr 19, 2024
7 checks passed
@tobbee tobbee deleted the feat-add-sidx branch April 19, 2024 13:22
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

Successfully merging this pull request may close these issues.

2 participants