MP4 Mux and Demux Bugs #37
Comments
|
The case of 12 samples is only for options fastOpen has set. By default, the analysis will be done up to maxAnalyzeDuration, but there is a bug here. Currently, maxAnalyzeDuration is set to 10 seconds, but the video is only 6 seconds long, which causes the bitrate not to be recalculated when exiting the analyzeStreams function, thus using the calculated value of 12 samples. I will fix to recalculate bitrate when io end appears in analyzeStreams later. In addition, I also agree that pts should be used for calculation here. duration I used the The mkv has a frame count of 602, I don't understand this error. Through testing, the file in test_muxing.zip does have only 600 frames of video. |
|
I'm glad we found the bug, thank you for fixing it. I checked and it did calculate bitrate more accurately for that video. Also, we can ignore
As that's a bad statement on my part and caused by me not understanding mediainfo.js vs checking frames individually. That said, I should back up and restate the issue I am concerned about. A key use of libmedia for me is to transcode and transmux files. A common use case will be that a user wants to upload a video to a website (example, tiktok) which has a 10 minute duration limit for files. Let's say the original file is 10 minutes, but is MKV and TikTok will not accept MKV but requires MP4. Since the original file is 10 minutes exactly the user would expect it meets TikTok's requirements for duration but needs to transmux the file to MP4. If when I transmux the file for them so that it meets requirements then it is a problem if the duration is now 10 minutes and 0.033 seconds, because the file will be rejected for not meeting the duration requirement. Another example would be the user has a 10 minute MP4 but the codec is not acceptable to TikTok (AV1 or mpeg-2) and the user needs me to transcode the file for them then when I transcode the new file cannot increase in duration. I hope I explained that well. What this means is that a transcoded or transmuxed file that has a longer than expected duration will be rejected and my service fails to help the user. This means that to me, the file being longer than the original is a problem, but if it's shorter than the original it's less so because if a transcoded or transmuxed file is shorter than expected, either the AV sync was improved (a good thing) or some frames at the beginning or end of the video were dropped (a bad thing but not probably not noticeable to the user, and also does not block uploading the video). So with that understanding, let me address the items individually.
My original question
was bad. I misunderstood how that worked and asked the question badly. But I did notice something kind of weird, and possibly unintended, while working through this. I did this with the transcoder demo: (File 1) original MP4 file -> (File 2) mkv file, copying both streams
and then this and I got two different MKV files that have different durations. Where this gets weird is when I transmux both those MKV files back to MP4, copying audio and video streams (File 2) -> (File 4)
and (File 3) -> (File 5)
Here's the summary: Both File 4 and File 5 have their first frame with a negative dts, which is why for MP4 I think duration needs to be calculated with pts and not dts. I think this is how libavformat/mov.c in ffmpeg handles it: We're beyond my knowledge at this point, so I'm mostly sharing this detail in case it helps. I think the expectation, and please let me know if I'm wrong, is that duration should not change when transcoding or transmuxing a file unless a different duration is specified, and if that's not the expectation then I need to adjust accordingly so my users don't get uploads rejected. Thank you! Supporting code and files below: The command I used to check frame data: and the relevant output files: bbb_input_AllFrames.txt and bbb_input_ffprobe.csv |
|
This might be helpful. If using ctts to calculate pts before calculating duration for MP4, it's possible that the MP4 file is missing the CTTS atom. This was the mkvMerge implementation to calculate pts when ctts is missing: https://gitlab.com/mbunkus/mkvtoolnix/-/commit/1813e97cbf8f3691c55207f6c72d046ec00a2381 Would also help AVPlayer handle damaged MP4 files without dropping frames. Edit: I'm working on building libmedia locally so I can test and contribute better, but am currently having some issues with using build-package.js to build cheap. Will post a follow up once it's clearer but right now it can't find the wat2wasm files, and also is calling import.meta for the CJS package. |
|
There are some errors in libmedia's duration calculation of tracks. I have fixed the calculation of video. But there are still some questions about the calculation of audio. As shown in the test video file above, we can see that the audio has 469 audio frames with 1024 samples and 1 audio frame with 768 samples, with total of 469 * 1024 + 768 = 481024 samples. The samplerate is 48000, so the calculated duration = 481024 / 48000 = 10.0213 seconds, but why is the calculated duration 10 seconds here? The duration in the mdhd box in the file which using FFmpeg remux is also 10 seconds. There may be some details I missed here, I need more time to determine the details. |
|
I understand the details here now. Duration cannot only be determined by the maximum pts, but also needs to consider the start time in the elst box. As shown in the test file above, the start time of the audio track in the elst box is 1024, so the duration of the audio needs to be subtracted by 1024 to equal 10 seconds. Now I have fixed it, the duration of the mp4 track needs to remove the part where the pts is a negative number. |
|
Brilliant, thank you. I sent a sponsorship via paypal. I have a couple questions on building libmedia so I can contribute better and also verify without waiting for https://zhaohappy.github.io/libmedia/test/avtranscoder.html to be updated. Do you build with npm or pnpm? I ask because npm doesn't support the "workspace:*" syntax in package.json. Or if it does, I cannot figure out how to make it work. For example in /avtranscoder/package.json I had to change the dependencies to look like this to run with npm I also had to put: into the main package.json file in libmedia/ If you are building with pnpm I can build that way as well to eliminate any potential variations. If you can share the .npmrc file (remove user names, passwords, and tokens if they are in there) then I'll mimic exactly. Also, when I built it using "node build-package.js --package=all" it put all the types files (and only the types files) into /dist/src/* for each package. It also created a /dist/ in each /src/* folder with all the compiled files, including the types files. Is that the correct and intended behavior? Finally, when using the packages, such as @libmedia/avtranscoder, in a separate project I'll be using the es6 modules to import into my Svelte/Sveltekit project that uses Vite for dev and vite build for prod. The avtranscoder library is only used in the browser, so it does not get server side rendered or run in nodejs. When I read https://github.com/daniellovera/libmedia/blob/master/site/docs/guide/quick-start.en-US.md and https://github.com/daniellovera/libmedia/blob/master/site/docs/guide/threads.en-US.md I was not understanding if I need to use the transformer with Vite. If I build @libmedia/avtranscoder the same way as you do and then import the package into my main project, does my main project need to use the transformer on the package again? |
|
Currently, the package.json in the subdirectory under the src directory is only used to publish npm packages. The package.json and webpack.config.js in the root directory are used for compilation and testing. The following is the steps of publishing npm packages now. If you need to publish to your private npm repository, you can refer to it:
The use of transformer depends on whether you will access pointer type data. If you only use @libmedia/avtranscoder, you don’t need it, but the AVCodecID constant enum is required in the getWasm callback. The esbuild in vite does not support typescript's constant enum. Here you can switch to tsc to compile or use the enum value directly. If you use the API of other packages, you usually need to access pointer type data, then you need to use transformer. Finally, thank you for your sponsorship. This is my first sponsorship :) |
|
First of many, no doubt. That makes sense about using the transformer. I'll simply use the enum value directly, saves complicating my build step. I found two other small items as part of replicating the build process that I'll send PRs for: In build-package.js, the buildAll command is missing avutil I checked that build process above and it will work, thank you. |
Hi,
I was testing https://zhaohappy.github.io/libmedia/test/avtranscoder.html with the attached file (bbb_input.mp4) and I noticed a couple bugs. The bugs change the duration, frame rate, and quality so I am sharing them here.
I used these settings:
on this file
https://github.com/user-attachments/assets/a2bd89be-13cf-430d-a6fb-36f6164d59d9
got this log
Created this file (I had to zip the mkv to attach):
test_muxing.zip
Issues:
MP4 can have negative DTS, and may need more than 12 samples due to how MP4 interleaves data. I think these are the issues, if it's helpful.
libmedia/src/avformat/demux.ts
Lines 222 to 230 in e8599c7
Should that be
And for MP4, should use PTS instead of DTS for duration calculation:
so
libmedia/src/avformat/demux.ts
Line 317 in e8599c7
&
libmedia/src/avformat/demux.ts
Line 350 in e8599c7
I think should be
const duration = Number(avpacket.pts - stream.startTime) * stream.timeBase.num / stream.timeBase.denI am still working on building libmedia myself so I am unable to test these just yet and wanted to share.
Thank you, I appreciate libmedia and it is great.
The text was updated successfully, but these errors were encountered: