GNOME Bugzilla – Bug 784258
qtmoovrecover can't recover video
Last modified: 2017-10-25 06:05:52 UTC
This bug seems to be a regression because I had this recovering feature working in the past - though it was not on the exact same video. I have two videos that have been encoded with gstreamer mp4mux moov-recovery-file. One reaches the EOS, the other is stopped abruptly. Both videos can't be recovered: http://gentil.com/tmp/qtmoovrecover.zip
Here is a simpler way to show case the bug: gst-launch-1.0 videotestsrc ! x264enc ! qtmux moov-recovery-file=test.mrf ! filesink location=test.mov gst-launch-1.0 qtmoovrecover recovery-input=test.mrf broken-input=test.mov fixed-output=testo.mov
What version have you used? Git master? Recently (early March) some fixes for qtmoovrecover were merged and it seems to be working for me after that.
Progress but still not working for me. More precisely: - I have Ubuntu 16.04 with 1.8.3. - I have downloaded the Ubuntu gstreamer1.0-plugins-good sources. - I have applied 4 patches from https://cgit.freedesktop.org/gstreamer/gst-plugins-good/log/gst/isomp4: 2017-03-27 atomsrecovery: Error out when fseek() fails instead of silently ignoring ea4e9fc2d4904117255c80c0b832253fb31d4f7a 2017-03-15 atomsrecovery: also handle extra atoms after 'mdia' in a 'trak' b434ba86f1acecbc1b61fb7832aa6c5432766295 2017-03-15 qtmux: avoid fallthrough to moovrecovery failure section 7e39dec391c37ded3fb5bb7d24c60de6634647f4 2017-03-15 atomsrecovery: expect more atom types at the headers 4d9b17ad77ef078ceff753f41db0e61338d0e20c Strictly speaking, I don't have git master but I truly believe that with those 4 patches I have the same thing as git master. The two command lines I wrote at "2017-06-27 19:04:10 UTC" are now working! But the files provided at "2017-06-27 19:00:04 UTC" can't still be recovered though there is no more error message. What's the problem?
Without 7e39dec391c37ded3fb5bb7d24c60de6634647f4 on qtmux it is unlikely that those files have enough information to be recovered with this tool. The .mrf should not have information about the position of the samples in the .mp4 so it won't be able to reconstruct the headers. How important is recovering those files? Or are you just trying to understand why it won't work with those 2 files?
OK, I start to understand. 7e39dec391c37ded3fb5bb7d24c60de6634647f4 is for the "failing" encoder side. So I need to commit that to my embedded device. Very painful because I'm going to have to recompile everything just for one small missing "return". Is there a way to push this pasth to ubuntu 16.04 1.8.3 package? No. Those files were just samples to test the features before release. Luckily, I can control both the encoder embedded device and the PC on which I do the recovery. I'm still confused about one point: I'm using mp4mux not qtmux. Is there any difference?
Its the same element with a small behaviour change.
It starts working but still not perfect. I have both on encoder-device and PC-fixer gstreamer 1.8.3 with the four patches mentioned in this thread. I can recover a video and play it but the output of mp4info is not very clean. Here are the files of this test: http://gentil.com/tmp/qtmoovrecover2.zip 20160530-071745.mp4: ORIGINAL BROKEN FILE a.mp4: FIXED FILE mp4info 20160530-071745.mp4 mp4info version -r 20160530-071745.mp4: ReadChildAtoms: "20160530-071745.mp4": In atom missing child atom moov FindIntegerProperty: no such property - moov.mvhd.modificationTime (src/mp4file.cpp,746) mp4info: can't open 20160530-071745.mp4 mp4info a.mp4 mp4info version -r a.mp4: ReadAtom: "a.mp4": invalid atom size, extends outside parent atom - skipping to end of "" "�:��" 754660230 vs 18767154 ReadAtom: "a.mp4": atom type �:�� is suspect Track Type Info 1 video H264 Baseline@4.2, 13.510 secs, 9716 kbps, 1920x1440 @ 29.829756 fps 2 audio MPEG-1 Audio (11172-3), 13.740 secs, 0 kbps, 44100 Hz ReadAtom: "a.mp4": invalid atom size, extends outside parent atom - skipping to end of "" "�:��" 754660230 vs 18767154 ReadAtom: "a.mp4": atom type �:�� is suspect Is it possible to have a really clean-up version of the fixed video?
Created attachment 355002 [details] [review] isomp4: atomsrecovery: handle common and large atom headers Do not assume all files are large files. Check and use the short or extended atom size field only if needed.
I have applied the patch but I'm still getting a similar output for mp4info on the provided example. Am I missing something?
Seems to work here with git master after applying this patch: $ mp4info testo.mov mp4info version -r testo.mov: ReadChildAtoms: "testo.mov": In avc1 atom, extra 4 bytes at end of atom Track Type Info 1 video H264 Unknown Profile f4@1.3, 30.500 secs, 1498 kbps, 320x240 @ 30.000000 fps ReadChildAtoms: "testo.mov": In avc1 atom, extra 4 bytes at end of atom
I have tried again and no luck. Upgrading to git master triggers way too many things... A diff between 1.8.3+5_patches and git_master shows no difference. Any idea?
Hard to tell. Maybe something is different in qtmux in 1.8. Can you share the .mov, the .mrf and the recovered .mov files after those patches to see if we can identify what is different? Additionally if you could try a setup with everything git master just to confirm this fix indeed works for master. Thanks.
Yes, tomorrow morning I will try a cerbero. But I don't want to mess my whole Ubuntu machine and overwriting all gstreamer. My command line is: gst-launch-1.0 qtmoovrecover recovery-input=20160530-071745.mrf broken-input=20160530-071745.mp4 fixed-output=a.mp4 File is there: http://gentil.com/tmp/a.zip
It's not working for me with a clean Cerbero + applying the patch. Am I doing something wrong? Can you send your atomsrecovery.c file? Mine: https://pastebin.com/ZWAp0rjH
(In reply to Gregoire from comment #13) > Yes, tomorrow morning I will try a cerbero. But I don't want to mess my > whole Ubuntu machine and overwriting all gstreamer. > > My command line is: > gst-launch-1.0 qtmoovrecover recovery-input=20160530-071745.mrf > broken-input=20160530-071745.mp4 fixed-output=a.mp4 > > File is there: http://gentil.com/tmp/a.zip Would you mind sharing the original unrecovered .mov and .mrf files so I can check them? Maybe your generated files have something different from the ones from videotestsrc. Also, you can use gst-uninstalled script (gstreamer/scripts) to set up an uninstalled environment to try master without having to install to your system.
Never mind, found the files that cause the issue.
The mdat is longer than its size by 2133036 bytes. Can you share the exact pipeline you are using to create these files? I'm guessing it might be related to some parameter you are using.
The pipeline is: appsrc name=source do-timestamp=true format=3 caps=video/x-raw,format=I420,width=1920,height=1440,framerate=30/1 ! nxvideoenc bitrate=10000000 ! queue ! mp4mux.video_0 alsasrc do-timestamp=true ! lamemp3enc ! mp4mux.audio_0 mp4mux moov-recovery-file=video.mrf name=mp4mux ! queue ! filesink location=video.mp4 nxvideoenc is a hardware-based encoder provided by Samsung/Nexell. The device runs the Gstreamer stack 1.8.3 from Ubuntu 16.04LTS on ARM-64bit.
unfortunately I couldn't reproduce it by going back to 1.8.3. Also I don't have nxenc or an ARM64 platform to test. Let me put some information here in case it helps you on anyone else debugging this further. The recovery element takes 2 inputs, one is the .mov and another is the .mrf. In the .mov there is only the stream data, just a massive blob. It starts with a "ftyp" and maybe some minor atoms and then there should be the 'mdat' atom that has all the data inside it. A quick read on the quicktime format spec should give a basic idea of how these files are structured. To recover this file, the .mrf file actually stores header information, which is the size, position and timing information for every sample that was stored to the .mov file. By combining those two it is possible to create a full mov/mp4 file that has the full headers and the data. In your case, the mdat file seems to be longer than what the mrf headers have stored and it will cause some extra random binary data to be placed at the end of the file. There is one way to improve this by limiting the mdat output to only write to disk what is actually in the mrf headers but that would only hide the real problem that is why that extra data is in the end on the first place. Maybe by adding some extra debgugging to your setup you can figure out where does that come from and why there isn't headers in the mrf for that data.
First, I appreciate your efforts. Please correct me if I'm wrong: from what I have understood, the problem is more during the encoding stage where the mdat is longer than the mrf headers or said differently, the mrf headers are shorter than the mdat. Perhaps there is an issue to write the mrf headers to file. What I'm confused about is that the mdat file is written at the beginning of the file, so how can the system know how long is going to be the mdat or the mrf headers? Anyway, the investigation is more on the encoding stage rather than during the pure recovery. Could you point in the code where I should hack/patch the recovery phase so that the mdat is trunk-ed to the size of the mrf headers?
(In reply to Gregoire from comment #20) > First, I appreciate your efforts. > > Please correct me if I'm wrong: from what I have understood, the problem is > more during the encoding stage where the mdat is longer than the mrf headers > or said differently, the mrf headers are shorter than the mdat. Perhaps > there is an issue to write the mrf headers to file. Yes, it looks to be on the encoding side. > > What I'm confused about is that the mdat file is written at the beginning of > the file, so how can the system know how long is going to be the mdat or the > mrf headers? It doesn't know, but for every sample that it puts in the mdat it writes about this sample on the mrf (size, offset, timing information, stream to which it belongs). The same information is kept in memory to write the final headers in case the file is successfully finished. The mrf is an extra step that is done, if enabled. > > Anyway, the investigation is more on the encoding stage rather than during > the pure recovery. > > Could you point in the code where I should hack/patch the recovery phase so > that the mdat is trunk-ed to the size of the mrf headers? The mdat is written by the pipeline (filesink), so mp4mux writes to it by pushing buffers downstream. The mrf is written by the "atoms_recov_write_trak_samples" function. Look in qtmux code for "moov_recov_file". Try to track the data added to the mrf and correlate with the buffers pushed downstream. You might find that some extra data is being pushed or some data is not being sent to the mrf. Good luck!
That' Good luck!' sounds like 'I don't want to hear about you ever ;-)' hahaha Thanks for the clarification. It helps. I understand but I think there are still things are not completely correct (independently of my situation): - can you please commit your latest patch to the tree, that would simplify the testing. This patch definitely helps. - I have done again a clean git master cerbero + the patch in this bug report, I still get a mild error: git clone git://anongit.freedesktop.org/gstreamer/cerbero ./cerbero-uninstalled bootstrap ./cerbero-uninstalled package gstreamer-1.0 ./cerbero-uninstalled buildone gst-plugins-good-1.0 (with patch) ./cerbero-uninstalled shell gst-launch-1.0 videotestsrc ! x264enc ! qtmux moov-recovery-file=test.mrf ! filesink location=test.mov CONTROL-C gst-launch-1.0 qtmoovrecover recovery-input=test.mrf broken-input=test.mov fixed-output=testo.mov mp4info testo.mov As you can see, it still reports something incorrect. mp4info version -r testo.mov: ReadChildAtoms: "testo.mov": In avc1 atom, extra 4 bytes at end of atom Track Type Info 1 video H264 Unknown Profile f4@1.3, 68.533 secs, 1500 kbps, 320x240 @ 30.000146 fps ReadChildAtoms: "testo.mov": In avc1 atom, extra 4 bytes at end of atom - My question in my previous post was more to hack/fix the problem in the recovery stage rather than in the encoding stage. In the atomsrecovery.c file, where is the logic that (could/would/should) compare the size of the mdat in the broken file and the size of the headers in the mrf file?
(In reply to Gregoire from comment #22) > > - can you please commit your latest patch to the tree, that would simplify > the testing. This patch definitely helps. Yeah, will merge it soon. > > - I have done again a clean git master cerbero + the patch in this bug > report, I still get a mild error: > > git clone git://anongit.freedesktop.org/gstreamer/cerbero > ./cerbero-uninstalled bootstrap > ./cerbero-uninstalled package gstreamer-1.0 > ./cerbero-uninstalled buildone gst-plugins-good-1.0 (with patch) > ./cerbero-uninstalled shell > gst-launch-1.0 videotestsrc ! x264enc ! qtmux moov-recovery-file=test.mrf ! > filesink location=test.mov > CONTROL-C > gst-launch-1.0 qtmoovrecover recovery-input=test.mrf broken-input=test.mov > fixed-output=testo.mov > mp4info testo.mov > > As you can see, it still reports something incorrect. > > mp4info version -r > testo.mov: > ReadChildAtoms: "testo.mov": In avc1 atom, extra 4 bytes at end of atom > Track Type Info > 1 video H264 Unknown Profile f4@1.3, 68.533 secs, 1500 kbps, 320x240 @ > 30.000146 fps > ReadChildAtoms: "testo.mov": In avc1 atom, extra 4 bytes at end of atom Those extra 4 bytes are actually intentional AFAIK. Some players out there expect that extra stuff. It is harmless. > > - My question in my previous post was more to hack/fix the problem in the > recovery stage rather than in the encoding stage. In the atomsrecovery.c > file, where is the logic that (could/would/should) compare the size of the > mdat in the broken file and the size of the headers in the mrf file? At the end of moov_recov_write_file, in atomsrecovery.c there is a piece of code for reading the mdat atom and writing it to the output. It just reads chunks from 1 file and writes it to the other file. Look for FOURCC_mdat and that should be the place. It writes the size of the mdat, then the mdat identifier and then reads the data from the file into the output. It doesn't check for the size of the mdat it intends to write and adds the rest of the file at the end. This will appear as bogus data to the players.
Created attachment 355202 [details] [review] atomsrecovery: read from mdat only what is on headers Something like this should work. I didn't test this much, yet. Let me know if it works for you. Should do some more testing on the week.
Just a very quick test: yes, it works. The file is clean from the mp4info point of view with the example provided above. I need to test more next week and understand the implications. It would still be better to find the problem during the encoding phase. Anyway, definitely many thanks for this second patch!
So, the two patches are working well for me. I still think that the second patch would make sense to be committed because the point of this plugin is to recover and if for any reason the sizes don't match, it still needs to be recovered properly. That being said, I'm trying to investigate the original problem of size mismatch. I have noticed one thing: if there is audio, the bitrate of the audio portion is 0kbps. videoonly-repaired.mp4: Track Type Info 1 video H264 Baseline@4.2, 3.213 secs, 9978 kbps, 1920x1440 @ 31.123561 fps audio_video-repaired.mp4: Track Type Info 1 video H264 Baseline@4.2, 2.890 secs, 9740 kbps, 1920x1440 @ 30.103806 fps 2 audio MPEG-1 Audio (11172-3), 3.108 secs, 0 kbps, 44100 Hz Please download the files here: http://gentil.com/tmp/qtmoovrecover3.zip My point is that the size mismatch problem is perhaps due to audio. Can you test with audio too?
commit 923b83a48cce999abd10f869dd721880aee19b0f Author: Thiago Santos <thiagossantos@gmail.com> Date: Sat Jul 8 22:11:49 2017 -0700 atomsrecovery: read from mdat only what is on headers It is possible that the mdat has more data than what was stored in the headers file. If we put that to the output the file will have bogus data at the end and some players will complain. https://bugzilla.gnome.org/show_bug.cgi?id=784258 commit 69605b6c61bde5b6b1bfdefaf7ca66802b0c5158 Author: Thiago Santos <thiagossantos@gmail.com> Date: Wed Jul 5 22:23:21 2017 -0700 isomp4: atomsrecovery: handle common and large atom headers Do not assume all files are large files. Check and use the short or extended atom size field only if needed. https://bugzilla.gnome.org/show_bug.cgi?id=784258