Re: [FFmpeg-devel] [PATCHv2] movenc: Write durations based on pts into mvhd/mdhd/tkhd/elst

2020-01-09 Thread Michael Niedermayer
On Thu, Jan 09, 2020 at 11:32:03PM +0200, Martin Storsjö wrote:
> On Wed, 8 Jan 2020, Martin Storsjö wrote:
> 
> >On Fri, 20 Dec 2019, Michael Niedermayer wrote:
> >
> >>On Tue, Dec 17, 2019 at 03:15:09PM +0200, Martin Storsjö wrote:
> >>>Keep all the existing data fields as they are (there's lots and
> >>>lots of nontrivial calculation and heuristics based on them in
> >>>their current form), but derive the duration as the difference
> >>>between the pts of the first packet to the maximum pts+duration
> >>>(not necessarily the last packet); use this duration in any box
> >>>where the actual presentation duration is supposed to be.
> >>>
> >>>Fixes: 8420
> >>>---
> >>>Fixed to fetch the duration for tmcd tracks from their designated
> >>>source track.
> >>>---
> >>> libavformat/movenc.c | 35 ---
> >>> 1 file changed, 28 insertions(+), 7 deletions(-)
> >>
> >>I found another case that changes, again dont know which is more correct
> >>
> >>make -j12 && ./ffmpeg -i ~/tickets/3453/mov_with_tmcd.mov -y  -bitexact
> >-codec copy -map 0 -t 2 file.mov ; ./ffprobe -v 0 file.mov -show_packets
> >-print_format compact > /tmp/before
> >>
> >>--- /tmp/before 2019-12-20 23:28:04.009327038 +0100
> >>+++ /tmp/after  2019-12-20 23:27:17.213326052 +0100
> >>@@ -188,7 +188,7 @@
> >>
> >packet|codec_type=audio|stream_index=1|pts=88320|pts_time=1.84|dts=88320|dts_time=1.84|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=408905|flags=K_
> >>
> >packet|codec_type=data|stream_index=2|pts=46|pts_time=1.84|dts=46|dts_time=1.84|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=413001|flags=K_
> >>
> >packet|codec_type=audio|stream_index=1|pts=89344|pts_time=1.861333|dts=89344|dts_time=1.861333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=413005|flags=K_
> >>
> >-packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=_D
> >>
> >+packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=__
> >>
> >packet|codec_type=audio|stream_index=1|pts=90240|pts_time=1.88|dts=90240|dts_time=1.88|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=417365|flags=K_
> >>
> >packet|codec_type=data|stream_index=2|pts=47|pts_time=1.88|dts=47|dts_time=1.88|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=421461|flags=K_
> >>
> >packet|codec_type=audio|stream_index=1|pts=91264|pts_time=1.901333|dts=91264|dts_time=1.901333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=421465|flags=K_
> >
> >I would say the new behaviour is more correct here.
> 
> So, if there's no new remarks to this patch, I'd like to push e.g. tomorrow.

ok with me

Thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCHv2] movenc: Write durations based on pts into mvhd/mdhd/tkhd/elst

2020-01-09 Thread Martin Storsjö

On Wed, 8 Jan 2020, Martin Storsjö wrote:


On Fri, 20 Dec 2019, Michael Niedermayer wrote:


On Tue, Dec 17, 2019 at 03:15:09PM +0200, Martin Storsjö wrote:

Keep all the existing data fields as they are (there's lots and
lots of nontrivial calculation and heuristics based on them in
their current form), but derive the duration as the difference
between the pts of the first packet to the maximum pts+duration
(not necessarily the last packet); use this duration in any box
where the actual presentation duration is supposed to be.

Fixes: 8420
---
Fixed to fetch the duration for tmcd tracks from their designated
source track.
---
 libavformat/movenc.c | 35 ---
 1 file changed, 28 insertions(+), 7 deletions(-)


I found another case that changes, again dont know which is more correct

make -j12 && ./ffmpeg -i ~/tickets/3453/mov_with_tmcd.mov -y  -bitexact 
-codec copy -map 0 -t 2 file.mov ; ./ffprobe -v 0 file.mov -show_packets 
-print_format compact > /tmp/before


--- /tmp/before 2019-12-20 23:28:04.009327038 +0100
+++ /tmp/after  2019-12-20 23:27:17.213326052 +0100
@@ -188,7 +188,7 @@


packet|codec_type=audio|stream_index=1|pts=88320|pts_time=1.84|dts=88320|dts_time=1.84|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=408905|flags=K_



packet|codec_type=data|stream_index=2|pts=46|pts_time=1.84|dts=46|dts_time=1.84|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=413001|flags=K_



packet|codec_type=audio|stream_index=1|pts=89344|pts_time=1.861333|dts=89344|dts_time=1.861333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=413005|flags=K_



-packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=_D



+packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=__



packet|codec_type=audio|stream_index=1|pts=90240|pts_time=1.88|dts=90240|dts_time=1.88|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=417365|flags=K_



packet|codec_type=data|stream_index=2|pts=47|pts_time=1.88|dts=47|dts_time=1.88|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=421461|flags=K_



packet|codec_type=audio|stream_index=1|pts=91264|pts_time=1.901333|dts=91264|dts_time=1.901333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=421465|flags=K_

I would say the new behaviour is more correct here.


So, if there's no new remarks to this patch, I'd like to push e.g. 
tomorrow.


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCHv2] movenc: Write durations based on pts into mvhd/mdhd/tkhd/elst

2020-01-08 Thread Martin Storsjö

On Fri, 20 Dec 2019, Michael Niedermayer wrote:


On Tue, Dec 17, 2019 at 03:15:09PM +0200, Martin Storsjö wrote:

Keep all the existing data fields as they are (there's lots and
lots of nontrivial calculation and heuristics based on them in
their current form), but derive the duration as the difference
between the pts of the first packet to the maximum pts+duration
(not necessarily the last packet); use this duration in any box
where the actual presentation duration is supposed to be.

Fixes: 8420
---
Fixed to fetch the duration for tmcd tracks from their designated
source track.
---
 libavformat/movenc.c | 35 ---
 1 file changed, 28 insertions(+), 7 deletions(-)


I found another case that changes, again dont know which is more correct

make -j12 && ./ffmpeg -i ~/tickets/3453/mov_with_tmcd.mov -y  -bitexact -codec 
copy -map 0 -t 2 file.mov ; ./ffprobe -v 0 file.mov -show_packets -print_format compact 
> /tmp/before

--- /tmp/before 2019-12-20 23:28:04.009327038 +0100
+++ /tmp/after  2019-12-20 23:27:17.213326052 +0100
@@ -188,7 +188,7 @@
packet|codec_type=audio|stream_index=1|pts=88320|pts_time=1.84|dts=88320|dts_time=1.84|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=408905|flags=K_
packet|codec_type=data|stream_index=2|pts=46|pts_time=1.84|dts=46|dts_time=1.84|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=413001|flags=K_
packet|codec_type=audio|stream_index=1|pts=89344|pts_time=1.861333|dts=89344|dts_time=1.861333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=413005|flags=K_
-packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=_D
+packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=__
packet|codec_type=audio|stream_index=1|pts=90240|pts_time=1.88|dts=90240|dts_time=1.88|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=417365|flags=K_
packet|codec_type=data|stream_index=2|pts=47|pts_time=1.88|dts=47|dts_time=1.88|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=421461|flags=K_
packet|codec_type=audio|stream_index=1|pts=91264|pts_time=1.901333|dts=91264|dts_time=1.901333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=421465|flags=K_


I would say the new behaviour is more correct here.

This command writes a truncated sequence; the sequence of muxed packets 
is this:


pts=0|pts_time=0.00|dts=-1024|dts_time=-0.08|duration=512|duration_time=0.04
...
pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04
pts=25600|pts_time=2.00|dts=24576|dts_time=1.92|duration=512|duration_time=0.04
pts=25088|pts_time=1.96|dts=25088|dts_time=1.96|duration=512|duration_time=0.04

Based on DTS, this previously wrote a total duration of 1.96 + 0.04 - 
(-0.08) = 2.08.


This duration also was set in the edit list, which then made the last 
frame (in presentation order) with a pts of 2.08 to be outside of the edit 
list, and thus be marked discardable.


With the patch, the duration of the file gets written as 2.08 + 0.04 - 
0.00 = 2.12, and thus the last frame no longer is marked as discardable.



If the reordering sequene wasn't truncated, if the following packet also 
would be included, the duration would end up calculated the same with both 
DTS and PTS:


pts=26112|pts_time=2.04|dts=25600|dts_time=2.00|duration=512|duration_time=0.04


// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCHv2] movenc: Write durations based on pts into mvhd/mdhd/tkhd/elst

2019-12-20 Thread Michael Niedermayer
On Tue, Dec 17, 2019 at 03:15:09PM +0200, Martin Storsjö wrote:
> Keep all the existing data fields as they are (there's lots and
> lots of nontrivial calculation and heuristics based on them in
> their current form), but derive the duration as the difference
> between the pts of the first packet to the maximum pts+duration
> (not necessarily the last packet); use this duration in any box
> where the actual presentation duration is supposed to be.
> 
> Fixes: 8420
> ---
> Fixed to fetch the duration for tmcd tracks from their designated
> source track.
> ---
>  libavformat/movenc.c | 35 ---
>  1 file changed, 28 insertions(+), 7 deletions(-)

I found another case that changes, again dont know which is more correct

make -j12 && ./ffmpeg -i ~/tickets/3453/mov_with_tmcd.mov -y  -bitexact -codec 
copy -map 0 -t 2 file.mov ; ./ffprobe -v 0 file.mov -show_packets -print_format 
compact > /tmp/before

--- /tmp/before 2019-12-20 23:28:04.009327038 +0100
+++ /tmp/after  2019-12-20 23:27:17.213326052 +0100
@@ -188,7 +188,7 @@
 
packet|codec_type=audio|stream_index=1|pts=88320|pts_time=1.84|dts=88320|dts_time=1.84|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=408905|flags=K_
 
packet|codec_type=data|stream_index=2|pts=46|pts_time=1.84|dts=46|dts_time=1.84|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=413001|flags=K_
 
packet|codec_type=audio|stream_index=1|pts=89344|pts_time=1.861333|dts=89344|dts_time=1.861333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=413005|flags=K_
-packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=_D
+packet|codec_type=video|stream_index=0|pts=26624|pts_time=2.08|dts=24064|dts_time=1.88|duration=512|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=776|pos=416589|flags=__
 
packet|codec_type=audio|stream_index=1|pts=90240|pts_time=1.88|dts=90240|dts_time=1.88|duration=1024|duration_time=0.021333|convergence_duration=N/A|convergence_duration_time=N/A|size=4096|pos=417365|flags=K_
 
packet|codec_type=data|stream_index=2|pts=47|pts_time=1.88|dts=47|dts_time=1.88|duration=1|duration_time=0.04|convergence_duration=N/A|convergence_duration_time=N/A|size=4|pos=421461|flags=K_
 
packet|codec_type=audio|stream_index=1|pts=91264|pts_time=1.901333|dts=91264|dts_time=1.901333|duration=896|duration_time=0.018667|convergence_duration=N/A|convergence_duration_time=N/A|size=3584|pos=421465|flags=K_


file should be here:
https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket3453/


[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Awnsering whenever a program halts or runs forever is
On a turing machine, in general impossible (turings halting problem).
On any real computer, always possible as a real computer has a finite number
of states N, and will either halt in less than N cycles or never halt.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCHv2] movenc: Write durations based on pts into mvhd/mdhd/tkhd/elst

2019-12-17 Thread Martin Storsjö
Keep all the existing data fields as they are (there's lots and
lots of nontrivial calculation and heuristics based on them in
their current form), but derive the duration as the difference
between the pts of the first packet to the maximum pts+duration
(not necessarily the last packet); use this duration in any box
where the actual presentation duration is supposed to be.

Fixes: 8420
---
Fixed to fetch the duration for tmcd tracks from their designated
source track.
---
 libavformat/movenc.c | 35 ---
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index dd144ae20a..7d2d3bc20e 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2760,10 +2760,28 @@ static int mov_write_minf_tag(AVFormatContext *s, 
AVIOContext *pb, MOVMuxContext
 return update_size(pb, pos);
 }
 
+static int64_t calc_pts_duration(MOVMuxContext *mov, MOVTrack *track)
+{
+if (track->tag == MKTAG('t','m','c','d')) {
+// tmcd tracks gets track_duration set in mov_write_moov_tag from
+// another track's duration, while the end_pts may be left at zero.
+// Calculate the pts duration for that track instead.
+return av_rescale(calc_pts_duration(mov, 
>tracks[track->src_track]),
+  track->timescale, 
mov->tracks[track->src_track].timescale);
+}
+if (track->end_pts != AV_NOPTS_VALUE &&
+track->start_dts != AV_NOPTS_VALUE &&
+track->start_cts != AV_NOPTS_VALUE) {
+return track->end_pts - (track->start_dts + track->start_cts);
+}
+return track->track_duration;
+}
+
 static int mov_write_mdhd_tag(AVIOContext *pb, MOVMuxContext *mov,
   MOVTrack *track)
 {
-int version = track->track_duration < INT32_MAX ? 0 : 1;
+int64_t duration = calc_pts_duration(mov, track);
+int version = duration < INT32_MAX ? 0 : 1;
 
 if (track->mode == MODE_ISM)
 version = 1;
@@ -2785,7 +2803,7 @@ static int mov_write_mdhd_tag(AVIOContext *pb, 
MOVMuxContext *mov,
 else if (!track->entry)
 (version == 1) ? avio_wb64(pb, 0) : avio_wb32(pb, 0);
 else
-(version == 1) ? avio_wb64(pb, track->track_duration) : avio_wb32(pb, 
track->track_duration); /* duration */
+(version == 1) ? avio_wb64(pb, duration) : avio_wb32(pb, duration); /* 
duration */
 avio_wb16(pb, track->language); /* language */
 avio_wb16(pb, 0); /* reserved (quality) */
 
@@ -2835,8 +2853,9 @@ static void write_matrix(AVIOContext *pb, int16_t a, 
int16_t b, int16_t c,
 static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
   MOVTrack *track, AVStream *st)
 {
-int64_t duration = av_rescale_rnd(track->track_duration, MOV_TIMESCALE,
-  track->timescale, AV_ROUND_UP);
+int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
+  MOV_TIMESCALE, track->timescale,
+  AV_ROUND_UP);
 int version = duration < INT32_MAX ? 0 : 1;
 int flags   = MOV_TKHD_FLAG_IN_MOVIE;
 int rotation = 0;
@@ -2982,8 +3001,9 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack 
*track)
 static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
   MOVTrack *track)
 {
-int64_t duration = av_rescale_rnd(track->track_duration, MOV_TIMESCALE,
-  track->timescale, AV_ROUND_UP);
+int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
+  MOV_TIMESCALE, track->timescale,
+  AV_ROUND_UP);
 int version = duration < INT32_MAX ? 0 : 1;
 int entry_size, entry_count, size;
 int64_t delay, start_ct = track->start_cts;
@@ -3269,7 +3289,8 @@ static int mov_write_mvhd_tag(AVIOContext *pb, 
MOVMuxContext *mov)
 
 for (i = 0; i < mov->nb_streams; i++) {
 if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) {
-int64_t max_track_len_temp = 
av_rescale_rnd(mov->tracks[i].track_duration,
+int64_t max_track_len_temp = av_rescale_rnd(
+calc_pts_duration(mov, 
>tracks[i]),
 MOV_TIMESCALE,
 mov->tracks[i].timescale,
 AV_ROUND_UP);
-- 
2.21.0 (Apple Git-122.2)

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".