Vanilagy / mp4-muxer

MP4 multiplexer in pure TypeScript with support for WebCodecs API, video & audio.
https://vanilagy.github.io/mp4-muxer/demo
MIT License
419 stars 32 forks source link

Faulty metadata when using fastStart = false #59

Closed pedrobroese closed 2 months ago

pedrobroese commented 2 months ago

Vanilagy, first of all thank you very much for putting together this great library, works as a charm for my project. In this project, one can edit a mp4 video in the browser animating gauges over the screen and then generating a new mp4 file, including the original sound. You can reproduce the issue I'll describe next anytime by feeding any mp4 file (max resolution 1920 x 1080) to my app at

https://easytelemetryreact.onrender.com/

The issue I am facing is, as the title describes, when I set the option fastStart to false (which is the best option to optimize memory usage in my case), I get a mismatch between the track timeScale and the movie timeSacale, as well as zero duration for the movie. This values in the mdat all together cause any mp4 player I've tryed to fail to seek the file and even to display the time elapsed bar properly. To get to the generated metada I first exported a new mp4 file on my app (click the red button in the control bar), and then fed the generated file back to the app, where it extracts the mdata with MP4Box and console.dir the result:

Outputmetadata

I underlined in red what appears to me to be the bad values. As you can see in the mdat below, from a regular file, timescale for movie and track match and you also get duration > 0.

GoPro metadata

The configurations I am using for the muxer are as following:

   //setup Muxer with video and audio parameters from original file Mdat
      muxer1 = new Muxer({
        // muxer1 = new WebMMuxer.Muxer({
        target: muxerTarget,
        video: {
          codec: 'avc',
          width: outputW,
          height: outputH,
          frameRate: FPS,
          firstTimestampBehavior: 'cross-track-offset'
        },
        audio: {
          codec: 'aac',
          numberOfChannels: 2,
          sampleRate: audioTrack.audio.sample_rate //adjust as per original
        },
        fastStart: false
      });
Vanilagy commented 2 months ago

Hi, thanks for the issue!

I don't think this is a problem with the timescales. The movie's timescale being different from a track's timescale is no issue, it's simply a matter of defining units. Each track can have a different timescale which best suits the sample timestamps.

I'd be surprised if files muxed with fastStart = false didn't play back, since that's the default usecase for this library and it has been tested by many people. Can you share with me your muxerTarget, as well as send an example file that is broken?

Vanilagy commented 2 months ago

When I create a video using this library's demo, but modified so that fastStart = false, the output video works fine. It plays back and the duration is also set correctly: CleanShot 2024-08-18 at 15 32 01@2x

Could it be that you're passing incorrect timestamps to the muxer? Remember, they have to be in microseconds.

pedrobroese commented 2 months ago

Thank you very much for your prompt response. Just for clarification, since you said "I'd be surprised if files muxed with fastStart = false didn't play back" I would like to make clear that the file does playback perfectly, in synch with the audio which is not even decoded (used addAudioChunkRaw) . The issue I am facing is that the resulting file is not seekable in any player, probably because of the duration = 0 in the mdat.

Regarding the target, the code is very similar to the example you provided:

import { Muxer, FileSystemWritableFileStreamTarget, ArrayBufferTarget } from 'mp4-muxer'

fileHandle = await window.showSaveFilePicker({
        startIn: 'videos',
        suggestedName: 'newSession.mp4', //--> Pass name with date and hour
        types: [{
          description: 'Video File',
          accept: { 'video/mp4': ['.mp4'] },
        }],
      });
      fileWritableStream = await fileHandle.createWritable();

      muxerTarget = new FileSystemWritableFileStreamTarget(fileWritableStream)

And finally, about the encodedChunk timestamp, just double checked and it is in micro seconds as required (if it wasn't I wouldn't get a file with video and audio in synch anyway). I also tryed to feed the timestamp again as a separate argument to the muxer, but got the same result...

 videoEncoder = new VideoEncoder({
        output: (encodedChunk, meta) => {
          let timeStamp = encodedChunk.timestamp
          muxer1.addVideoChunk(encodedChunk, meta, timeStamp);
...

And finally find below a short example of a video generated by my app. Curiosly, in this embedded player it is seeakable, however, I've tried all windows media player and VLC player and all of them present the issue as I described:

https://github.com/user-attachments/assets/ccbab8d3-1605-40c3-b787-cb9ef95f806d

Vanilagy commented 2 months ago

Thanks, I see that the metadata is indeed strange, even though I can't explain how this could happen. Just to rule out target issues, can you render the video into an ArrayBufferTarget instead and then download that? Using something like:

const downloadBlob = (blob) => {
    let url = window.URL.createObjectURL(blob);
    let a = document.createElement('a');
    a.style.display = 'none';
    a.href = url;
    a.download = 'davinci.mp4';
    document.body.appendChild(a);
    a.click();
    window.URL.revokeObjectURL(url);
};

Because then I can know if this is a muxer logic issue or something's wrong with the target/writing process.

pedrobroese commented 2 months ago

Thanks, I see that the metadata is indeed strange, even though I can't explain how this could happen. Just to rule out target issues, can you render the video into an ArrayBufferTarget instead and then download that? Using something like:

const downloadBlob = (blob) => {
  let url = window.URL.createObjectURL(blob);
  let a = document.createElement('a');
  a.style.display = 'none';
  a.href = url;
  a.download = 'davinci.mp4';
  document.body.appendChild(a);
  a.click();
  window.URL.revokeObjectURL(url);
};

Because then I can know if this is a muxer logic issue or something's wrong with the target/writing process.

As requested, below a video generated by muxing to ArrayBufferTarget... same result.

https://github.com/user-attachments/assets/6c3b1b6c-4805-40a5-aca1-234d0a5c2567

As you can see in the mdat, still get zero duration... If you can point out for me which functions/lines to look into I'll be happy to help in the debugging

image

pedrobroese commented 2 months ago

I did some further investigation and I believe I was able to get closer to the problem source: When the encoded frames are fed to the muxer, a sample interface is created in the muxer. For some reason, the sample duration is being recorded as NaN. So, when the mdat is prepared here:

image

samples.duration = NaN results in duration =0. I double checked the args I used to call the VideoFrame constructor and also checked the resulting EncodedChunk and both timestamp and duration are correct in the EncodedChunks that are fed to the muxer

image

pedrobroese commented 2 months ago

Ok, figured out the problem. I was adding the audio to the muxer by using addAudioChunkRaw and I was not passing the duration ...

image

Changed code to

 let duration = audioSample.duration / audioSample.timescale * 1e6

muxer1.addAudioChunkRaw(audioSample.data, type, correctedAudioCTS, duration)

correctedAudioCTS += duration;

and now everything works as charm. At first, I didn't suspect that missing data in the audiotrack would affect movie duration mdat, but since the code calculates duration by looping through all the tracks to find which is the longest, any missing duration in any track would cause this same problem without any errors..

As a suggestion, since duration is a mandatory arg, for the addAudioChunkRaw, maybe it should return an error if the argument is not passed.

Vanilagy commented 2 months ago

@pedrobroese Hah, it's funny for me, this issue just solved itself 😂 I just opened it, got a little drink, and when I came back it was already closed. Thanks for doing the investigation!

Yeah, perhaps I should do some runtime validation! I'm generally a huge fan of runtime validation for libraries as it prevents issues like these, where libraries internals encounter errors due to faulty inputs in a way that is non-obvious to a user. Good suggestion, I think I'll add that!

Another question, though: When I investigated the video you sent, I noticed strange datetime values here in the mvhd box: CleanShot 2024-08-18 at 23 14 08@2x

This is strange, since these two values are set to be identical by the writer: CleanShot 2024-08-18 at 23 14 44@2x

Would you mind checking if the fixed video still exhibits these weird datetimes? I use this tool here: https://www.onlinemp4parser.com/

pedrobroese commented 2 months ago

@Vanilagy, there is an old joke in my company that says that there are 2 kinds of problems:

Type 1: Is my problem Type 2: Is not my problem

Recently, I added to the list the type 3: A non-problem.

This issue was definitely a type 3 problem, and I apologize for bothering you with it. Glad that, at least, it served to bring to your attention this issue with de dates. And yes, this issue persists even after I fixed the code. Below a sample video from after I fixed the code, so you can check all the mdat.

https://github.com/user-attachments/assets/8821a7b6-301a-41f6-8829-391442ead116

pedrobroese commented 2 months ago

Maybe this problem with the dates is something related to the parsing of the mdat by the online MP4 parser. I just parsed the mdat with MP4Box and got the dates correctly:

image

Vanilagy commented 2 months ago

You are correct! I've verified it by testing it with another tool, https://gpac.github.io/mp4box.js/test/filereader.html. Then everything's fine as expected!

Thanks for the cooperation, you didn't waste my time at all :)

Vanilagy commented 2 months ago

I just released a new version (v5.0.0) that adds strict argument validation everywhere, try upgrading to it. The reason I bumped the major version is because this new level of strictness will surely cause somebody's current code to fail.