MediaInfoLib icon indicating copy to clipboard operation
MediaInfoLib copied to clipboard

mp4 edts media time info is incorrect

Open quink-black opened this issue 3 years ago • 10 comments

  1. media_time is a signed value, either int(64) media_time or int(32) media_time. There is a special value -1. MediaInfoLib handled it as unsigned:
name="Media time">4294967295
  1. The unit of media_time and segment_duration is different:

segment_duration is an integer that specifies the duration of this edit segment in units of the timescale in the Movie Header Box media_time is an integer containing the starting time within the media of this edit segment (in media time scale units, in composition time).

Current code use moov_mvhd_TimeScale for media_time, which should be Streams[moov_trak_tkhd_TrackID].mdhd_TimeScale. However, since mdhd comes after edts, we don't know mdhd_TimeScale yet when handle edts. The following patch doesn't work:

diff --git a/Source/MediaInfo/Multiple/File_Mpeg4_Elements.cpp b/Source/MediaInfo/Multiple/File_Mpeg4_Elements.cpp
index 89f6ae0d0..51ddf0e74 100644
--- a/Source/MediaInfo/Multiple/File_Mpeg4_Elements.cpp
+++ b/Source/MediaInfo/Multiple/File_Mpeg4_Elements.cpp
@@ -3856,7 +3856,7 @@ void File_Mpeg4::moov_trak_edts_elst()
         stream::edts_struct edts;
         Element_Begin1("Entry");
         Get_B_DEPENDOFVERSION(edts.Duration,                    "Track duration"); Param_Info2C(moov_mvhd_TimeScale, (int64u)edts.Duration*1000/moov_mvhd_TimeScale, " ms");
-        Get_B_DEPENDOFVERSION(edts.Delay,                       "Media time"); Param_Info2C(moov_mvhd_TimeScale && (edts.Delay!=(int32u)-1), (int64u)edts.Delay*1000/moov_mvhd_TimeScale, " ms");
+        Get_B_DEPENDOFVERSION(edts.Delay,                       "Media time"); Param_Info2C(Streams[moov_trak_tkhd_TrackID].mdhd_TimeScale && (edts.Delay!=(int32u)-1), (int64u)edts.Delay*1000/Streams[moov_trak_tkhd_TrackID].mdhd_TimeScale, " ms");^M
         Get_B4 (edts.Rate,                                      "Media rate"); Param_Info1(((float)edts.Rate)/0x10000);
         Element_End0();

quink-black avatar Oct 13 '21 09:10 quink-black

Yes, also editlist Media time can be in samples, not in ms. See a file genarated by: ffmpeg -f lavfi -i "sine=frequency=1000:duration=5" -c:a eac3 out.mp4 on recent ffmpeg.

Since EAC3 is lossy it has initial 256 samples of silence that must be discarded and that is what it is set to, 256. Also 512 samples are to be trimmed in the end, but ffmpeg does not support that (i.e it does not support writing media duration in eac3 case, only aac, and does not drop it using media duration in editlist even in aac case).

ValZapod avatar Oct 14 '21 09:10 ValZapod

Yes, also editlist Media time can be in samples, not in ms.

media_time is still in media time scale units, and the media time scale is sample rate in this case, so media_time equals to sample_number / sample_rate.

quink-black avatar Oct 14 '21 11:10 quink-black

There is also a problem that it cannot read sgpd and sbgp atoms that signal that decoding should happen not after 1024 samples but from 1023 then dropping 1023 then using 1023's MDCT state to decode stuff after that!! WOW. See how it should be parsed. https://sourceforge.net/p/bmxlib/bmx/merge-requests/7/

AB1F      Track duration:                    5000 (0x00001388) - 5000 (0x1388) ms
AB23      Media time:                        1024 (0x00000400) - 1024 (0x400) ms
AB27      Media rate:                        65536 (0x00010000) - 1.000
***
B00A      sgpd (26 bytes)
B00A       Header (8 bytes)
B00A        Size:                            26 (0x0000001A)
B00E        Name:                            sgpd
B012       Unknown:                          (18 bytes)
B024      sbgp (28 bytes)
B024       Header (8 bytes)
B024        Size:                            28 (0x0000001C)
B028        Name:                            sbgp
B02C       Unknown:                          (20 bytes)

https://user-images.githubusercontent.com/31514790/137624365-ac3fca67-4fe5-4655-b8e6-a9ba5f26768c.mp4

ValZapod avatar Oct 17 '21 11:10 ValZapod

media_time equals to sample_number / sample_rate.

I think it is a good idea to just remove ms whatsover then. Just parse the raw value, let people extract their own value from the other place (Streams[moov_trak_tkhd_TrackID].mdhd_TimeScale) and calculate. @JeromeMartinez

ValZapod avatar Oct 18 '21 19:10 ValZapod

I will point out that my file outaaceditlist.mp4 is invalid in one little case, because of this bug in ffmpeg: https://github.com/FFmpeg/FFmpeg/commit/f37e66b3937a914e16d89a9050f042ad89567245

mdhd atom MUST CONTAIN duration pre media time's editlist (btw, is media duration in editlist not accounted for?), not the same as post editlist in mvhd and tkhd. -c copy of the file will fix this.

ValZapod avatar Jan 29 '22 08:01 ValZapod

New sample

https://user-images.githubusercontent.com/31514790/151662537-de43900f-ecdf-4238-9ede-d5d2de671a80.mp4

and EAC3 (EAC3 is still wrong duration though in edit list and no sgpd/sbgp, though standard says roll is needed)

https://user-images.githubusercontent.com/31514790/151820288-94c8a78b-1f8a-48e7-b4ca-32de01b59c87.mp4

ValZapod avatar Jan 29 '22 13:01 ValZapod

Looks like that it would be better to support sgpd at the same time for having coherent behavior, on my todo-list but this latter is long... :(.

JeromeMartinez avatar Jan 31 '22 14:01 JeromeMartinez

support sgpd at the same time

Only roll meta is important. sgpd and sbgp are shown as unknown atoms, good enough for me.

ValZapod avatar Jan 31 '22 15:01 ValZapod

Updated eac3 sample that I duplicated by mistake with aac sample. So again, no roll metadata in eac3 that is supposed to be there per standard, and no correct media duration in editlist that should have been limited to 5 s 0 ms instead of 5 s 10 ms. Anyway, would not have applied due to a bug in ffmpeg that does not remove the remainder and does not use media duration. Alas.

ValZapod avatar Jan 31 '22 15:01 ValZapod