diff mbox series

[FFmpeg-devel,4/5] avformat/movenc: reduce tfra box size when every sample is a sync sample

Message ID tencent_1B6C68F203F56AFA7C46ACF63829C3E9B50A@qq.com
State New
Headers show
Series [FFmpeg-devel,1/5] avformat/movenc: remove unused argument from get_sample_flags() | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Zhao Zhili Dec. 3, 2021, 5:06 a.m. UTC 
---
 libavformat/movenc.c  |  8 +++++
 libavformat/movenc.h  |  1 +
 tests/ref/fate/movenc | 80 +++++++++++++++++++++----------------------
 3 files changed, 49 insertions(+), 40 deletions(-)

Comments

Zhao Zhili Dec. 3, 2021, 7:37 a.m. UTC | #1 
> On Dec 3, 2021, at 1:06 PM, Zhao Zhili <quinkblack@foxmail.com> wrote:
> 
> ---
> libavformat/movenc.c  |  8 +++++
> libavformat/movenc.h  |  1 +
> tests/ref/fate/movenc | 80 +++++++++++++++++++++----------------------
> 3 files changed, 49 insertions(+), 40 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index f8731d33c5..01dfd21a43 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4938,6 +4938,7 @@ static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
>     int number_of_entry = 0;
>     int i;
> 
> +    if (!track->every_sample_keyframe) {
>     /* We can't write and then update number_of_entry, because we cannot fix
>      * the case when number_of_entry is zero, since zero indicates that every
>      * sample is a sync sample. So get number_of_entry first.
> @@ -4948,6 +4949,7 @@ static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
>     }
>     if (!number_of_entry)
>         return 0;
> +    }
> 
>     avio_wb32(pb, 0); /* size placeholder */
>     ffio_wfourcc(pb, "tfra");
> @@ -4956,6 +4958,9 @@ static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
> 
>     avio_wb32(pb, track->track_id);
>     avio_wb32(pb, 0); /* length of traf/trun/sample num */
> +    if (track->every_sample_keyframe) {
> +    avio_wb32(pb, 0);
> +    } else {
>     avio_wb32(pb, number_of_entry);
>     for (i = 0; i < track->nb_frag_info; i++) {
>         if (!(track->frag_info[i].first_sample_flags & MOV_SYNC_SAMPLE))
> @@ -4966,6 +4971,7 @@ static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
>         avio_w8(pb, 1); /* trun number */
>         avio_w8(pb, 1); /* sample number */
>     }
> +    }

The patch is according to the spec. However, when there is a single track and every
sample is a sync sample, I think we still want to have the table entry to get moof
offset (when there is no other index method like sidx). So enable the optimization
conditionally, or just drop the optimization, any idea? 

> 
>     return update_size(pb, pos);
> }
> @@ -5999,6 +6005,7 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
>         trk->cluster[trk->entry].flags |= MOV_DISPOSABLE_SAMPLE;
>         trk->has_disposable++;
>     }
> +    trk->every_sample_keyframe &= !!(pkt->flags & AV_PKT_FLAG_KEY);
> 
>     prft = (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
>     if (prft && prft_size == sizeof(AVProducerReferenceTime))
> @@ -6769,6 +6776,7 @@ static int mov_init(AVFormatContext *s)
>         if (track->language < 0)
>             track->language = 32767;  // Unspecified Macintosh language code
>         track->mode = mov->mode;
> +        track->every_sample_keyframe = 1;
>         track->tag  = mov_find_codec_tag(s, track);
>         if (!track->tag) {
>             av_log(s, AV_LOG_ERROR, "Could not find tag for codec %s in stream #%d, "
> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> index e3a5e2864a..e78a08c2dd 100644
> --- a/libavformat/movenc.h
> +++ b/libavformat/movenc.h
> @@ -97,6 +97,7 @@ typedef struct MOVTrack {
>     long        chunkCount;
>     int         has_keyframes;
>     int         has_disposable;
> +    int         every_sample_keyframe;
> #define MOV_TRACK_CTTS         0x0001
> #define MOV_TRACK_STPS         0x0002
> #define MOV_TRACK_ENABLED      0x0004
Michael Niedermayer Dec. 3, 2021, 1:33 p.m. UTC | #2 
On Fri, Dec 03, 2021 at 01:06:56PM +0800, Zhao Zhili wrote:
> ---
>  libavformat/movenc.c  |  8 +++++
>  libavformat/movenc.h  |  1 +
>  tests/ref/fate/movenc | 80 +++++++++++++++++++++----------------------
>  3 files changed, 49 insertions(+), 40 deletions(-)

Heres a testcase this seems to break

ffmpeg -i matrixbench_mpeg2.mpg -movflags frag_keyframe -t 3 /tmp/1/file.ismv
ismindex -n /tmp/1/file /tmp/1/file.ismv
cd /tmp/1
ismindex -split -n file-split file.ismv

Before the patch there is

'Fragments(audio=0)'
in
'./QualityLevels(127986)'

afterwards the directory is empty


[...]
Zhao Zhili Dec. 3, 2021, 1:54 p.m. UTC | #3 
> On Dec 3, 2021, at 9:33 PM, Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
> On Fri, Dec 03, 2021 at 01:06:56PM +0800, Zhao Zhili wrote:
>> ---
>> libavformat/movenc.c  |  8 +++++
>> libavformat/movenc.h  |  1 +
>> tests/ref/fate/movenc | 80 +++++++++++++++++++++----------------------
>> 3 files changed, 49 insertions(+), 40 deletions(-)
> 
> Heres a testcase this seems to break
> 
> ffmpeg -i matrixbench_mpeg2.mpg -movflags frag_keyframe -t 3 /tmp/1/file.ismv
> ismindex -n /tmp/1/file /tmp/1/file.ismv
> cd /tmp/1
> ismindex -split -n file-split file.ismv
> 
> Before the patch there is
> 
> 'Fragments(audio=0)'
> in
> './QualityLevels(127986)'
> 
> afterwards the directory is empty
> 

ismindex mistakes 0 number of entry as ENOMEM.

    track->chunks  = avio_rb32(f);
    track->offsets = av_calloc(track->chunks, sizeof(*track->offsets));
    if (!track->offsets) {
        track->chunks = 0;
        ret = AVERROR(ENOMEM);
        goto fail;
    }

Now I think the feature makes more trouble than useful. Please drop patch 4
and 5.

> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Old school: Use the lowest level language in which you can solve the problem
>            conveniently.
> New school: Use the highest level language in which the latest supercomputer
>            can solve the problem without the user falling asleep waiting.
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index f8731d33c5..01dfd21a43 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -4938,6 +4938,7 @@  static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
     int number_of_entry = 0;
     int i;
 
+    if (!track->every_sample_keyframe) {
     /* We can't write and then update number_of_entry, because we cannot fix
      * the case when number_of_entry is zero, since zero indicates that every
      * sample is a sync sample. So get number_of_entry first.
@@ -4948,6 +4949,7 @@  static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
     }
     if (!number_of_entry)
         return 0;
+    }
 
     avio_wb32(pb, 0); /* size placeholder */
     ffio_wfourcc(pb, "tfra");
@@ -4956,6 +4958,9 @@  static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
 
     avio_wb32(pb, track->track_id);
     avio_wb32(pb, 0); /* length of traf/trun/sample num */
+    if (track->every_sample_keyframe) {
+    avio_wb32(pb, 0);
+    } else {
     avio_wb32(pb, number_of_entry);
     for (i = 0; i < track->nb_frag_info; i++) {
         if (!(track->frag_info[i].first_sample_flags & MOV_SYNC_SAMPLE))
@@ -4966,6 +4971,7 @@  static int mov_write_tfra_tag(AVIOContext *pb, MOVTrack *track)
         avio_w8(pb, 1); /* trun number */
         avio_w8(pb, 1); /* sample number */
     }
+    }
 
     return update_size(pb, pos);
 }
@@ -5999,6 +6005,7 @@  int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
         trk->cluster[trk->entry].flags |= MOV_DISPOSABLE_SAMPLE;
         trk->has_disposable++;
     }
+    trk->every_sample_keyframe &= !!(pkt->flags & AV_PKT_FLAG_KEY);
 
     prft = (AVProducerReferenceTime *)av_packet_get_side_data(pkt, AV_PKT_DATA_PRFT, &prft_size);
     if (prft && prft_size == sizeof(AVProducerReferenceTime))
@@ -6769,6 +6776,7 @@  static int mov_init(AVFormatContext *s)
         if (track->language < 0)
             track->language = 32767;  // Unspecified Macintosh language code
         track->mode = mov->mode;
+        track->every_sample_keyframe = 1;
         track->tag  = mov_find_codec_tag(s, track);
         if (!track->tag) {
             av_log(s, AV_LOG_ERROR, "Could not find tag for codec %s in stream #%d, "
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index e3a5e2864a..e78a08c2dd 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -97,6 +97,7 @@  typedef struct MOVTrack {
     long        chunkCount;
     int         has_keyframes;
     int         has_disposable;
+    int         every_sample_keyframe;
 #define MOV_TRACK_CTTS         0x0001
 #define MOV_TRACK_STPS         0x0002
 #define MOV_TRACK_ENABLED      0x0004
diff --git a/tests/ref/fate/movenc b/tests/ref/fate/movenc
index 529a1c6da5..3a593dcb9b 100644
--- a/tests/ref/fate/movenc
+++ b/tests/ref/fate/movenc
@@ -1,54 +1,54 @@ 
 write_data len 36, time nopts, type header atom ftyp
 write_data len 2429, time nopts, type header atom -
 write_data len 788, time 1000000, type sync atom moof
-write_data len 110, time nopts, type trailer atom -
-e38b4db4d6542159a18a231df1f8a17a 3363 non-empty-moov
+write_data len 91, time nopts, type trailer atom -
+cc672db4650646c640508579e7c08c3a 3344 non-empty-moov
 write_data len 36, time nopts, type header atom ftyp
 write_data len 2761, time nopts, type header atom -
 write_data len 908, time 966667, type sync atom moof
-write_data len 110, time nopts, type trailer atom -
-aced0fb08e209f7b892fc69319de4c6c 3815 non-empty-moov-elst
+write_data len 91, time nopts, type trailer atom -
+9f4f458599a3c5e064295d8bede69a5b 3796 non-empty-moov-elst
 write_data len 36, time nopts, type header atom ftyp
 write_data len 2669, time nopts, type header atom -
 write_data len 908, time 1000000, type sync atom moof
-write_data len 110, time nopts, type trailer atom -
-d2879ae4007bec11714c47ed9c4606c9 3723 non-empty-moov-no-elst
+write_data len 91, time nopts, type trailer atom -
+eff9b48e62ae0d52676a154328a2db5f 3704 non-empty-moov-no-elst
 write_data len 24, time nopts, type header atom ftyp
 write_data len 1171, time nopts, type header atom -
 write_data len 728, time 0, type sync atom moof
 write_data len 828, time nopts, type unknown atom -
 write_data len 728, time 999999, type sync atom moof
 write_data len 812, time nopts, type unknown atom -
-write_data len 148, time nopts, type trailer atom -
-92ce825ff40505ec8676191705adb7e7 4439 ismv
+write_data len 110, time nopts, type trailer atom -
+6e76f6cd18354c43eafbc342a236323f 4401 ismv
 write_data len 36, time nopts, type header atom ftyp
 write_data len 1123, time nopts, type header atom -
 write_data len 796, time 0, type sync atom moof
 write_data len 788, time 1000000, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-7c8bbc289e14ae747ad3f9c73107912b 2891 empty-moov
+write_data len 110, time nopts, type trailer atom -
+334ccaacc76cfc292fafdf6b0031ba83 2853 empty-moov
 write_data len 36, time nopts, type header atom ftyp
 write_data len 1123, time nopts, type header atom -
 write_data len 1068, time 0, type sync atom moof
 write_data len 908, time 1000000, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-0c6a543b723d16f5bb2bbb1204625a41 3283 empty-moov-no-elst
+write_data len 110, time nopts, type trailer atom -
+2914a8737c86d5194c655cf540a26ccb 3245 empty-moov-no-elst
 write_data len 36, time nopts, type header atom ftyp
 write_data len 1123, time nopts, type header atom -
 write_data len 900, time -33333, type sync atom moof
 write_data len 908, time 966667, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-f99a6f2437e513d6dcf521f8d167f342 3115 empty-moov-no-elst-no-adjust
+write_data len 110, time nopts, type trailer atom -
+5919754b902a696df1bbe92afd3eea7d 3077 empty-moov-no-elst-no-adjust
 write_data len 1159, time nopts, type header atom ftyp
 write_data len 796, time 0, type sync atom moof
 write_data len 788, time 1000000, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-7c8bbc289e14ae747ad3f9c73107912b 2891 delay-moov
+write_data len 110, time nopts, type trailer atom -
+334ccaacc76cfc292fafdf6b0031ba83 2853 delay-moov
 write_data len 1231, time nopts, type header atom ftyp
 write_data len 916, time -33333, type sync atom moof
 write_data len 908, time 966667, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-a7ddf0bfd32683de9dd22afe3b1135a4 3203 delay-moov-elst
+write_data len 110, time nopts, type trailer atom -
+d09ca12c5c03fa58d6c5776d25c597b9 3165 delay-moov-elst
 write_data len 1195, time nopts, type header atom ftyp
 write_data len 836, time 0, type sync atom moof
 write_data len 67, time nopts, type trailer atom -
@@ -64,65 +64,65 @@  write_data len 1123, time nopts, type header atom -
 write_data len 796, time 0, type sync atom moof
 write_data len 788, time 1000000, type sync atom moof
 289ee982188d66988a374a462b0b5376 1584 empty-moov-content
-write_data len 148, time nopts, type trailer atom -
+write_data len 110, time nopts, type trailer atom -
 write_data len 1159, time nopts, type header atom ftyp
 351ae2c8b6d35d98b4848c309cce6704 1159 delay-moov-header
 write_data len 796, time 0, type sync atom moof
 write_data len 788, time 1000000, type sync atom moof
 289ee982188d66988a374a462b0b5376 1584 delay-moov-content
-write_data len 148, time nopts, type trailer atom -
+write_data len 110, time nopts, type trailer atom -
 write_data len 28, time nopts, type header atom -
 write_data len 1123, time nopts, type header atom -
 write_data len 884, time 0, type sync atom sidx
 write_data len 876, time 1000000, type sync atom sidx
 c0307f99a2a362205b7e3d65b1066f86 876 empty-moov-second-frag
-write_data len 148, time nopts, type trailer atom -
+write_data len 110, time nopts, type trailer atom -
 write_data len 28, time nopts, type header atom -
 write_data len 1123, time nopts, type header atom -
 write_data len 876, time 1000000, type sync atom sidx
 c0307f99a2a362205b7e3d65b1066f86 876 empty-moov-second-frag-discont
-write_data len 110, time nopts, type trailer atom -
+write_data len 91, time nopts, type trailer atom -
 write_data len 1223, time nopts, type header atom -
 write_data len 876, time 1000000, type sync atom sidx
 c0307f99a2a362205b7e3d65b1066f86 876 delay-moov-second-frag-discont
-write_data len 110, time nopts, type trailer atom -
+write_data len 91, time nopts, type trailer atom -
 write_data len 1223, time nopts, type header atom ftyp
 b3811928793ed0749927eb2f7958421c 1223 delay-moov-elst-init
 write_data len 988, time -33333, type sync atom sidx
 write_data len 996, time 966667, type sync atom sidx
 0df125407c7e81978ce722e0ae4f6f84 996 delay-moov-elst-second-frag
-write_data len 148, time nopts, type trailer atom -
+write_data len 110, time nopts, type trailer atom -
 write_data len 1223, time nopts, type header atom ftyp
 b3811928793ed0749927eb2f7958421c 1223 delay-moov-elst-init-discont
 write_data len 996, time 966667, type sync atom sidx
 0df125407c7e81978ce722e0ae4f6f84 996 delay-moov-elst-second-frag-discont
-write_data len 110, time nopts, type trailer atom -
+write_data len 91, time nopts, type trailer atom -
 write_data len 1223, time nopts, type header atom ftyp
 041ac8efc35a0d023c26d05eedb20403 1223 delay-moov-elst-signal-init
 write_data len 1004, time -33333, type sync atom sidx
 write_data len 996, time 966667, type sync atom sidx
 5a583d89318827d2569eecbeaa18c238 996 delay-moov-elst-signal-second-frag
-write_data len 148, time nopts, type trailer atom -
+write_data len 110, time nopts, type trailer atom -
 write_data len 1223, time nopts, type header atom ftyp
 041ac8efc35a0d023c26d05eedb20403 1223 delay-moov-elst-signal-init-discont
 write_data len 996, time 966667, type sync atom sidx
 5a583d89318827d2569eecbeaa18c238 996 delay-moov-elst-signal-second-frag-discont
-write_data len 110, time nopts, type trailer atom -
+write_data len 91, time nopts, type trailer atom -
 write_data len 1247, time nopts, type header atom ftyp
 80511a51d1ac9cde62337eed7176ae03 1247 delay-moov-elst-signal-init-discont-largets
 write_data len 996, time 279621233333, type sync atom sidx
 dc695d65e8a0cdafee28acd8a5ccf81a 996 delay-moov-elst-signal-second-frag-discont-largets
-write_data len 110, time nopts, type trailer atom -
+write_data len 91, time nopts, type trailer atom -
 write_data len 1223, time nopts, type header atom ftyp
 write_data len 2572, time -333333, type sync atom sidx
 write_data len 996, time 5166667, type sync atom sidx
-write_data len 148, time nopts, type trailer atom -
-0009ab3c8ebc80a286e5b10dfacc8ef3 4939 vfr
+write_data len 110, time nopts, type trailer atom -
+a7cf74137808890eef675c2d3d104818 4901 vfr
 write_data len 1223, time nopts, type header atom ftyp
 write_data len 2572, time -333333, type sync atom sidx
 write_data len 996, time 5166667, type sync atom sidx
-write_data len 148, time nopts, type trailer atom -
-0009ab3c8ebc80a286e5b10dfacc8ef3 4939 vfr-noduration
+write_data len 110, time nopts, type trailer atom -
+a7cf74137808890eef675c2d3d104818 4901 vfr-noduration
 write_data len 1231, time nopts, type header atom ftyp
 write_data len 1500, time -333333, type sync atom moof
 write_data len 1500, time nopts, type unknown atom -
@@ -130,24 +130,24 @@  write_data len 916, time nopts, type unknown atom -
 write_data len 1500, time 9666667, type sync atom moof
 write_data len 1500, time nopts, type unknown atom -
 write_data len 1004, time nopts, type unknown atom -
-write_data len 148, time nopts, type trailer atom -
-9549eeeac8731d820dc395bc73aa605f 9299 large_frag
+write_data len 110, time nopts, type trailer atom -
+1d86c7a4ff34ef48b17e0424ef83c60a 9261 large_frag
 write_data len 1231, time nopts, type header atom ftyp
 write_data len 684, time -33333, type sync atom moof
 write_data len 504, time 800000, type boundary atom moof
 write_data len 420, time 1266667, type boundary atom moof
 write_data len 668, time 1566667, type sync atom moof
 write_data len 440, time 2233333, type boundary atom moof
-write_data len 205, time nopts, type trailer atom -
-9ec014d07518b90a5321d792d737bfc0 4152 vfr-noduration-interleave
+write_data len 110, time nopts, type trailer atom -
+52fe35e68d8f522f247e9cdfb9677f1a 4057 vfr-noduration-interleave
 write_data len 1231, time nopts, type header atom ftyp
 write_data len 916, time 0, type sync atom moof
 write_data len 908, time 1000000, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-b8076064cedf2fefc242d9af67cdaeba 3203 delay-moov-elst-neg-cts
+write_data len 110, time nopts, type trailer atom -
+efd24aed8e3d17d6ee7ca5e764391c61 3165 delay-moov-elst-neg-cts
 write_data len 36, time nopts, type header atom ftyp
 write_data len 1123, time nopts, type header atom -
 write_data len 900, time 0, type sync atom moof
 write_data len 908, time 1000000, type sync atom moof
-write_data len 148, time nopts, type trailer atom -
-c1307485f65c4a00a06ca82f5e0b1361 3115 empty-moov-neg-cts
+write_data len 110, time nopts, type trailer atom -
+fe10ded5223335608980555f972902cc 3077 empty-moov-neg-cts