Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF
@ 2023-01-04 22:16 Vignesh Venkatasubramanian
  2023-01-04 22:16 ` [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF Vignesh Venkatasubramanian
  2023-01-05  9:34 ` [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF "zhilizhao(赵志立)"
  0 siblings, 2 replies; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-04 22:16 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian

Allow specifying the movie_timescale options to AVIF ouptut.

This also makes sure that when movie_timescale is not specified,
the default value of 1000 is used instead of 0. Animated AVIF
files which don't specify the movie_timescale will have the
correct duration written in the track and movie headers after this
change (instead of writing 0).

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/movenc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 7d49892283..36c76f7f60 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -7758,6 +7758,11 @@ static const AVCodecTag codec_f4v_tags[] = {
 };
 
 #if CONFIG_AVIF_MUXER
+
+static const AVOption avif_options[] = {
+    { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
+    { NULL },
+};
 static const AVCodecTag codec_avif_tags[] = {
     { AV_CODEC_ID_AV1,     MKTAG('a','v','0','1') },
     { AV_CODEC_ID_NONE, 0 },
@@ -7767,6 +7772,7 @@ static const AVCodecTag *const codec_avif_tags_list[] = { codec_avif_tags, NULL
 static const AVClass mov_avif_muxer_class = {
     .class_name = "avif muxer",
     .item_name  = av_default_item_name,
+    .option     = avif_options,
     .version    = LIBAVUTIL_VERSION_INT,
 };
 #endif
-- 
2.39.0.314.g84b9a713c41-goog

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-04 22:16 [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF Vignesh Venkatasubramanian
@ 2023-01-04 22:16 ` Vignesh Venkatasubramanian
  2023-01-05  9:45   ` "zhilizhao(赵志立)"
  2023-01-05  9:34 ` [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF "zhilizhao(赵志立)"
  1 sibling, 1 reply; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-04 22:16 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian

The HEIF specification permits specifying the looping behavior of
animated sequences by using the EditList (elst) box. The track
duration will be set to the total duration of all the loops (or
infinite) and the duration of a single loop will be set in the edit
list box.

The default behavior is to loop infinitely.

Compliance verification:
* This was added in libavif recently [1] and the files produced by
  ffmpeg after this change have EditList boxes similar to the ones
  produced by libavif (and avifdec is able to parse the loop count as
  intended).
* ComplianceWarden is ok with the produced files.
* Chrome is able to play back the produced files.

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
 libavformat/movenc.h |  1 +
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 36c76f7f60..8d31317838 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
                                       mov->movie_timescale, track->timescale,
                                       AV_ROUND_UP);
-    int version = duration < INT32_MAX ? 0 : 1;
+    int version;
     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
     int group   = 0;
 
@@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
     size_t display_matrix_size;
     int       i;
 
+    if (mov->mode == MODE_AVIF)
+        if (!mov->avif_loop_count)
+            duration = INT64_MAX;
+        else
+            duration *= mov->avif_loop_count;
+
+     version = duration < INT32_MAX ? 0 : 1;
+
     if (st) {
         if (mov->per_stream_grouping)
             group = st->index;
@@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
     return update_size(pb, pos);
 }
 
-// This box seems important for the psp playback ... without it the movie seems to hang
+// This box is written in the following cases:
+//   * Seems important for the psp playback. Without it the movie seems to hang.
+//   * Used for specifying the looping behavior of animated AVIF (as specified
+//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
 static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
                               MOVTrack *track)
 {
@@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
     int entry_size, entry_count, size;
     int64_t delay, start_ct = track->start_cts;
     int64_t start_dts = track->start_dts;
+    int flags = 0;
 
     if (track->entry) {
         if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
@@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
 
     delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
                            track->timescale, AV_ROUND_DOWN);
+
+    if (mov->mode == MODE_AVIF) {
+        delay = 0;
+        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
+        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
+        // list is not repeated, while (flags & 1) equal to 1 specifies that the
+        // edit list is repeated.
+        flags = mov->avif_loop_count != 1;
+        start_ct = 0;
+    }
+
     version |= delay < INT32_MAX ? 0 : 1;
 
     entry_size = (version == 1) ? 20 : 12;
@@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
     avio_wb32(pb, size - 8);
     ffio_wfourcc(pb, "elst");
     avio_w8(pb, version);
-    avio_wb24(pb, 0); /* flags */
+    avio_wb24(pb, flags); /* flags */
 
     avio_wb32(pb, entry_count);
     if (delay > 0) { /* add an empty edit to delay presentation */
@@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
             avio_wb32(pb, -1);
         }
         avio_wb32(pb, 0x00010000);
-    } else {
+    } else if (mov->mode != MODE_AVIF) {
         /* Avoid accidentally ending up with start_ct = -1 which has got a
          * special meaning. Normally start_ct should end up positive or zero
          * here, but use FFMIN in case dts is a small positive integer
@@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
                    "Not writing any edit list even though one would have been required\n");
     }
 
+    if (mov->is_animated_avif)
+        mov_write_edts_tag(pb, mov, track);
+
     if (track->tref_tag)
         mov_write_tref_tag(pb, track);
 
@@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
 
 static const AVOption avif_options[] = {
     { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
+    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
     { NULL },
 };
 static const AVCodecTag codec_avif_tags[] = {
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index c6b3313deb..e85d83abdb 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
     int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
     int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
     int is_animated_avif;
+    int avif_loop_count;
 } MOVMuxContext;
 
 #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
-- 
2.39.0.314.g84b9a713c41-goog

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF
  2023-01-04 22:16 [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF Vignesh Venkatasubramanian
  2023-01-04 22:16 ` [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF Vignesh Venkatasubramanian
@ 2023-01-05  9:34 ` "zhilizhao(赵志立)"
  2023-01-05 17:26   ` Vignesh Venkatasubramanian
  1 sibling, 1 reply; 11+ messages in thread
From: "zhilizhao(赵志立)" @ 2023-01-05  9:34 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Vignesh Venkatasubramanian



> On Jan 5, 2023, at 06:16, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> 
> Allow specifying the movie_timescale options to AVIF ouptut.
> 
> This also makes sure that when movie_timescale is not specified,
> the default value of 1000 is used instead of 0. Animated AVIF
> files which don't specify the movie_timescale will have the
> correct duration written in the track and movie headers after this
> change (instead of writing 0).
> 
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
> libavformat/movenc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 7d49892283..36c76f7f60 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -7758,6 +7758,11 @@ static const AVCodecTag codec_f4v_tags[] = {
> };
> 
> #if CONFIG_AVIF_MUXER
> +
> +static const AVOption avif_options[] = {
> +    { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> +    { NULL },
> +};

If there is a chance to add more options which is shared with
mov_isobmff_muxer_class, define a common option to avoid
duplication. Otherwise keep it as this.

> static const AVCodecTag codec_avif_tags[] = {
>     { AV_CODEC_ID_AV1,     MKTAG('a','v','0','1') },
>     { AV_CODEC_ID_NONE, 0 },
> @@ -7767,6 +7772,7 @@ static const AVCodecTag *const codec_avif_tags_list[] = { codec_avif_tags, NULL
> static const AVClass mov_avif_muxer_class = {
>     .class_name = "avif muxer",
>     .item_name  = av_default_item_name,
> +    .option     = avif_options,
>     .version    = LIBAVUTIL_VERSION_INT,
> };
> #endif
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 
> _______________________________________________
> 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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-04 22:16 ` [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF Vignesh Venkatasubramanian
@ 2023-01-05  9:45   ` "zhilizhao(赵志立)"
  2023-01-05 17:34     ` Vignesh Venkatasubramanian
  0 siblings, 1 reply; 11+ messages in thread
From: "zhilizhao(赵志立)" @ 2023-01-05  9:45 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Vignesh Venkatasubramanian



> On Jan 5, 2023, at 06:16, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> 
> The HEIF specification permits specifying the looping behavior of
> animated sequences by using the EditList (elst) box. The track
> duration will be set to the total duration of all the loops (or
> infinite) and the duration of a single loop will be set in the edit
> list box.
> 
> The default behavior is to loop infinitely.
> 
> Compliance verification:
> * This was added in libavif recently [1] and the files produced by
>  ffmpeg after this change have EditList boxes similar to the ones
>  produced by libavif (and avifdec is able to parse the loop count as
>  intended).
> * ComplianceWarden is ok with the produced files.
> * Chrome is able to play back the produced files.
> 
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
> libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
> libavformat/movenc.h |  1 +
> 2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 36c76f7f60..8d31317838 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
>                                       mov->movie_timescale, track->timescale,
>                                       AV_ROUND_UP);
> -    int version = duration < INT32_MAX ? 0 : 1;
> +    int version;
>     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
>     int group   = 0;
> 
> @@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>     size_t display_matrix_size;
>     int       i;
> 
> +    if (mov->mode == MODE_AVIF)
> +        if (!mov->avif_loop_count)
> +            duration = INT64_MAX;
> +        else
> +            duration *= mov->avif_loop_count;
> +
> +     version = duration < INT32_MAX ? 0 : 1;
> +
>     if (st) {
>         if (mov->per_stream_grouping)
>             group = st->index;
> @@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
>     return update_size(pb, pos);
> }
> 
> -// This box seems important for the psp playback ... without it the movie seems to hang
> +// This box is written in the following cases:
> +//   * Seems important for the psp playback. Without it the movie seems to hang.
> +//   * Used for specifying the looping behavior of animated AVIF (as specified
> +//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
> static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>                               MOVTrack *track)
> {
> @@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>     int entry_size, entry_count, size;
>     int64_t delay, start_ct = track->start_cts;
>     int64_t start_dts = track->start_dts;
> +    int flags = 0;
> 
>     if (track->entry) {
>         if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
> @@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> 
>     delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
>                            track->timescale, AV_ROUND_DOWN);
> +
> +    if (mov->mode == MODE_AVIF) {
> +        delay = 0;
> +        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
> +        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
> +        // list is not repeated, while (flags & 1) equal to 1 specifies that the
> +        // edit list is repeated.
> +        flags = mov->avif_loop_count != 1;
> +        start_ct = 0;
> +    }
> +
>     version |= delay < INT32_MAX ? 0 : 1;
> 
>     entry_size = (version == 1) ? 20 : 12;
> @@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>     avio_wb32(pb, size - 8);
>     ffio_wfourcc(pb, "elst");
>     avio_w8(pb, version);
> -    avio_wb24(pb, 0); /* flags */
> +    avio_wb24(pb, flags); /* flags */
> 
>     avio_wb32(pb, entry_count);
>     if (delay > 0) { /* add an empty edit to delay presentation */
> @@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>             avio_wb32(pb, -1);
>         }
>         avio_wb32(pb, 0x00010000);
> -    } else {
> +    } else if (mov->mode != MODE_AVIF) {
>         /* Avoid accidentally ending up with start_ct = -1 which has got a
>          * special meaning. Normally start_ct should end up positive or zero
>          * here, but use FFMIN in case dts is a small positive integer
> @@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
>                    "Not writing any edit list even though one would have been required\n");
>     }
> 
> +    if (mov->is_animated_avif)
> +        mov_write_edts_tag(pb, mov, track);
> +
>     if (track->tref_tag)
>         mov_write_tref_tag(pb, track);
> 
> @@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
> 
> static const AVOption avif_options[] = {
>     { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> +    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },

The code treat -loop 1 means playback 1 time. It can be misleading
to mean 2 times, playback 1 time then repeat/loop 1 time. That’s
how ‘-stream_loop’ option works. I’m not English native speaker,
I don’t know which one is correct. Better make the offset by 1
clear.

>     { NULL },
> };
> static const AVCodecTag codec_avif_tags[] = {
> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> index c6b3313deb..e85d83abdb 100644
> --- a/libavformat/movenc.h
> +++ b/libavformat/movenc.h
> @@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
>     int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
>     int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
>     int is_animated_avif;
> +    int avif_loop_count;
> } MOVMuxContext;
> 
> #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 
> _______________________________________________
> 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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF
  2023-01-05  9:34 ` [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF "zhilizhao(赵志立)"
@ 2023-01-05 17:26   ` Vignesh Venkatasubramanian
  0 siblings, 0 replies; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-05 17:26 UTC (permalink / raw)
  To: zhilizhao(赵志立)
  Cc: FFmpeg development discussions and patches

On Thu, Jan 5, 2023 at 1:34 AM "zhilizhao(赵志立)" <quinkblack@foxmail.com> wrote:
>
>
>
> > On Jan 5, 2023, at 06:16, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > Allow specifying the movie_timescale options to AVIF ouptut.
> >
> > This also makes sure that when movie_timescale is not specified,
> > the default value of 1000 is used instead of 0. Animated AVIF
> > files which don't specify the movie_timescale will have the
> > correct duration written in the track and movie headers after this
> > change (instead of writing 0).
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> > libavformat/movenc.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > index 7d49892283..36c76f7f60 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -7758,6 +7758,11 @@ static const AVCodecTag codec_f4v_tags[] = {
> > };
> >
> > #if CONFIG_AVIF_MUXER
> > +
> > +static const AVOption avif_options[] = {
> > +    { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> > +    { NULL },
> > +};
>
> If there is a chance to add more options which is shared with
> mov_isobmff_muxer_class, define a common option to avoid
> duplication. Otherwise keep it as this.
>

As of now, this is the only one that's being repeated. I think we can
make a common option if we decide to copy over more from the
mov_isobmff_muxer_class.

> > static const AVCodecTag codec_avif_tags[] = {
> >     { AV_CODEC_ID_AV1,     MKTAG('a','v','0','1') },
> >     { AV_CODEC_ID_NONE, 0 },
> > @@ -7767,6 +7772,7 @@ static const AVCodecTag *const codec_avif_tags_list[] = { codec_avif_tags, NULL
> > static const AVClass mov_avif_muxer_class = {
> >     .class_name = "avif muxer",
> >     .item_name  = av_default_item_name,
> > +    .option     = avif_options,
> >     .version    = LIBAVUTIL_VERSION_INT,
> > };
> > #endif
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >
> > _______________________________________________
> > 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".
>


-- 
Vignesh
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-05  9:45   ` "zhilizhao(赵志立)"
@ 2023-01-05 17:34     ` Vignesh Venkatasubramanian
  2023-01-06  9:45       ` "zhilizhao(赵志立)"
  0 siblings, 1 reply; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-05 17:34 UTC (permalink / raw)
  To: zhilizhao(赵志立)
  Cc: FFmpeg development discussions and patches

On Thu, Jan 5, 2023 at 1:45 AM "zhilizhao(赵志立)" <quinkblack@foxmail.com> wrote:
>
>
>
> > On Jan 5, 2023, at 06:16, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > The HEIF specification permits specifying the looping behavior of
> > animated sequences by using the EditList (elst) box. The track
> > duration will be set to the total duration of all the loops (or
> > infinite) and the duration of a single loop will be set in the edit
> > list box.
> >
> > The default behavior is to loop infinitely.
> >
> > Compliance verification:
> > * This was added in libavif recently [1] and the files produced by
> >  ffmpeg after this change have EditList boxes similar to the ones
> >  produced by libavif (and avifdec is able to parse the loop count as
> >  intended).
> > * ComplianceWarden is ok with the produced files.
> > * Chrome is able to play back the produced files.
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> > libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
> > libavformat/movenc.h |  1 +
> > 2 files changed, 32 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > index 36c76f7f60..8d31317838 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
> >                                       mov->movie_timescale, track->timescale,
> >                                       AV_ROUND_UP);
> > -    int version = duration < INT32_MAX ? 0 : 1;
> > +    int version;
> >     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
> >     int group   = 0;
> >
> > @@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     size_t display_matrix_size;
> >     int       i;
> >
> > +    if (mov->mode == MODE_AVIF)
> > +        if (!mov->avif_loop_count)
> > +            duration = INT64_MAX;
> > +        else
> > +            duration *= mov->avif_loop_count;
> > +
> > +     version = duration < INT32_MAX ? 0 : 1;
> > +
> >     if (st) {
> >         if (mov->per_stream_grouping)
> >             group = st->index;
> > @@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
> >     return update_size(pb, pos);
> > }
> >
> > -// This box seems important for the psp playback ... without it the movie seems to hang
> > +// This box is written in the following cases:
> > +//   * Seems important for the psp playback. Without it the movie seems to hang.
> > +//   * Used for specifying the looping behavior of animated AVIF (as specified
> > +//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
> > static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >                               MOVTrack *track)
> > {
> > @@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     int entry_size, entry_count, size;
> >     int64_t delay, start_ct = track->start_cts;
> >     int64_t start_dts = track->start_dts;
> > +    int flags = 0;
> >
> >     if (track->entry) {
> >         if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
> > @@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >
> >     delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
> >                            track->timescale, AV_ROUND_DOWN);
> > +
> > +    if (mov->mode == MODE_AVIF) {
> > +        delay = 0;
> > +        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
> > +        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
> > +        // list is not repeated, while (flags & 1) equal to 1 specifies that the
> > +        // edit list is repeated.
> > +        flags = mov->avif_loop_count != 1;
> > +        start_ct = 0;
> > +    }
> > +
> >     version |= delay < INT32_MAX ? 0 : 1;
> >
> >     entry_size = (version == 1) ? 20 : 12;
> > @@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     avio_wb32(pb, size - 8);
> >     ffio_wfourcc(pb, "elst");
> >     avio_w8(pb, version);
> > -    avio_wb24(pb, 0); /* flags */
> > +    avio_wb24(pb, flags); /* flags */
> >
> >     avio_wb32(pb, entry_count);
> >     if (delay > 0) { /* add an empty edit to delay presentation */
> > @@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >             avio_wb32(pb, -1);
> >         }
> >         avio_wb32(pb, 0x00010000);
> > -    } else {
> > +    } else if (mov->mode != MODE_AVIF) {
> >         /* Avoid accidentally ending up with start_ct = -1 which has got a
> >          * special meaning. Normally start_ct should end up positive or zero
> >          * here, but use FFMIN in case dts is a small positive integer
> > @@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
> >                    "Not writing any edit list even though one would have been required\n");
> >     }
> >
> > +    if (mov->is_animated_avif)
> > +        mov_write_edts_tag(pb, mov, track);
> > +
> >     if (track->tref_tag)
> >         mov_write_tref_tag(pb, track);
> >
> > @@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
> >
> > static const AVOption avif_options[] = {
> >     { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> > +    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
>
> The code treat -loop 1 means playback 1 time. It can be misleading
> to mean 2 times, playback 1 time then repeat/loop 1 time. That’s
> how ‘-stream_loop’ option works. I’m not English native speaker,
> I don’t know which one is correct. Better make the offset by 1
> clear.
>

libavif uses the term "repetitionCount" instead of "loop" to not have
this ambiguity.

The behavior of the "loop" parameter in this implementation is similar
to WebP's "loop" parameter in ffmpeg (i.e.) loop means the number of
times it will be played back (loop == 1 => 1 playback, loop == 2 => 2
playbacks and so on).

> >     { NULL },
> > };
> > static const AVCodecTag codec_avif_tags[] = {
> > diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> > index c6b3313deb..e85d83abdb 100644
> > --- a/libavformat/movenc.h
> > +++ b/libavformat/movenc.h
> > @@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
> >     int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
> >     int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
> >     int is_animated_avif;
> > +    int avif_loop_count;
> > } MOVMuxContext;
> >
> > #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >
> > _______________________________________________
> > 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".
>


-- 
Vignesh
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-05 17:34     ` Vignesh Venkatasubramanian
@ 2023-01-06  9:45       ` "zhilizhao(赵志立)"
  2023-01-12 19:30         ` Vignesh Venkatasubramanian
  0 siblings, 1 reply; 11+ messages in thread
From: "zhilizhao(赵志立)" @ 2023-01-06  9:45 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



> On Jan 6, 2023, at 01:34, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> 
> On Thu, Jan 5, 2023 at 1:45 AM "zhilizhao(赵志立)" <quinkblack@foxmail.com> wrote:
>> 
>> 
>> 
>>> On Jan 5, 2023, at 06:16, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
>>> 
>>> The HEIF specification permits specifying the looping behavior of
>>> animated sequences by using the EditList (elst) box. The track
>>> duration will be set to the total duration of all the loops (or
>>> infinite) and the duration of a single loop will be set in the edit
>>> list box.
>>> 
>>> The default behavior is to loop infinitely.
>>> 
>>> Compliance verification:
>>> * This was added in libavif recently [1] and the files produced by
>>> ffmpeg after this change have EditList boxes similar to the ones
>>> produced by libavif (and avifdec is able to parse the loop count as
>>> intended).
>>> * ComplianceWarden is ok with the produced files.
>>> * Chrome is able to play back the produced files.
>>> 
>>> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
>>> ---
>>> libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
>>> libavformat/movenc.h |  1 +
>>> 2 files changed, 32 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index 36c76f7f60..8d31317838 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>>>    int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
>>>                                      mov->movie_timescale, track->timescale,
>>>                                      AV_ROUND_UP);
>>> -    int version = duration < INT32_MAX ? 0 : 1;
>>> +    int version;
>>>    int flags   = MOV_TKHD_FLAG_IN_MOVIE;
>>>    int group   = 0;
>>> 
>>> @@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>>>    size_t display_matrix_size;
>>>    int       i;
>>> 
>>> +    if (mov->mode == MODE_AVIF)
>>> +        if (!mov->avif_loop_count)
>>> +            duration = INT64_MAX;
>>> +        else
>>> +            duration *= mov->avif_loop_count;
>>> +
>>> +     version = duration < INT32_MAX ? 0 : 1;
>>> +
>>>    if (st) {
>>>        if (mov->per_stream_grouping)
>>>            group = st->index;
>>> @@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
>>>    return update_size(pb, pos);
>>> }
>>> 
>>> -// This box seems important for the psp playback ... without it the movie seems to hang
>>> +// This box is written in the following cases:
>>> +//   * Seems important for the psp playback. Without it the movie seems to hang.
>>> +//   * Used for specifying the looping behavior of animated AVIF (as specified
>>> +//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
>>> static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>>>                              MOVTrack *track)
>>> {
>>> @@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>>>    int entry_size, entry_count, size;
>>>    int64_t delay, start_ct = track->start_cts;
>>>    int64_t start_dts = track->start_dts;
>>> +    int flags = 0;
>>> 
>>>    if (track->entry) {
>>>        if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
>>> @@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>>> 
>>>    delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
>>>                           track->timescale, AV_ROUND_DOWN);
>>> +
>>> +    if (mov->mode == MODE_AVIF) {
>>> +        delay = 0;
>>> +        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
>>> +        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
>>> +        // list is not repeated, while (flags & 1) equal to 1 specifies that the
>>> +        // edit list is repeated.
>>> +        flags = mov->avif_loop_count != 1;
>>> +        start_ct = 0;
>>> +    }
>>> +
>>>    version |= delay < INT32_MAX ? 0 : 1;
>>> 
>>>    entry_size = (version == 1) ? 20 : 12;
>>> @@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>>>    avio_wb32(pb, size - 8);
>>>    ffio_wfourcc(pb, "elst");
>>>    avio_w8(pb, version);
>>> -    avio_wb24(pb, 0); /* flags */
>>> +    avio_wb24(pb, flags); /* flags */
>>> 
>>>    avio_wb32(pb, entry_count);
>>>    if (delay > 0) { /* add an empty edit to delay presentation */
>>> @@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>>>            avio_wb32(pb, -1);
>>>        }
>>>        avio_wb32(pb, 0x00010000);
>>> -    } else {
>>> +    } else if (mov->mode != MODE_AVIF) {
>>>        /* Avoid accidentally ending up with start_ct = -1 which has got a
>>>         * special meaning. Normally start_ct should end up positive or zero
>>>         * here, but use FFMIN in case dts is a small positive integer
>>> @@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
>>>                   "Not writing any edit list even though one would have been required\n");
>>>    }
>>> 
>>> +    if (mov->is_animated_avif)
>>> +        mov_write_edts_tag(pb, mov, track);
>>> +
>>>    if (track->tref_tag)
>>>        mov_write_tref_tag(pb, track);
>>> 
>>> @@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
>>> 
>>> static const AVOption avif_options[] = {
>>>    { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
>>> +    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
>> 
>> The code treat -loop 1 means playback 1 time. It can be misleading
>> to mean 2 times, playback 1 time then repeat/loop 1 time. That’s
>> how ‘-stream_loop’ option works. I’m not English native speaker,
>> I don’t know which one is correct. Better make the offset by 1
>> clear.
>> 
> 
> libavif uses the term "repetitionCount" instead of "loop" to not have
> this ambiguity.
> 
> The behavior of the "loop" parameter in this implementation is similar
> to WebP's "loop" parameter in ffmpeg (i.e.) loop means the number of
> times it will be played back (loop == 1 => 1 playback, loop == 2 => 2
> playbacks and so on).

LGTM then.

> 
>>>    { NULL },
>>> };
>>> static const AVCodecTag codec_avif_tags[] = {
>>> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
>>> index c6b3313deb..e85d83abdb 100644
>>> --- a/libavformat/movenc.h
>>> +++ b/libavformat/movenc.h
>>> @@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
>>>    int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
>>>    int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
>>>    int is_animated_avif;
>>> +    int avif_loop_count;
>>> } MOVMuxContext;
>>> 
>>> #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
>>> --
>>> 2.39.0.314.g84b9a713c41-goog
>>> 
>>> _______________________________________________
>>> 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".
>> 
> 
> 
> -- 
> Vignesh
> _______________________________________________
> 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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-06  9:45       ` "zhilizhao(赵志立)"
@ 2023-01-12 19:30         ` Vignesh Venkatasubramanian
  2023-01-12 20:45           ` Vignesh Venkatasubramanian
  0 siblings, 1 reply; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-12 19:30 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Fri, Jan 6, 2023 at 1:45 AM "zhilizhao(赵志立)" <quinkblack@foxmail.com> wrote:
>
>
>
> > On Jan 6, 2023, at 01:34, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > On Thu, Jan 5, 2023 at 1:45 AM "zhilizhao(赵志立)" <quinkblack@foxmail.com> wrote:
> >>
> >>
> >>
> >>> On Jan 5, 2023, at 06:16, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> >>>
> >>> The HEIF specification permits specifying the looping behavior of
> >>> animated sequences by using the EditList (elst) box. The track
> >>> duration will be set to the total duration of all the loops (or
> >>> infinite) and the duration of a single loop will be set in the edit
> >>> list box.
> >>>
> >>> The default behavior is to loop infinitely.
> >>>
> >>> Compliance verification:
> >>> * This was added in libavif recently [1] and the files produced by
> >>> ffmpeg after this change have EditList boxes similar to the ones
> >>> produced by libavif (and avifdec is able to parse the loop count as
> >>> intended).
> >>> * ComplianceWarden is ok with the produced files.
> >>> * Chrome is able to play back the produced files.
> >>>
> >>> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> >>> ---
> >>> libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
> >>> libavformat/movenc.h |  1 +
> >>> 2 files changed, 32 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> >>> index 36c76f7f60..8d31317838 100644
> >>> --- a/libavformat/movenc.c
> >>> +++ b/libavformat/movenc.c
> >>> @@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>    int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
> >>>                                      mov->movie_timescale, track->timescale,
> >>>                                      AV_ROUND_UP);
> >>> -    int version = duration < INT32_MAX ? 0 : 1;
> >>> +    int version;
> >>>    int flags   = MOV_TKHD_FLAG_IN_MOVIE;
> >>>    int group   = 0;
> >>>
> >>> @@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>    size_t display_matrix_size;
> >>>    int       i;
> >>>
> >>> +    if (mov->mode == MODE_AVIF)
> >>> +        if (!mov->avif_loop_count)
> >>> +            duration = INT64_MAX;
> >>> +        else
> >>> +            duration *= mov->avif_loop_count;
> >>> +
> >>> +     version = duration < INT32_MAX ? 0 : 1;
> >>> +
> >>>    if (st) {
> >>>        if (mov->per_stream_grouping)
> >>>            group = st->index;
> >>> @@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
> >>>    return update_size(pb, pos);
> >>> }
> >>>
> >>> -// This box seems important for the psp playback ... without it the movie seems to hang
> >>> +// This box is written in the following cases:
> >>> +//   * Seems important for the psp playback. Without it the movie seems to hang.
> >>> +//   * Used for specifying the looping behavior of animated AVIF (as specified
> >>> +//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
> >>> static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>                              MOVTrack *track)
> >>> {
> >>> @@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>    int entry_size, entry_count, size;
> >>>    int64_t delay, start_ct = track->start_cts;
> >>>    int64_t start_dts = track->start_dts;
> >>> +    int flags = 0;
> >>>
> >>>    if (track->entry) {
> >>>        if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
> >>> @@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>
> >>>    delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
> >>>                           track->timescale, AV_ROUND_DOWN);
> >>> +
> >>> +    if (mov->mode == MODE_AVIF) {
> >>> +        delay = 0;
> >>> +        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
> >>> +        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
> >>> +        // list is not repeated, while (flags & 1) equal to 1 specifies that the
> >>> +        // edit list is repeated.
> >>> +        flags = mov->avif_loop_count != 1;
> >>> +        start_ct = 0;
> >>> +    }
> >>> +
> >>>    version |= delay < INT32_MAX ? 0 : 1;
> >>>
> >>>    entry_size = (version == 1) ? 20 : 12;
> >>> @@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>    avio_wb32(pb, size - 8);
> >>>    ffio_wfourcc(pb, "elst");
> >>>    avio_w8(pb, version);
> >>> -    avio_wb24(pb, 0); /* flags */
> >>> +    avio_wb24(pb, flags); /* flags */
> >>>
> >>>    avio_wb32(pb, entry_count);
> >>>    if (delay > 0) { /* add an empty edit to delay presentation */
> >>> @@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >>>            avio_wb32(pb, -1);
> >>>        }
> >>>        avio_wb32(pb, 0x00010000);
> >>> -    } else {
> >>> +    } else if (mov->mode != MODE_AVIF) {
> >>>        /* Avoid accidentally ending up with start_ct = -1 which has got a
> >>>         * special meaning. Normally start_ct should end up positive or zero
> >>>         * here, but use FFMIN in case dts is a small positive integer
> >>> @@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
> >>>                   "Not writing any edit list even though one would have been required\n");
> >>>    }
> >>>
> >>> +    if (mov->is_animated_avif)
> >>> +        mov_write_edts_tag(pb, mov, track);
> >>> +
> >>>    if (track->tref_tag)
> >>>        mov_write_tref_tag(pb, track);
> >>>
> >>> @@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
> >>>
> >>> static const AVOption avif_options[] = {
> >>>    { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> >>> +    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
> >>
> >> The code treat -loop 1 means playback 1 time. It can be misleading
> >> to mean 2 times, playback 1 time then repeat/loop 1 time. That’s
> >> how ‘-stream_loop’ option works. I’m not English native speaker,
> >> I don’t know which one is correct. Better make the offset by 1
> >> clear.
> >>
> >
> > libavif uses the term "repetitionCount" instead of "loop" to not have
> > this ambiguity.
> >
> > The behavior of the "loop" parameter in this implementation is similar
> > to WebP's "loop" parameter in ffmpeg (i.e.) loop means the number of
> > times it will be played back (loop == 1 => 1 playback, loop == 2 => 2
> > playbacks and so on).
>
> LGTM then.
>
> >
> >>>    { NULL },
> >>> };
> >>> static const AVCodecTag codec_avif_tags[] = {
> >>> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> >>> index c6b3313deb..e85d83abdb 100644
> >>> --- a/libavformat/movenc.h
> >>> +++ b/libavformat/movenc.h
> >>> @@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
> >>>    int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
> >>>    int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
> >>>    int is_animated_avif;
> >>> +    int avif_loop_count;
> >>> } MOVMuxContext;
> >>>
> >>> #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
> >>> --
> >>> 2.39.0.314.g84b9a713c41-goog
> >>>
> >>> _______________________________________________
> >>> 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".
> >>
> >
> >
> > --
> > Vignesh
> > _______________________________________________
> > 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".
>
> _______________________________________________
> 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".

If there are no further comments, can this series of patches be merged please?

-- 
Vignesh
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-12 19:30         ` Vignesh Venkatasubramanian
@ 2023-01-12 20:45           ` Vignesh Venkatasubramanian
  2023-01-13  4:40             ` "zhilizhao(赵志立)"
  0 siblings, 1 reply; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-12 20:45 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian

The HEIF specification permits specifying the looping behavior of
animated sequences by using the EditList (elst) box. The track
duration will be set to the total duration of all the loops (or
infinite) and the duration of a single loop will be set in the edit
list box.

The default behavior is to loop infinitely.

Compliance verification:
* This was added in libavif recently [1] and the files produced by
  ffmpeg after this change have EditList boxes similar to the ones
  produced by libavif (and avifdec is able to parse the loop count as
  intended).
* ComplianceWarden is ok with the produced files.
* Chrome is able to play back the produced files.

[1] https://github.com/AOMediaCodec/libavif/commit/4d2776a3af53ae1aefdaed463b75ba12fd9cf8c2

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
 libavformat/movenc.h |  1 +
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 36c76f7f60..8d31317838 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
                                       mov->movie_timescale, track->timescale,
                                       AV_ROUND_UP);
-    int version = duration < INT32_MAX ? 0 : 1;
+    int version;
     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
     int group   = 0;
 
@@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
     size_t display_matrix_size;
     int       i;
 
+    if (mov->mode == MODE_AVIF)
+        if (!mov->avif_loop_count)
+            duration = INT64_MAX;
+        else
+            duration *= mov->avif_loop_count;
+
+     version = duration < INT32_MAX ? 0 : 1;
+
     if (st) {
         if (mov->per_stream_grouping)
             group = st->index;
@@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
     return update_size(pb, pos);
 }
 
-// This box seems important for the psp playback ... without it the movie seems to hang
+// This box is written in the following cases:
+//   * Seems important for the psp playback. Without it the movie seems to hang.
+//   * Used for specifying the looping behavior of animated AVIF (as specified
+//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
 static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
                               MOVTrack *track)
 {
@@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
     int entry_size, entry_count, size;
     int64_t delay, start_ct = track->start_cts;
     int64_t start_dts = track->start_dts;
+    int flags = 0;
 
     if (track->entry) {
         if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
@@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
 
     delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
                            track->timescale, AV_ROUND_DOWN);
+
+    if (mov->mode == MODE_AVIF) {
+        delay = 0;
+        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
+        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
+        // list is not repeated, while (flags & 1) equal to 1 specifies that the
+        // edit list is repeated.
+        flags = mov->avif_loop_count != 1;
+        start_ct = 0;
+    }
+
     version |= delay < INT32_MAX ? 0 : 1;
 
     entry_size = (version == 1) ? 20 : 12;
@@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
     avio_wb32(pb, size - 8);
     ffio_wfourcc(pb, "elst");
     avio_w8(pb, version);
-    avio_wb24(pb, 0); /* flags */
+    avio_wb24(pb, flags); /* flags */
 
     avio_wb32(pb, entry_count);
     if (delay > 0) { /* add an empty edit to delay presentation */
@@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
             avio_wb32(pb, -1);
         }
         avio_wb32(pb, 0x00010000);
-    } else {
+    } else if (mov->mode != MODE_AVIF) {
         /* Avoid accidentally ending up with start_ct = -1 which has got a
          * special meaning. Normally start_ct should end up positive or zero
          * here, but use FFMIN in case dts is a small positive integer
@@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
                    "Not writing any edit list even though one would have been required\n");
     }
 
+    if (mov->is_animated_avif)
+        mov_write_edts_tag(pb, mov, track);
+
     if (track->tref_tag)
         mov_write_tref_tag(pb, track);
 
@@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
 
 static const AVOption avif_options[] = {
     { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
+    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
     { NULL },
 };
 static const AVCodecTag codec_avif_tags[] = {
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index c6b3313deb..e85d83abdb 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
     int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
     int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
     int is_animated_avif;
+    int avif_loop_count;
 } MOVMuxContext;
 
 #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
-- 
2.39.0.314.g84b9a713c41-goog

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-12 20:45           ` Vignesh Venkatasubramanian
@ 2023-01-13  4:40             ` "zhilizhao(赵志立)"
  2023-01-13 22:14               ` Vignesh Venkatasubramanian
  0 siblings, 1 reply; 11+ messages in thread
From: "zhilizhao(赵志立)" @ 2023-01-13  4:40 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Vignesh Venkatasubramanian



> On Jan 13, 2023, at 04:45, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> 
> The HEIF specification permits specifying the looping behavior of
> animated sequences by using the EditList (elst) box. The track
> duration will be set to the total duration of all the loops (or
> infinite) and the duration of a single loop will be set in the edit
> list box.
> 
> The default behavior is to loop infinitely.
> 
> Compliance verification:
> * This was added in libavif recently [1] and the files produced by
>  ffmpeg after this change have EditList boxes similar to the ones
>  produced by libavif (and avifdec is able to parse the loop count as
>  intended).
> * ComplianceWarden is ok with the produced files.
> * Chrome is able to play back the produced files.
> 
> [1] https://github.com/AOMediaCodec/libavif/commit/4d2776a3af53ae1aefdaed463b75ba12fd9cf8c2

Pushed.

Please use [PATCH v2] next time. It isn’t clear whether it’s the same
version as another patch or not, until diffed manually.

> 
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
> libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
> libavformat/movenc.h |  1 +
> 2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 36c76f7f60..8d31317838 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
>                                       mov->movie_timescale, track->timescale,
>                                       AV_ROUND_UP);
> -    int version = duration < INT32_MAX ? 0 : 1;
> +    int version;
>     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
>     int group   = 0;
> 
> @@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
>     size_t display_matrix_size;
>     int       i;
> 
> +    if (mov->mode == MODE_AVIF)
> +        if (!mov->avif_loop_count)
> +            duration = INT64_MAX;
> +        else
> +            duration *= mov->avif_loop_count;
> +
> +     version = duration < INT32_MAX ? 0 : 1;
> +
>     if (st) {
>         if (mov->per_stream_grouping)
>             group = st->index;
> @@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
>     return update_size(pb, pos);
> }
> 
> -// This box seems important for the psp playback ... without it the movie seems to hang
> +// This box is written in the following cases:
> +//   * Seems important for the psp playback. Without it the movie seems to hang.
> +//   * Used for specifying the looping behavior of animated AVIF (as specified
> +//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
> static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>                               MOVTrack *track)
> {
> @@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>     int entry_size, entry_count, size;
>     int64_t delay, start_ct = track->start_cts;
>     int64_t start_dts = track->start_dts;
> +    int flags = 0;
> 
>     if (track->entry) {
>         if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
> @@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> 
>     delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
>                            track->timescale, AV_ROUND_DOWN);
> +
> +    if (mov->mode == MODE_AVIF) {
> +        delay = 0;
> +        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
> +        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
> +        // list is not repeated, while (flags & 1) equal to 1 specifies that the
> +        // edit list is repeated.
> +        flags = mov->avif_loop_count != 1;
> +        start_ct = 0;
> +    }
> +
>     version |= delay < INT32_MAX ? 0 : 1;
> 
>     entry_size = (version == 1) ? 20 : 12;
> @@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>     avio_wb32(pb, size - 8);
>     ffio_wfourcc(pb, "elst");
>     avio_w8(pb, version);
> -    avio_wb24(pb, 0); /* flags */
> +    avio_wb24(pb, flags); /* flags */
> 
>     avio_wb32(pb, entry_count);
>     if (delay > 0) { /* add an empty edit to delay presentation */
> @@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
>             avio_wb32(pb, -1);
>         }
>         avio_wb32(pb, 0x00010000);
> -    } else {
> +    } else if (mov->mode != MODE_AVIF) {
>         /* Avoid accidentally ending up with start_ct = -1 which has got a
>          * special meaning. Normally start_ct should end up positive or zero
>          * here, but use FFMIN in case dts is a small positive integer
> @@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
>                    "Not writing any edit list even though one would have been required\n");
>     }
> 
> +    if (mov->is_animated_avif)
> +        mov_write_edts_tag(pb, mov, track);
> +
>     if (track->tref_tag)
>         mov_write_tref_tag(pb, track);
> 
> @@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
> 
> static const AVOption avif_options[] = {
>     { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> +    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
>     { NULL },
> };
> static const AVCodecTag codec_avif_tags[] = {
> diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> index c6b3313deb..e85d83abdb 100644
> --- a/libavformat/movenc.h
> +++ b/libavformat/movenc.h
> @@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
>     int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
>     int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
>     int is_animated_avif;
> +    int avif_loop_count;
> } MOVMuxContext;
> 
> #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
> -- 
> 2.39.0.314.g84b9a713c41-goog
> 
> _______________________________________________
> 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".

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF
  2023-01-13  4:40             ` "zhilizhao(赵志立)"
@ 2023-01-13 22:14               ` Vignesh Venkatasubramanian
  0 siblings, 0 replies; 11+ messages in thread
From: Vignesh Venkatasubramanian @ 2023-01-13 22:14 UTC (permalink / raw)
  To: zhilizhao(赵志立)
  Cc: FFmpeg development discussions and patches

On Thu, Jan 12, 2023 at 8:40 PM "zhilizhao(赵志立)" <quinkblack@foxmail.com> wrote:
>
>
>
> > On Jan 13, 2023, at 04:45, Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > The HEIF specification permits specifying the looping behavior of
> > animated sequences by using the EditList (elst) box. The track
> > duration will be set to the total duration of all the loops (or
> > infinite) and the duration of a single loop will be set in the edit
> > list box.
> >
> > The default behavior is to loop infinitely.
> >
> > Compliance verification:
> > * This was added in libavif recently [1] and the files produced by
> >  ffmpeg after this change have EditList boxes similar to the ones
> >  produced by libavif (and avifdec is able to parse the loop count as
> >  intended).
> > * ComplianceWarden is ok with the produced files.
> > * Chrome is able to play back the produced files.
> >
> > [1] https://github.com/AOMediaCodec/libavif/commit/4d2776a3af53ae1aefdaed463b75ba12fd9cf8c2
>
> Pushed.
>

Thank you!

> Please use [PATCH v2] next time. It isn’t clear whether it’s the same
> version as another patch or not, until diffed manually.
>

Sorry, will use v2 in the future. The patch was the same except for
adding the missing libavif link in the commit message.

> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> > libavformat/movenc.c | 35 +++++++++++++++++++++++++++++++----
> > libavformat/movenc.h |  1 +
> > 2 files changed, 32 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> > index 36c76f7f60..8d31317838 100644
> > --- a/libavformat/movenc.c
> > +++ b/libavformat/movenc.c
> > @@ -3287,7 +3287,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
> >                                       mov->movie_timescale, track->timescale,
> >                                       AV_ROUND_UP);
> > -    int version = duration < INT32_MAX ? 0 : 1;
> > +    int version;
> >     int flags   = MOV_TKHD_FLAG_IN_MOVIE;
> >     int group   = 0;
> >
> > @@ -3295,6 +3295,14 @@ static int mov_write_tkhd_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     size_t display_matrix_size;
> >     int       i;
> >
> > +    if (mov->mode == MODE_AVIF)
> > +        if (!mov->avif_loop_count)
> > +            duration = INT64_MAX;
> > +        else
> > +            duration *= mov->avif_loop_count;
> > +
> > +     version = duration < INT32_MAX ? 0 : 1;
> > +
> >     if (st) {
> >         if (mov->per_stream_grouping)
> >             group = st->index;
> > @@ -3414,7 +3422,10 @@ static int mov_write_tapt_tag(AVIOContext *pb, MOVTrack *track)
> >     return update_size(pb, pos);
> > }
> >
> > -// This box seems important for the psp playback ... without it the movie seems to hang
> > +// This box is written in the following cases:
> > +//   * Seems important for the psp playback. Without it the movie seems to hang.
> > +//   * Used for specifying the looping behavior of animated AVIF (as specified
> > +//   in Section 9.6 of the HEIF specification ISO/IEC 23008-12).
> > static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >                               MOVTrack *track)
> > {
> > @@ -3425,6 +3436,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     int entry_size, entry_count, size;
> >     int64_t delay, start_ct = track->start_cts;
> >     int64_t start_dts = track->start_dts;
> > +    int flags = 0;
> >
> >     if (track->entry) {
> >         if (start_dts != track->cluster[0].dts || start_ct != track->cluster[0].cts) {
> > @@ -3440,6 +3452,17 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >
> >     delay = av_rescale_rnd(start_dts + start_ct, mov->movie_timescale,
> >                            track->timescale, AV_ROUND_DOWN);
> > +
> > +    if (mov->mode == MODE_AVIF) {
> > +        delay = 0;
> > +        // Section 9.6.3 of ISO/IEC 23008-12: flags specifies repetition of the
> > +        // edit list as follows: (flags & 1) equal to 0 specifies that the edit
> > +        // list is not repeated, while (flags & 1) equal to 1 specifies that the
> > +        // edit list is repeated.
> > +        flags = mov->avif_loop_count != 1;
> > +        start_ct = 0;
> > +    }
> > +
> >     version |= delay < INT32_MAX ? 0 : 1;
> >
> >     entry_size = (version == 1) ? 20 : 12;
> > @@ -3452,7 +3475,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >     avio_wb32(pb, size - 8);
> >     ffio_wfourcc(pb, "elst");
> >     avio_w8(pb, version);
> > -    avio_wb24(pb, 0); /* flags */
> > +    avio_wb24(pb, flags); /* flags */
> >
> >     avio_wb32(pb, entry_count);
> >     if (delay > 0) { /* add an empty edit to delay presentation */
> > @@ -3469,7 +3492,7 @@ static int mov_write_edts_tag(AVIOContext *pb, MOVMuxContext *mov,
> >             avio_wb32(pb, -1);
> >         }
> >         avio_wb32(pb, 0x00010000);
> > -    } else {
> > +    } else if (mov->mode != MODE_AVIF) {
> >         /* Avoid accidentally ending up with start_ct = -1 which has got a
> >          * special meaning. Normally start_ct should end up positive or zero
> >          * here, but use FFMIN in case dts is a small positive integer
> > @@ -3670,6 +3693,9 @@ static int mov_write_trak_tag(AVFormatContext *s, AVIOContext *pb, MOVMuxContext
> >                    "Not writing any edit list even though one would have been required\n");
> >     }
> >
> > +    if (mov->is_animated_avif)
> > +        mov_write_edts_tag(pb, mov, track);
> > +
> >     if (track->tref_tag)
> >         mov_write_tref_tag(pb, track);
> >
> > @@ -7761,6 +7787,7 @@ static const AVCodecTag codec_f4v_tags[] = {
> >
> > static const AVOption avif_options[] = {
> >     { "movie_timescale", "set movie timescale", offsetof(MOVMuxContext, movie_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> > +    { "loop", "Number of times to loop animated AVIF: 0 - infinite loop", offsetof(MOVMuxContext, avif_loop_count), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, 0 },
> >     { NULL },
> > };
> > static const AVCodecTag codec_avif_tags[] = {
> > diff --git a/libavformat/movenc.h b/libavformat/movenc.h
> > index c6b3313deb..e85d83abdb 100644
> > --- a/libavformat/movenc.h
> > +++ b/libavformat/movenc.h
> > @@ -249,6 +249,7 @@ typedef struct MOVMuxContext {
> >     int64_t avif_extent_pos[2];  // index 0 is YUV and 1 is Alpha.
> >     int avif_extent_length[2];   // index 0 is YUV and 1 is Alpha.
> >     int is_animated_avif;
> > +    int avif_loop_count;
> > } MOVMuxContext;
> >
> > #define FF_MOV_FLAG_RTP_HINT              (1 <<  0)
> > --
> > 2.39.0.314.g84b9a713c41-goog
> >
> > _______________________________________________
> > 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".
>


-- 
Vignesh
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-01-13 22:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-04 22:16 [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF Vignesh Venkatasubramanian
2023-01-04 22:16 ` [FFmpeg-devel] [PATCH 2/2] avformat/movenc: Add loop parameter to animated AVIF Vignesh Venkatasubramanian
2023-01-05  9:45   ` "zhilizhao(赵志立)"
2023-01-05 17:34     ` Vignesh Venkatasubramanian
2023-01-06  9:45       ` "zhilizhao(赵志立)"
2023-01-12 19:30         ` Vignesh Venkatasubramanian
2023-01-12 20:45           ` Vignesh Venkatasubramanian
2023-01-13  4:40             ` "zhilizhao(赵志立)"
2023-01-13 22:14               ` Vignesh Venkatasubramanian
2023-01-05  9:34 ` [FFmpeg-devel] [PATCH 1/2] avformat/movenc: Add movie_timescale option to AVIF "zhilizhao(赵志立)"
2023-01-05 17:26   ` Vignesh Venkatasubramanian

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git