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.

Sign up for GitHub

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

Jump to bottom

mp4 edts media time info is incorrect #1441

Open
quink-black opened this issue Oct 13, 2021 · 10 comments · May be fixed by #1442
Open

mp4 edts media time info is incorrect #1441

quink-black opened this issue Oct 13, 2021 · 10 comments · May be fixed by #1442

Comments

@quink-black
Copy link

  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
Copy link
Author

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 added a commit to quink-black/MediaInfoLib that referenced this issue Oct 19, 2021
@quink-black
Fix mp4 edts trace info
b998ecf 
1. Media time is signed value
2. Media time is in unit of mdhd time scale. Since mdhd comes after
edts, we don't know mdhd_TimeScale yet when handle edts. Give a hint
to user on which time scale to use.

Fix MediaArea#1441
@quink-black quink-black linked a pull request Oct 19, 2021 that will close this issue
Open
quink-black added a commit to quink-black/MediaInfoLib that referenced this issue Oct 20, 2021
@quink-black
Fix mp4 edts trace info
56bc23f 
1. Media time is signed value
2. Media time is in unit of mdhd time scale. Since mdhd comes after
edts, we don't know mdhd_TimeScale yet when handle edts. Give a hint
to user on which time scale to use.

Fix MediaArea#1441
JeromeMartinez pushed a commit to JeromeMartinez/MediaInfoLib that referenced this issue Oct 27, 2021
@quink-black @JeromeMartinez
Fix mp4 edts trace info
69f8ad3 
1. Media time is signed value
2. Media time is in unit of mdhd time scale. Since mdhd comes after
edts, we don't know mdhd_TimeScale yet when handle edts. Give a hint
to user on which time scale to use.

Fix MediaArea#1441
@JeromeMartinez
Copy link
Member

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... :(.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix mp4 edts trace info quink-black/MediaInfoLib
3 participants
@JeromeMartinez @quink-black and others