[FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

Stores the item ids of all the items found in the file and
processes the primary item at the end of the meta box. This patch
does not change any behavior. It sets up the code for parsing
alpha channel (and possibly images with 'grid') in follow up
patches.

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/isom.h |   4 ++
 libavformat/mov.c  | 148 ++++++++++++++++++++++++++++-----------------
 2 files changed, 97 insertions(+), 55 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index f05c2d9c28..d8b262e915 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -318,6 +318,10 @@ typedef struct MOVContext {
     uint32_t max_stts_delta;
     int is_still_picture_avif;
     int primary_item_id;
+    int *avif_item_ids;
+    int avif_item_ids_size;
+    int *avif_extent_lengths;
+    int64_t *avif_extent_offsets;
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 88669faa70..9df5055d4e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4683,6 +4683,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return ret;
 }
 
+static int avif_add_stream(MOVContext *c, int item_id)
+{
+    MOVStreamContext *sc;
+    AVStream *st;
+    int item_index = -1;
+    for (int i = 0; i < c->avif_item_ids_size; i++)
+        if (c->avif_item_ids[i] == item_id) {
+            item_index = i;
+            break;
+        }
+    if (item_index < 0)
+        return AVERROR_INVALIDDATA;
+    st = avformat_new_stream(c->fc, NULL);
+    if (!st)
+        return AVERROR(ENOMEM);
+    st->id = c->fc->nb_streams;
+    sc = av_mallocz(sizeof(MOVStreamContext));
+    if (!sc)
+        return AVERROR(ENOMEM);
+
+    st->priv_data = sc;
+    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
+    st->codecpar->codec_id = AV_CODEC_ID_AV1;
+    sc->ffindex = st->index;
+    c->trak_index = st->index;
+    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
+    st->time_base.num = st->time_base.den = 1;
+    st->nb_frames = 1;
+    sc->time_scale = 1;
+    sc = st->priv_data;
+    sc->pb = c->fc->pb;
+    sc->pb_is_copied = 1;
+
+    // Populate the necessary fields used by mov_build_index.
+    sc->stsc_count = 1;
+    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
+    if (!sc->stsc_data)
+        return AVERROR(ENOMEM);
+    sc->stsc_data[0].first = 1;
+    sc->stsc_data[0].count = 1;
+    sc->stsc_data[0].id = 1;
+    sc->chunk_count = 1;
+    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
+    if (!sc->chunk_offsets)
+        return AVERROR(ENOMEM);
+    sc->sample_count = 1;
+    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
+    if (!sc->sample_sizes)
+        return AVERROR(ENOMEM);
+    sc->stts_count = 1;
+    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
+    if (!sc->stts_data)
+        return AVERROR(ENOMEM);
+    sc->stts_data[0].count = 1;
+    // Not used for still images. But needed by mov_build_index.
+    sc->stts_data[0].duration = 0;
+    sc->sample_sizes[0] = c->avif_extent_lengths[item_index];
+    sc->chunk_offsets[0] = c->avif_extent_offsets[item_index];
+
+    mov_build_index(c, st);
+    return 0;
+}
+
 static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     while (atom.size > 8) {
@@ -4692,9 +4755,25 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         tag = avio_rl32(pb);
         atom.size -= 4;
         if (tag == MKTAG('h','d','l','r')) {
+            int ret;
             avio_seek(pb, -8, SEEK_CUR);
             atom.size += 8;
-            return mov_read_default(c, pb, atom);
+            ret = mov_read_default(c, pb, atom);
+            if (ret < 0)
+                return ret;
+            if (c->is_still_picture_avif) {
+                int ret;
+                // Add a stream for the YUV planes (primary item).
+                ret = avif_add_stream(c, c->primary_item_id);
+                if (ret)
+                    return ret;
+                // For still AVIF images, the meta box contains all the
+                // necessary information that would generally be provided by the
+                // moov box. So simply mark that we have found the moov box so
+                // that parsing can continue.
+                c->found_moov = 1;
+            }
+            return ret;
         }
     }
     return 0;
@@ -7483,8 +7562,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     int item_count, extent_count;
     uint64_t base_offset, extent_offset, extent_length;
     uint8_t value;
-    AVStream *st;
-    MOVStreamContext *sc;
 
     if (!c->is_still_picture_avif) {
         // * For non-avif, we simply ignore the iloc box.
@@ -7498,27 +7575,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return 0;
     }
 
-    st = avformat_new_stream(c->fc, NULL);
-    if (!st)
-        return AVERROR(ENOMEM);
-    st->id = c->fc->nb_streams;
-    sc = av_mallocz(sizeof(MOVStreamContext));
-    if (!sc)
-        return AVERROR(ENOMEM);
-
-    st->priv_data = sc;
-    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id = AV_CODEC_ID_AV1;
-    sc->ffindex = st->index;
-    c->trak_index = st->index;
-    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
-    st->time_base.num = st->time_base.den = 1;
-    st->nb_frames = 1;
-    sc->time_scale = 1;
-    sc = st->priv_data;
-    sc->pb = c->fc->pb;
-    sc->pb_is_copied = 1;
-
     version = avio_r8(pb);
     avio_rb24(pb);  // flags.
 
@@ -7534,32 +7590,20 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     }
     item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
 
-    // Populate the necessary fields used by mov_build_index.
-    sc->stsc_count = 1;
-    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
-    if (!sc->stsc_data)
+    c->avif_item_ids = av_malloc_array(item_count, sizeof(*c->avif_item_ids));
+    if (!c->avif_item_ids)
         return AVERROR(ENOMEM);
-    sc->stsc_data[0].first = 1;
-    sc->stsc_data[0].count = 1;
-    sc->stsc_data[0].id = 1;
-    sc->chunk_count = 1;
-    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
-    if (!sc->chunk_offsets)
+    c->avif_item_ids_size = item_count;
+    c->avif_extent_lengths = av_malloc_array(item_count, sizeof(*c->avif_extent_lengths));
+    if (!c->avif_extent_lengths)
         return AVERROR(ENOMEM);
-    sc->sample_count = 1;
-    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
-    if (!sc->sample_sizes)
+    c->avif_extent_offsets = av_malloc_array(item_count, sizeof(*c->avif_extent_offsets));
+    if (!c->avif_extent_offsets)
         return AVERROR(ENOMEM);
-    sc->stts_count = 1;
-    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
-    if (!sc->stts_data)
-        return AVERROR(ENOMEM);
-    sc->stts_data[0].count = 1;
-    // Not used for still images. But needed by mov_build_index.
-    sc->stts_data[0].duration = 0;
 
     for (int i = 0; i < item_count; i++) {
         int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
+        c->avif_item_ids[i] = item_id;
         if (version > 0)
             avio_rb16(pb);  // construction_method.
         avio_rb16(pb);  // data_reference_index.
@@ -7575,20 +7619,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             if (rb_size(pb, &extent_offset, offset_size) < 0 ||
                 rb_size(pb, &extent_length, length_size) < 0)
                 return AVERROR_INVALIDDATA;
-            if (item_id == c->primary_item_id) {
-                sc->sample_sizes[0] = extent_length;
-                sc->chunk_offsets[0] = base_offset + extent_offset;
-            }
+            c->avif_extent_lengths[i] = extent_length;
+            c->avif_extent_offsets[i] = base_offset + extent_offset;
         }
     }
 
-    mov_build_index(c, st);
-
-    // For still AVIF images, the iloc box contains all the necessary
-    // information that would generally be provided by the moov box. So simply
-    // mark that we have found the moov box so that parsing can continue.
-    c->found_moov = 1;
-
     return atom.size;
 }
 
@@ -8192,6 +8227,9 @@ static int mov_read_close(AVFormatContext *s)
 
     av_freep(&mov->aes_decrypt);
     av_freep(&mov->chapter_tracks);
+    av_freep(&mov->avif_item_ids);
+    av_freep(&mov->avif_extent_lengths);
+    av_freep(&mov->avif_extent_offsets);
 
     return 0;
 }
-- 
2.37.0.rc0.161.g10f37bed90-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] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Vignesh Venkatasubramanian]

Parse the alpha channel for still AVIF images and expose it as a
separate track. This is the simplest way of supporting AVIF alpha
channel in a codec independent manner (similar to how ffmpeg
supports animated AVIF with alpha channel).

One can use the alphamerge filter to get a transparent image with
a single command. For example:

ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/isom.h |  1 +
 libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index d8b262e915..62b95b40ff 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -318,6 +318,7 @@ typedef struct MOVContext {
     uint32_t max_stts_delta;
     int is_still_picture_avif;
     int primary_item_id;
+    int alpha_item_id;
     int *avif_item_ids;
     int avif_item_ids_size;
     int *avif_extent_lengths;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9df5055d4e..72b17b618d 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4758,6 +4758,7 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             int ret;
             avio_seek(pb, -8, SEEK_CUR);
             atom.size += 8;
+            c->alpha_item_id = -1;
             ret = mov_read_default(c, pb, atom);
             if (ret < 0)
                 return ret;
@@ -4767,6 +4768,12 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
                 ret = avif_add_stream(c, c->primary_item_id);
                 if (ret)
                     return ret;
+                if (c->alpha_item_id != -1) {
+                    // Add a stream for the Alpha plane.
+                    ret = avif_add_stream(c, c->alpha_item_id);
+                    if (ret)
+                        return ret;
+                }
                 // For still AVIF images, the meta box contains all the
                 // necessary information that would generally be provided by the
                 // moov box. So simply mark that we have found the moov box so
@@ -7556,6 +7563,64 @@ static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return atom.size;
 }
 
+static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+    int entry_count, size, version, flags;
+    int index = 0, auxC_alpha_index = -1;
+    size = avio_rb32(pb);
+    if (avio_rl32(pb) != MKTAG('i','p','c','o'))
+        return AVERROR_INVALIDDATA;
+    size -= 8;
+    while (size > 0) {
+        int sub_size, sub_type;
+        sub_size = avio_rb32(pb);
+        sub_type = avio_rl32(pb);
+        sub_size -= 8;
+        size -= sub_size + 8;
+        index++;
+        if (sub_type == MKTAG('a','u','x','C')) {
+            const char *expected_alpha_urn = "urn:mpeg:mpegB:cicp:systems:auxiliary:alpha";
+            avio_rb32(pb);  // version & flags.
+            sub_size -= 4;
+            if (sub_size >= strlen(expected_alpha_urn) + 1) {
+                char alpha_urn[44];
+                avio_read(pb, alpha_urn, 44);
+                sub_size -= 44;
+                if (!strncmp(alpha_urn, expected_alpha_urn, 44)) {
+                    auxC_alpha_index = index;
+                }
+            }
+        }
+        avio_skip(pb, sub_size);
+    }
+    if (auxC_alpha_index == -1)
+        return atom.size;
+
+    // ipma.
+    size = avio_rb32(pb);
+    if (avio_rl32(pb) != MKTAG('i','p','m','a'))
+        return AVERROR_INVALIDDATA;
+    version = avio_r8(pb);
+    flags = avio_rb24(pb);
+    entry_count = avio_rb32(pb);
+    for (int i = 0; i < entry_count; i++) {
+        int item_id, association_count;
+        item_id = (version < 1) ? avio_rb16(pb) : avio_rb32(pb);
+        association_count = avio_r8(pb);
+        for (int j = 0; j < association_count; j++) {
+            int property_index;
+            if (flags & 1)
+                property_index = avio_rb16(pb) & 0x7fff;
+            else
+                property_index = avio_r8(pb) & 0x7f;
+            if (property_index == auxC_alpha_index) {
+                c->alpha_item_id = item_id;
+            }
+        }
+    }
+    return atom.size;
+}
+
 static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     int version, offset_size, length_size, base_offset_size, index_size;
@@ -7732,6 +7797,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('i','l','o','c'), mov_read_iloc },
 { MKTAG('p','c','m','C'), mov_read_pcmc }, /* PCM configuration box */
 { MKTAG('p','i','t','m'), mov_read_pitm },
+{ MKTAG('i','p','r','p'), mov_read_iprp },
 { 0, NULL }
 };
 
-- 
2.37.0.rc0.161.g10f37bed90-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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Anton Khirnov]

Quoting Vignesh Venkatasubramanian (2022-06-30 23:04:34)
> Parse the alpha channel for still AVIF images and expose it as a
> separate track. This is the simplest way of supporting AVIF alpha
> channel in a codec independent manner (similar to how ffmpeg
> supports animated AVIF with alpha channel).
> 
> One can use the alphamerge filter to get a transparent image with
> a single command. For example:
> 
> ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png
> 
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
>  libavformat/isom.h |  1 +
>  libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)

I am against this patch, this is a digusting hack.

These are not two streams, it is a single stream and should be exposed
as such.

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Vignesh Venkatasubramanian]

On Sat, Jul 2, 2022 at 2:35 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Vignesh Venkatasubramanian (2022-06-30 23:04:34)
> > Parse the alpha channel for still AVIF images and expose it as a
> > separate track. This is the simplest way of supporting AVIF alpha
> > channel in a codec independent manner (similar to how ffmpeg
> > supports animated AVIF with alpha channel).
> >
> > One can use the alphamerge filter to get a transparent image with
> > a single command. For example:
> >
> > ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> >  libavformat/isom.h |  1 +
> >  libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 67 insertions(+)
>
> I am against this patch, this is a digusting hack.
>
> These are not two streams, it is a single stream and should be exposed
> as such.
>

Yes, while it is a hack, it is also the simplest way of supporting
AVIF images with alpha. Since AVIF alpha images require two separate
decoders, i could not think of any other solution that does not
involve modifying the underlying av1 codec itself.

If there are any alternative solutions where we can expose a single
stream that needs two codecs, then i am open to implementing that.

Otherwise, this solution improves the current situation where there is
no way for the user to extract the alpha channel (albeit in a hacky
way).

> --
> Anton Khirnov



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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Jan Ekström]

On Sat, Jul 2, 2022 at 7:32 PM Vignesh Venkatasubramanian
<vigneshv-at-google.com@ffmpeg.org> wrote:
>
> On Sat, Jul 2, 2022 at 2:35 AM Anton Khirnov <anton@khirnov.net> wrote:
> >
> > Quoting Vignesh Venkatasubramanian (2022-06-30 23:04:34)
> > > Parse the alpha channel for still AVIF images and expose it as a
> > > separate track. This is the simplest way of supporting AVIF alpha
> > > channel in a codec independent manner (similar to how ffmpeg
> > > supports animated AVIF with alpha channel).
> > >
> > > One can use the alphamerge filter to get a transparent image with
> > > a single command. For example:
> > >
> > > ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png
> > >
> > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > ---
> > >  libavformat/isom.h |  1 +
> > >  libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 67 insertions(+)
> >
> > I am against this patch, this is a digusting hack.
> >
> > These are not two streams, it is a single stream and should be exposed
> > as such.
> >
>
> Yes, while it is a hack, it is also the simplest way of supporting
> AVIF images with alpha. Since AVIF alpha images require two separate
> decoders, i could not think of any other solution that does not
> involve modifying the underlying av1 codec itself.
>
> If there are any alternative solutions where we can expose a single
> stream that needs two codecs, then i am open to implementing that.
>
> Otherwise, this solution improves the current situation where there is
> no way for the user to extract the alpha channel (albeit in a hacky
> way).

I have discussed this on IRC, and as far as I can see we have multiple
cases where an additional image has to be decoded by a separate
decoder context to get alpha, namely:

- vp8/9 (currently handled by AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL)
- HEIF-like formats (such as AVIF), which are codec agnostic (you can
put JPEG, HEVC, AV1 etc into HEIF)

This means the logic (preferably) should not be codec-specific, and
would allow to implement at least two separate

My proposal - which Anton didn't really like - was to add a side data
entry AV_PKT_DATA_AUXILIARY_IMAGE , which could have type (ALPHA,
DEPTH etc) as well as the relevant packet(s) - as well as if any of
specifications define codec initialization data, that. Then
libavcodec/decode or so could initialize a secondary decoder context
for alpha images and add the third plane if parameters match what's
required.

This would also make remux possible, since containers that do not
support such auxiliary images would ignore the alpha plane, and those
that do would properly pass it through.

As for encoding, not fully sure how it should be integrated, if any
encoders actually at this moment do proper alpha coding, or do all API
clients have to separately encode with one context the primary image,
and the alpha with another? That said, the multi-stream implementation
is already merged on the muxer side, so that can be utilized in the
mean time, even though the auxiliary images are not technically a
separate stream within HEIF-likes.

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Vignesh Venkatasubramanian]

On Sat, Jul 2, 2022 at 12:35 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Sat, Jul 2, 2022 at 7:32 PM Vignesh Venkatasubramanian
> <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > On Sat, Jul 2, 2022 at 2:35 AM Anton Khirnov <anton@khirnov.net> wrote:
> > >
> > > Quoting Vignesh Venkatasubramanian (2022-06-30 23:04:34)
> > > > Parse the alpha channel for still AVIF images and expose it as a
> > > > separate track. This is the simplest way of supporting AVIF alpha
> > > > channel in a codec independent manner (similar to how ffmpeg
> > > > supports animated AVIF with alpha channel).
> > > >
> > > > One can use the alphamerge filter to get a transparent image with
> > > > a single command. For example:
> > > >
> > > > ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png
> > > >
> > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > > ---
> > > >  libavformat/isom.h |  1 +
> > > >  libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 67 insertions(+)
> > >
> > > I am against this patch, this is a digusting hack.
> > >
> > > These are not two streams, it is a single stream and should be exposed
> > > as such.
> > >
> >
> > Yes, while it is a hack, it is also the simplest way of supporting
> > AVIF images with alpha. Since AVIF alpha images require two separate
> > decoders, i could not think of any other solution that does not
> > involve modifying the underlying av1 codec itself.
> >
> > If there are any alternative solutions where we can expose a single
> > stream that needs two codecs, then i am open to implementing that.
> >
> > Otherwise, this solution improves the current situation where there is
> > no way for the user to extract the alpha channel (albeit in a hacky
> > way).
>
> I have discussed this on IRC,

Thank you for discussing this!

> and as far as I can see we have multiple
> cases where an additional image has to be decoded by a separate
> decoder context to get alpha, namely:
>
> - vp8/9 (currently handled by AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL)
> - HEIF-like formats (such as AVIF), which are codec agnostic (you can
> put JPEG, HEVC, AV1 etc into HEIF)
>
> This means the logic (preferably) should not be codec-specific, and
> would allow to implement at least two separate
>
> My proposal - which Anton didn't really like - was to add a side data
> entry AV_PKT_DATA_AUXILIARY_IMAGE , which could have type (ALPHA,
> DEPTH etc) as well as the relevant packet(s) - as well as if any of
> specifications define codec initialization data, that. Then
> libavcodec/decode or so could initialize a secondary decoder context
> for alpha images and add the third plane if parameters match what's
> required.
>

My only concern about this solution is that, while the demuxer can
easily put the alpha data in the side data, we will then have to
modify the decoder (in case of HEIF, we will have to change each
underlying codec like av1, hevc, jpeg, etc) to support handling of
this side data. I was just hoping it would be nicer if we can have
something simpler without having to change every individual libavcodec
implementation. ffmpeg already has a way to cleanly initialize codecs
and pass data to them, so it would be nicer if we can leverage that
somehow instead of making each libavcodec implementation handle this
side data type.

For example, the proposed solution is what we do for the VP8 alpha
demuxing and there is already some fragmentation. The libvpx based vp8
decoder supports alpha channel decoding while the native ffmpeg vp8
decoder does not support it. So, a user has to explicitly pass -c:v
libvpx-vp8 to be able to decode files with alpha channel properly.

> This would also make remux possible, since containers that do not
> support such auxiliary images would ignore the alpha plane, and those
> that do would properly pass it through.
>

The remux is possible in the multi-stream approach/hack as well. But I
do see your point.

> As for encoding, not fully sure how it should be integrated, if any
> encoders actually at this moment do proper alpha coding, or do all API
> clients have to separately encode with one context the primary image,
> and the alpha with another?

I am not sure about other codecs, but in the case of AVIF/AV1, the
encoder does not understand/support alpha channels. The only way to do
it is to use two separate encoders.

> That said, the multi-stream implementation
> is already merged on the muxer side, so that can be utilized in the
> mean time, even though the auxiliary images are not technically a
> separate stream within HEIF-likes.
>
> Jan
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Jan Ekström]

On Sun, Jul 3, 2022 at 12:15 AM Vignesh Venkatasubramanian
<vigneshv-at-google.com@ffmpeg.org> wrote:
>
> On Sat, Jul 2, 2022 at 12:35 PM Jan Ekström <jeebjp@gmail.com> wrote:
> >
> > On Sat, Jul 2, 2022 at 7:32 PM Vignesh Venkatasubramanian
> > <vigneshv-at-google.com@ffmpeg.org> wrote:
> > >
> > > On Sat, Jul 2, 2022 at 2:35 AM Anton Khirnov <anton@khirnov.net> wrote:
> > > >
> > > > Quoting Vignesh Venkatasubramanian (2022-06-30 23:04:34)
> > > > > Parse the alpha channel for still AVIF images and expose it as a
> > > > > separate track. This is the simplest way of supporting AVIF alpha
> > > > > channel in a codec independent manner (similar to how ffmpeg
> > > > > supports animated AVIF with alpha channel).
> > > > >
> > > > > One can use the alphamerge filter to get a transparent image with
> > > > > a single command. For example:
> > > > >
> > > > > ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png
> > > > >
> > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > > > ---
> > > > >  libavformat/isom.h |  1 +
> > > > >  libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  2 files changed, 67 insertions(+)
> > > >
> > > > I am against this patch, this is a digusting hack.
> > > >
> > > > These are not two streams, it is a single stream and should be exposed
> > > > as such.
> > > >
> > >
> > > Yes, while it is a hack, it is also the simplest way of supporting
> > > AVIF images with alpha. Since AVIF alpha images require two separate
> > > decoders, i could not think of any other solution that does not
> > > involve modifying the underlying av1 codec itself.
> > >
> > > If there are any alternative solutions where we can expose a single
> > > stream that needs two codecs, then i am open to implementing that.
> > >
> > > Otherwise, this solution improves the current situation where there is
> > > no way for the user to extract the alpha channel (albeit in a hacky
> > > way).
> >
> > I have discussed this on IRC,
>
> Thank you for discussing this!
>
> > and as far as I can see we have multiple
> > cases where an additional image has to be decoded by a separate
> > decoder context to get alpha, namely:
> >
> > - vp8/9 (currently handled by AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL)
> > - HEIF-like formats (such as AVIF), which are codec agnostic (you can
> > put JPEG, HEVC, AV1 etc into HEIF)
> >
> > This means the logic (preferably) should not be codec-specific, and
> > would allow to implement at least two separate
> >
> > My proposal - which Anton didn't really like - was to add a side data
> > entry AV_PKT_DATA_AUXILIARY_IMAGE , which could have type (ALPHA,
> > DEPTH etc) as well as the relevant packet(s) - as well as if any of
> > specifications define codec initialization data, that. Then
> > libavcodec/decode or so could initialize a secondary decoder context
> > for alpha images and add the third plane if parameters match what's
> > required.
> >
>
> My only concern about this solution is that, while the demuxer can
> easily put the alpha data in the side data, we will then have to
> modify the decoder (in case of HEIF, we will have to change each
> underlying codec like av1, hevc, jpeg, etc) to support handling of
> this side data. I was just hoping it would be nicer if we can have
> something simpler without having to change every individual libavcodec
> implementation. ffmpeg already has a way to cleanly initialize codecs
> and pass data to them, so it would be nicer if we can leverage that
> somehow instead of making each libavcodec implementation handle this
> side data type.

Yes This is why I specifically noted that general decode logic should
be modified, not separate decoders. Just like the recent ICC profile
<-> color space information patch set by Niklas.

It is clearly a thing that is going to get utilized by multiple
things, so having it specific to specific codecs makes no sense :) .

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Vignesh Venkatasubramanian]

On Sun, Jul 3, 2022 at 5:18 AM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Sun, Jul 3, 2022 at 12:15 AM Vignesh Venkatasubramanian
> <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > On Sat, Jul 2, 2022 at 12:35 PM Jan Ekström <jeebjp@gmail.com> wrote:
> > >
> > > On Sat, Jul 2, 2022 at 7:32 PM Vignesh Venkatasubramanian
> > > <vigneshv-at-google.com@ffmpeg.org> wrote:
> > > >
> > > > On Sat, Jul 2, 2022 at 2:35 AM Anton Khirnov <anton@khirnov.net> wrote:
> > > > >
> > > > > Quoting Vignesh Venkatasubramanian (2022-06-30 23:04:34)
> > > > > > Parse the alpha channel for still AVIF images and expose it as a
> > > > > > separate track. This is the simplest way of supporting AVIF alpha
> > > > > > channel in a codec independent manner (similar to how ffmpeg
> > > > > > supports animated AVIF with alpha channel).
> > > > > >
> > > > > > One can use the alphamerge filter to get a transparent image with
> > > > > > a single command. For example:
> > > > > >
> > > > > > ffmpeg -i image_with_alpha.avif -filter_complex alphamerge image_with_alpha.png
> > > > > >
> > > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > > > > ---
> > > > > >  libavformat/isom.h |  1 +
> > > > > >  libavformat/mov.c  | 66 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  2 files changed, 67 insertions(+)
> > > > >
> > > > > I am against this patch, this is a digusting hack.
> > > > >
> > > > > These are not two streams, it is a single stream and should be exposed
> > > > > as such.
> > > > >
> > > >
> > > > Yes, while it is a hack, it is also the simplest way of supporting
> > > > AVIF images with alpha. Since AVIF alpha images require two separate
> > > > decoders, i could not think of any other solution that does not
> > > > involve modifying the underlying av1 codec itself.
> > > >
> > > > If there are any alternative solutions where we can expose a single
> > > > stream that needs two codecs, then i am open to implementing that.
> > > >
> > > > Otherwise, this solution improves the current situation where there is
> > > > no way for the user to extract the alpha channel (albeit in a hacky
> > > > way).
> > >
> > > I have discussed this on IRC,
> >
> > Thank you for discussing this!
> >
> > > and as far as I can see we have multiple
> > > cases where an additional image has to be decoded by a separate
> > > decoder context to get alpha, namely:
> > >
> > > - vp8/9 (currently handled by AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL)
> > > - HEIF-like formats (such as AVIF), which are codec agnostic (you can
> > > put JPEG, HEVC, AV1 etc into HEIF)
> > >
> > > This means the logic (preferably) should not be codec-specific, and
> > > would allow to implement at least two separate
> > >
> > > My proposal - which Anton didn't really like - was to add a side data
> > > entry AV_PKT_DATA_AUXILIARY_IMAGE , which could have type (ALPHA,
> > > DEPTH etc) as well as the relevant packet(s) - as well as if any of
> > > specifications define codec initialization data, that. Then
> > > libavcodec/decode or so could initialize a secondary decoder context
> > > for alpha images and add the third plane if parameters match what's
> > > required.
> > >
> >
> > My only concern about this solution is that, while the demuxer can
> > easily put the alpha data in the side data, we will then have to
> > modify the decoder (in case of HEIF, we will have to change each
> > underlying codec like av1, hevc, jpeg, etc) to support handling of
> > this side data. I was just hoping it would be nicer if we can have
> > something simpler without having to change every individual libavcodec
> > implementation. ffmpeg already has a way to cleanly initialize codecs
> > and pass data to them, so it would be nicer if we can leverage that
> > somehow instead of making each libavcodec implementation handle this
> > side data type.
>
> Yes This is why I specifically noted that general decode logic should
> be modified, not separate decoders. Just like the recent ICC profile
> <-> color space information patch set by Niklas.
>

Ah okay, i see what you are saying now. This makes sense. I am also in
favor of this solution then.

We can abandon this second patch for now and i will see if i can
implement the side data based solution.

The first patch in this series is still a refactor that will be useful
for the future. So i will leave that open for review.

> It is clearly a thing that is going to get utilized by multiple
> things, so having it specific to specific codecs makes no sense :) .

Yes, sounds good!

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Anton Khirnov]

Quoting Vignesh Venkatasubramanian (2022-07-02 23:15:35)
> > As for encoding, not fully sure how it should be integrated, if any
> > encoders actually at this moment do proper alpha coding, or do all API
> > clients have to separately encode with one context the primary image,
> > and the alpha with another?
> 
> I am not sure about other codecs, but in the case of AVIF/AV1, the
> encoder does not understand/support alpha channels. The only way to do
> it is to use two separate encoders.

Can this not be handled in our encoder wrapper?

We should strive as much as possible to shield our callers from
codec-specific implementation details.

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

Re: [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel

[Vignesh Venkatasubramanian]

On Tue, Jul 5, 2022 at 9:54 AM Anton Khirnov <anton@khirnov.net> wrote:
>
> Quoting Vignesh Venkatasubramanian (2022-07-02 23:15:35)
> > > As for encoding, not fully sure how it should be integrated, if any
> > > encoders actually at this moment do proper alpha coding, or do all API
> > > clients have to separately encode with one context the primary image,
> > > and the alpha with another?
> >
> > I am not sure about other codecs, but in the case of AVIF/AV1, the
> > encoder does not understand/support alpha channels. The only way to do
> > it is to use two separate encoders.
>
> Can this not be handled in our encoder wrapper?
>

By encoder wrapper, you mean codec wrappers in libavcodec like
libaomenc.c right ? Yes, it is possible to do this in the encoder
wrapper by keeping multiple instances of the encoder. But each encoder
wrapper has to be updated to support it. AV1, for example, has three
different encoders that can be used as of today (aom, rav1e and
svt-av1).

> We should strive as much as possible to shield our callers from
> codec-specific implementation details.
>
> --
> Anton Khirnov
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[James Zern]

On Thu, Jun 30, 2022 at 2:04 PM Vignesh Venkatasubramanian
<vigneshv-at-google.com@ffmpeg.org> wrote:
>
> Stores the item ids of all the items found in the file and
> processes the primary item at the end of the meta box. This patch
> does not change any behavior. It sets up the code for parsing
> alpha channel (and possibly images with 'grid') in follow up
> patches.
>
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
>  libavformat/isom.h |   4 ++
>  libavformat/mov.c  | 148 ++++++++++++++++++++++++++++-----------------
>  2 files changed, 97 insertions(+), 55 deletions(-)
>
> [...]

@@ -4692,9 +4755,25 @@ static int mov_read_meta(MOVContext *c,
AVIOContext *pb, MOVAtom atom)
         tag = avio_rl32(pb);
         atom.size -= 4;
         if (tag == MKTAG('h','d','l','r')) {
+            int ret;
             avio_seek(pb, -8, SEEK_CUR);
             atom.size += 8;
-            return mov_read_default(c, pb, atom);
+            ret = mov_read_default(c, pb, atom);
+            if (ret < 0)

In some other cases these two lines are combined, if ((ret = ...

+                return ret;
+            if (c->is_still_picture_avif) {
+                int ret;
+                // Add a stream for the YUV planes (primary item).
+                ret = avif_add_stream(c, c->primary_item_id);
+                if (ret)

This could be updated too and use '< 0' to match other code.

+                    return ret;
+                // For still AVIF images, the meta box contains all the
+                // necessary information that would generally be
provided by the
+                // moov box. So simply mark that we have found the moov box so
+                // that parsing can continue.
+                c->found_moov = 1;
+            }
+            return ret;
         }
_______________________________________________
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] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

Stores the item ids of all the items found in the file and
processes the primary item at the end of the meta box. This patch
does not change any behavior. It sets up the code for parsing
alpha channel (and possibly images with 'grid') in follow up
patches.

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/isom.h |   4 ++
 libavformat/mov.c  | 146 ++++++++++++++++++++++++++++-----------------
 2 files changed, 95 insertions(+), 55 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index f05c2d9c28..d8b262e915 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -318,6 +318,10 @@ typedef struct MOVContext {
     uint32_t max_stts_delta;
     int is_still_picture_avif;
     int primary_item_id;
+    int *avif_item_ids;
+    int avif_item_ids_size;
+    int *avif_extent_lengths;
+    int64_t *avif_extent_offsets;
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 88669faa70..cd87088f3e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4683,6 +4683,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return ret;
 }
 
+static int avif_add_stream(MOVContext *c, int item_id)
+{
+    MOVStreamContext *sc;
+    AVStream *st;
+    int item_index = -1;
+    for (int i = 0; i < c->avif_item_ids_size; i++)
+        if (c->avif_item_ids[i] == item_id) {
+            item_index = i;
+            break;
+        }
+    if (item_index < 0)
+        return AVERROR_INVALIDDATA;
+    st = avformat_new_stream(c->fc, NULL);
+    if (!st)
+        return AVERROR(ENOMEM);
+    st->id = c->fc->nb_streams;
+    sc = av_mallocz(sizeof(MOVStreamContext));
+    if (!sc)
+        return AVERROR(ENOMEM);
+
+    st->priv_data = sc;
+    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
+    st->codecpar->codec_id = AV_CODEC_ID_AV1;
+    sc->ffindex = st->index;
+    c->trak_index = st->index;
+    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
+    st->time_base.num = st->time_base.den = 1;
+    st->nb_frames = 1;
+    sc->time_scale = 1;
+    sc = st->priv_data;
+    sc->pb = c->fc->pb;
+    sc->pb_is_copied = 1;
+
+    // Populate the necessary fields used by mov_build_index.
+    sc->stsc_count = 1;
+    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
+    if (!sc->stsc_data)
+        return AVERROR(ENOMEM);
+    sc->stsc_data[0].first = 1;
+    sc->stsc_data[0].count = 1;
+    sc->stsc_data[0].id = 1;
+    sc->chunk_count = 1;
+    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
+    if (!sc->chunk_offsets)
+        return AVERROR(ENOMEM);
+    sc->sample_count = 1;
+    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
+    if (!sc->sample_sizes)
+        return AVERROR(ENOMEM);
+    sc->stts_count = 1;
+    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
+    if (!sc->stts_data)
+        return AVERROR(ENOMEM);
+    sc->stts_data[0].count = 1;
+    // Not used for still images. But needed by mov_build_index.
+    sc->stts_data[0].duration = 0;
+    sc->sample_sizes[0] = c->avif_extent_lengths[item_index];
+    sc->chunk_offsets[0] = c->avif_extent_offsets[item_index];
+
+    mov_build_index(c, st);
+    return 0;
+}
+
 static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     while (atom.size > 8) {
@@ -4692,9 +4755,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         tag = avio_rl32(pb);
         atom.size -= 4;
         if (tag == MKTAG('h','d','l','r')) {
+            int ret;
             avio_seek(pb, -8, SEEK_CUR);
             atom.size += 8;
-            return mov_read_default(c, pb, atom);
+            if ((ret = mov_read_default(c, pb, atom)) < 0)
+                return ret;
+            if (c->is_still_picture_avif) {
+                int ret;
+                // Add a stream for the YUV planes (primary item).
+                if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
+                    return ret;
+                // For still AVIF images, the meta box contains all the
+                // necessary information that would generally be provided by the
+                // moov box. So simply mark that we have found the moov box so
+                // that parsing can continue.
+                c->found_moov = 1;
+            }
+            return ret;
         }
     }
     return 0;
@@ -7483,8 +7560,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     int item_count, extent_count;
     uint64_t base_offset, extent_offset, extent_length;
     uint8_t value;
-    AVStream *st;
-    MOVStreamContext *sc;
 
     if (!c->is_still_picture_avif) {
         // * For non-avif, we simply ignore the iloc box.
@@ -7498,27 +7573,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return 0;
     }
 
-    st = avformat_new_stream(c->fc, NULL);
-    if (!st)
-        return AVERROR(ENOMEM);
-    st->id = c->fc->nb_streams;
-    sc = av_mallocz(sizeof(MOVStreamContext));
-    if (!sc)
-        return AVERROR(ENOMEM);
-
-    st->priv_data = sc;
-    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id = AV_CODEC_ID_AV1;
-    sc->ffindex = st->index;
-    c->trak_index = st->index;
-    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
-    st->time_base.num = st->time_base.den = 1;
-    st->nb_frames = 1;
-    sc->time_scale = 1;
-    sc = st->priv_data;
-    sc->pb = c->fc->pb;
-    sc->pb_is_copied = 1;
-
     version = avio_r8(pb);
     avio_rb24(pb);  // flags.
 
@@ -7534,32 +7588,20 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     }
     item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
 
-    // Populate the necessary fields used by mov_build_index.
-    sc->stsc_count = 1;
-    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
-    if (!sc->stsc_data)
+    c->avif_item_ids = av_malloc_array(item_count, sizeof(*c->avif_item_ids));
+    if (!c->avif_item_ids)
         return AVERROR(ENOMEM);
-    sc->stsc_data[0].first = 1;
-    sc->stsc_data[0].count = 1;
-    sc->stsc_data[0].id = 1;
-    sc->chunk_count = 1;
-    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
-    if (!sc->chunk_offsets)
+    c->avif_item_ids_size = item_count;
+    c->avif_extent_lengths = av_malloc_array(item_count, sizeof(*c->avif_extent_lengths));
+    if (!c->avif_extent_lengths)
         return AVERROR(ENOMEM);
-    sc->sample_count = 1;
-    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
-    if (!sc->sample_sizes)
+    c->avif_extent_offsets = av_malloc_array(item_count, sizeof(*c->avif_extent_offsets));
+    if (!c->avif_extent_offsets)
         return AVERROR(ENOMEM);
-    sc->stts_count = 1;
-    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
-    if (!sc->stts_data)
-        return AVERROR(ENOMEM);
-    sc->stts_data[0].count = 1;
-    // Not used for still images. But needed by mov_build_index.
-    sc->stts_data[0].duration = 0;
 
     for (int i = 0; i < item_count; i++) {
         int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
+        c->avif_item_ids[i] = item_id;
         if (version > 0)
             avio_rb16(pb);  // construction_method.
         avio_rb16(pb);  // data_reference_index.
@@ -7575,20 +7617,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             if (rb_size(pb, &extent_offset, offset_size) < 0 ||
                 rb_size(pb, &extent_length, length_size) < 0)
                 return AVERROR_INVALIDDATA;
-            if (item_id == c->primary_item_id) {
-                sc->sample_sizes[0] = extent_length;
-                sc->chunk_offsets[0] = base_offset + extent_offset;
-            }
+            c->avif_extent_lengths[i] = extent_length;
+            c->avif_extent_offsets[i] = base_offset + extent_offset;
         }
     }
 
-    mov_build_index(c, st);
-
-    // For still AVIF images, the iloc box contains all the necessary
-    // information that would generally be provided by the moov box. So simply
-    // mark that we have found the moov box so that parsing can continue.
-    c->found_moov = 1;
-
     return atom.size;
 }
 
@@ -8192,6 +8225,9 @@ static int mov_read_close(AVFormatContext *s)
 
     av_freep(&mov->aes_decrypt);
     av_freep(&mov->chapter_tracks);
+    av_freep(&mov->avif_item_ids);
+    av_freep(&mov->avif_extent_lengths);
+    av_freep(&mov->avif_extent_offsets);
 
     return 0;
 }
-- 
2.37.0.144.g8ac04bfd2-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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

On Mon, Jul 11, 2022 at 3:25 PM James Zern
<jzern-at-google.com@ffmpeg.org> wrote:
>
> On Thu, Jun 30, 2022 at 2:04 PM Vignesh Venkatasubramanian
> <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > Stores the item ids of all the items found in the file and
> > processes the primary item at the end of the meta box. This patch
> > does not change any behavior. It sets up the code for parsing
> > alpha channel (and possibly images with 'grid') in follow up
> > patches.
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> >  libavformat/isom.h |   4 ++
> >  libavformat/mov.c  | 148 ++++++++++++++++++++++++++++-----------------
> >  2 files changed, 97 insertions(+), 55 deletions(-)
> >
> > [...]
>
> @@ -4692,9 +4755,25 @@ static int mov_read_meta(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>          tag = avio_rl32(pb);
>          atom.size -= 4;
>          if (tag == MKTAG('h','d','l','r')) {
> +            int ret;
>              avio_seek(pb, -8, SEEK_CUR);
>              atom.size += 8;
> -            return mov_read_default(c, pb, atom);
> +            ret = mov_read_default(c, pb, atom);
> +            if (ret < 0)
>
> In some other cases these two lines are combined, if ((ret = ...
>

Done.

> +                return ret;
> +            if (c->is_still_picture_avif) {
> +                int ret;
> +                // Add a stream for the YUV planes (primary item).
> +                ret = avif_add_stream(c, c->primary_item_id);
> +                if (ret)
>
> This could be updated too and use '< 0' to match other code.
>

Done.

> +                    return ret;
> +                // For still AVIF images, the meta box contains all the
> +                // necessary information that would generally be
> provided by the
> +                // moov box. So simply mark that we have found the moov box so
> +                // that parsing can continue.
> +                c->found_moov = 1;
> +            }
> +            return ret;
>          }
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

On Wed, Jul 13, 2022 at 9:12 AM Vignesh Venkatasubramanian
<vigneshv@google.com> wrote:
>
> On Mon, Jul 11, 2022 at 3:25 PM James Zern
> <jzern-at-google.com@ffmpeg.org> wrote:
> >
> > On Thu, Jun 30, 2022 at 2:04 PM Vignesh Venkatasubramanian
> > <vigneshv-at-google.com@ffmpeg.org> wrote:
> > >
> > > Stores the item ids of all the items found in the file and
> > > processes the primary item at the end of the meta box. This patch
> > > does not change any behavior. It sets up the code for parsing
> > > alpha channel (and possibly images with 'grid') in follow up
> > > patches.
> > >
> > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > ---
> > >  libavformat/isom.h |   4 ++
> > >  libavformat/mov.c  | 148 ++++++++++++++++++++++++++++-----------------
> > >  2 files changed, 97 insertions(+), 55 deletions(-)
> > >
> > > [...]
> >
> > @@ -4692,9 +4755,25 @@ static int mov_read_meta(MOVContext *c,
> > AVIOContext *pb, MOVAtom atom)
> >          tag = avio_rl32(pb);
> >          atom.size -= 4;
> >          if (tag == MKTAG('h','d','l','r')) {
> > +            int ret;
> >              avio_seek(pb, -8, SEEK_CUR);
> >              atom.size += 8;
> > -            return mov_read_default(c, pb, atom);
> > +            ret = mov_read_default(c, pb, atom);
> > +            if (ret < 0)
> >
> > In some other cases these two lines are combined, if ((ret = ...
> >
>
> Done.
>
> > +                return ret;
> > +            if (c->is_still_picture_avif) {
> > +                int ret;
> > +                // Add a stream for the YUV planes (primary item).
> > +                ret = avif_add_stream(c, c->primary_item_id);
> > +                if (ret)
> >
> > This could be updated too and use '< 0' to match other code.
> >
>
> Done.
>
> > +                    return ret;
> > +                // For still AVIF images, the meta box contains all the
> > +                // necessary information that would generally be
> > provided by the
> > +                // moov box. So simply mark that we have found the moov box so
> > +                // that parsing can continue.
> > +                c->found_moov = 1;
> > +            }
> > +            return ret;
> >          }
> > _______________________________________________
> > 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

Any further comments on this one?

Please note that i have abandoned the second patch in this list. But
this one is still up for merging.

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[James Zern]

On Fri, Jul 22, 2022 at 11:21 AM Vignesh Venkatasubramanian
<vigneshv-at-google.com@ffmpeg.org> wrote:
>
> On Wed, Jul 13, 2022 at 9:12 AM Vignesh Venkatasubramanian
> <vigneshv@google.com> wrote:
> >
> > On Mon, Jul 11, 2022 at 3:25 PM James Zern
> > <jzern-at-google.com@ffmpeg.org> wrote:
> > >
> > > On Thu, Jun 30, 2022 at 2:04 PM Vignesh Venkatasubramanian
> > > <vigneshv-at-google.com@ffmpeg.org> wrote:
> > > >
> > > > Stores the item ids of all the items found in the file and
> > > > processes the primary item at the end of the meta box. This patch
> > > > does not change any behavior. It sets up the code for parsing
> > > > alpha channel (and possibly images with 'grid') in follow up
> > > > patches.
> > > >
> > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > > ---
> > > >  libavformat/isom.h |   4 ++
> > > >  libavformat/mov.c  | 148 ++++++++++++++++++++++++++++-----------------
> > > >  2 files changed, 97 insertions(+), 55 deletions(-)
> > > >
> > > > [...]
> > >
> > > @@ -4692,9 +4755,25 @@ static int mov_read_meta(MOVContext *c,
> > > AVIOContext *pb, MOVAtom atom)
> > >          tag = avio_rl32(pb);
> > >          atom.size -= 4;
> > >          if (tag == MKTAG('h','d','l','r')) {
> > > +            int ret;
> > >              avio_seek(pb, -8, SEEK_CUR);
> > >              atom.size += 8;
> > > -            return mov_read_default(c, pb, atom);
> > > +            ret = mov_read_default(c, pb, atom);
> > > +            if (ret < 0)
> > >
> > > In some other cases these two lines are combined, if ((ret = ...
> > >
> >
> > Done.
> >
> > > +                return ret;
> > > +            if (c->is_still_picture_avif) {
> > > +                int ret;
> > > +                // Add a stream for the YUV planes (primary item).
> > > +                ret = avif_add_stream(c, c->primary_item_id);
> > > +                if (ret)
> > >
> > > This could be updated too and use '< 0' to match other code.
> > >
> >
> > Done.
> >
> > > +                    return ret;
> > > +                // For still AVIF images, the meta box contains all the
> > > +                // necessary information that would generally be
> > > provided by the
> > > +                // moov box. So simply mark that we have found the moov box so
> > > +                // that parsing can continue.
> > > +                c->found_moov = 1;
> > > +            }
> > > +            return ret;
> > >          }
> > > _______________________________________________
> > > 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
>
> Any further comments on this one?
>
> Please note that i have abandoned the second patch in this list. But
> this one is still up for merging.
>

This looks like it needs to be rebased. I'll take a look after that.
_______________________________________________
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] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

Stores the item ids of all the items found in the file and
processes the primary item at the end of the meta box. This patch
does not change any behavior. It sets up the code for parsing
alpha channel (and possibly images with 'grid') in follow up
patches.

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/isom.h |   4 ++
 libavformat/mov.c  | 146 ++++++++++++++++++++++++++++-----------------
 2 files changed, 95 insertions(+), 55 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index f05c2d9c28..d8b262e915 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -318,6 +318,10 @@ typedef struct MOVContext {
     uint32_t max_stts_delta;
     int is_still_picture_avif;
     int primary_item_id;
+    int *avif_item_ids;
+    int avif_item_ids_size;
+    int *avif_extent_lengths;
+    int64_t *avif_extent_offsets;
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a09a762d91..fc6a691da4 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4698,6 +4698,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return ret;
 }
 
+static int avif_add_stream(MOVContext *c, int item_id)
+{
+    MOVStreamContext *sc;
+    AVStream *st;
+    int item_index = -1;
+    for (int i = 0; i < c->avif_item_ids_size; i++)
+        if (c->avif_item_ids[i] == item_id) {
+            item_index = i;
+            break;
+        }
+    if (item_index < 0)
+        return AVERROR_INVALIDDATA;
+    st = avformat_new_stream(c->fc, NULL);
+    if (!st)
+        return AVERROR(ENOMEM);
+    st->id = c->fc->nb_streams;
+    sc = av_mallocz(sizeof(MOVStreamContext));
+    if (!sc)
+        return AVERROR(ENOMEM);
+
+    st->priv_data = sc;
+    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
+    st->codecpar->codec_id = AV_CODEC_ID_AV1;
+    sc->ffindex = st->index;
+    c->trak_index = st->index;
+    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
+    st->time_base.num = st->time_base.den = 1;
+    st->nb_frames = 1;
+    sc->time_scale = 1;
+    sc = st->priv_data;
+    sc->pb = c->fc->pb;
+    sc->pb_is_copied = 1;
+
+    // Populate the necessary fields used by mov_build_index.
+    sc->stsc_count = 1;
+    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
+    if (!sc->stsc_data)
+        return AVERROR(ENOMEM);
+    sc->stsc_data[0].first = 1;
+    sc->stsc_data[0].count = 1;
+    sc->stsc_data[0].id = 1;
+    sc->chunk_count = 1;
+    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
+    if (!sc->chunk_offsets)
+        return AVERROR(ENOMEM);
+    sc->sample_count = 1;
+    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
+    if (!sc->sample_sizes)
+        return AVERROR(ENOMEM);
+    sc->stts_count = 1;
+    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
+    if (!sc->stts_data)
+        return AVERROR(ENOMEM);
+    sc->stts_data[0].count = 1;
+    // Not used for still images. But needed by mov_build_index.
+    sc->stts_data[0].duration = 0;
+    sc->sample_sizes[0] = c->avif_extent_lengths[item_index];
+    sc->chunk_offsets[0] = c->avif_extent_offsets[item_index];
+
+    mov_build_index(c, st);
+    return 0;
+}
+
 static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     while (atom.size > 8) {
@@ -4707,9 +4770,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         tag = avio_rl32(pb);
         atom.size -= 4;
         if (tag == MKTAG('h','d','l','r')) {
+            int ret;
             avio_seek(pb, -8, SEEK_CUR);
             atom.size += 8;
-            return mov_read_default(c, pb, atom);
+            if ((ret = mov_read_default(c, pb, atom)) < 0)
+                return ret;
+            if (c->is_still_picture_avif) {
+                int ret;
+                // Add a stream for the YUV planes (primary item).
+                if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
+                    return ret;
+                // For still AVIF images, the meta box contains all the
+                // necessary information that would generally be provided by the
+                // moov box. So simply mark that we have found the moov box so
+                // that parsing can continue.
+                c->found_moov = 1;
+            }
+            return ret;
         }
     }
     return 0;
@@ -7478,8 +7555,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     int item_count, extent_count;
     uint64_t base_offset, extent_offset, extent_length;
     uint8_t value;
-    AVStream *st;
-    MOVStreamContext *sc;
 
     if (!c->is_still_picture_avif) {
         // * For non-avif, we simply ignore the iloc box.
@@ -7493,27 +7568,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return 0;
     }
 
-    st = avformat_new_stream(c->fc, NULL);
-    if (!st)
-        return AVERROR(ENOMEM);
-    st->id = c->fc->nb_streams;
-    sc = av_mallocz(sizeof(MOVStreamContext));
-    if (!sc)
-        return AVERROR(ENOMEM);
-
-    st->priv_data = sc;
-    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id = AV_CODEC_ID_AV1;
-    sc->ffindex = st->index;
-    c->trak_index = st->index;
-    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
-    st->time_base.num = st->time_base.den = 1;
-    st->nb_frames = 1;
-    sc->time_scale = 1;
-    sc = st->priv_data;
-    sc->pb = c->fc->pb;
-    sc->pb_is_copied = 1;
-
     version = avio_r8(pb);
     avio_rb24(pb);  // flags.
 
@@ -7529,34 +7583,22 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     }
     item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
 
-    // Populate the necessary fields used by mov_build_index.
-    sc->stsc_count = 1;
-    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
-    if (!sc->stsc_data)
+    c->avif_item_ids = av_malloc_array(item_count, sizeof(*c->avif_item_ids));
+    if (!c->avif_item_ids)
         return AVERROR(ENOMEM);
-    sc->stsc_data[0].first = 1;
-    sc->stsc_data[0].count = 1;
-    sc->stsc_data[0].id = 1;
-    sc->chunk_count = 1;
-    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
-    if (!sc->chunk_offsets)
+    c->avif_item_ids_size = item_count;
+    c->avif_extent_lengths = av_malloc_array(item_count, sizeof(*c->avif_extent_lengths));
+    if (!c->avif_extent_lengths)
         return AVERROR(ENOMEM);
-    sc->sample_count = 1;
-    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
-    if (!sc->sample_sizes)
+    c->avif_extent_offsets = av_malloc_array(item_count, sizeof(*c->avif_extent_offsets));
+    if (!c->avif_extent_offsets)
         return AVERROR(ENOMEM);
-    sc->stts_count = 1;
-    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
-    if (!sc->stts_data)
-        return AVERROR(ENOMEM);
-    sc->stts_data[0].count = 1;
-    // Not used for still images. But needed by mov_build_index.
-    sc->stts_data[0].duration = 0;
 
     for (int i = 0; i < item_count; i++) {
         int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
         if (avio_feof(pb))
             return AVERROR_INVALIDDATA;
+        c->avif_item_ids[i] = item_id;
         if (version > 0)
             avio_rb16(pb);  // construction_method.
         avio_rb16(pb);  // data_reference_index.
@@ -7572,20 +7614,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             if (rb_size(pb, &extent_offset, offset_size) < 0 ||
                 rb_size(pb, &extent_length, length_size) < 0)
                 return AVERROR_INVALIDDATA;
-            if (item_id == c->primary_item_id) {
-                sc->sample_sizes[0] = extent_length;
-                sc->chunk_offsets[0] = base_offset + extent_offset;
-            }
+            c->avif_extent_lengths[i] = extent_length;
+            c->avif_extent_offsets[i] = base_offset + extent_offset;
         }
     }
 
-    mov_build_index(c, st);
-
-    // For still AVIF images, the iloc box contains all the necessary
-    // information that would generally be provided by the moov box. So simply
-    // mark that we have found the moov box so that parsing can continue.
-    c->found_moov = 1;
-
     return atom.size;
 }
 
@@ -8189,6 +8222,9 @@ static int mov_read_close(AVFormatContext *s)
 
     av_freep(&mov->aes_decrypt);
     av_freep(&mov->chapter_tracks);
+    av_freep(&mov->avif_item_ids);
+    av_freep(&mov->avif_extent_lengths);
+    av_freep(&mov->avif_extent_offsets);
 
     return 0;
 }
-- 
2.37.1.359.gd136c6c3e2-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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[James Zern]

On Wed, Jul 27, 2022 at 9:12 AM Vignesh Venkatasubramanian
<vigneshv-at-google.com@ffmpeg.org> wrote:
>
> Stores the item ids of all the items found in the file and
> processes the primary item at the end of the meta box. This patch
> does not change any behavior. It sets up the code for parsing
> alpha channel (and possibly images with 'grid') in follow up
> patches.
>
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
>  libavformat/isom.h |   4 ++
>  libavformat/mov.c  | 146 ++++++++++++++++++++++++++++-----------------
>  2 files changed, 95 insertions(+), 55 deletions(-)
>

lgtm. I'll submit this soon if there aren't any other comments.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Andreas Rheinhardt]

Vignesh Venkatasubramanian:
> Stores the item ids of all the items found in the file and
> processes the primary item at the end of the meta box. This patch
> does not change any behavior. It sets up the code for parsing
> alpha channel (and possibly images with 'grid') in follow up
> patches.
> 
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
>  libavformat/isom.h |   4 ++
>  libavformat/mov.c  | 146 ++++++++++++++++++++++++++++-----------------
>  2 files changed, 95 insertions(+), 55 deletions(-)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index f05c2d9c28..d8b262e915 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -318,6 +318,10 @@ typedef struct MOVContext {
>      uint32_t max_stts_delta;
>      int is_still_picture_avif;
>      int primary_item_id;
> +    int *avif_item_ids;
> +    int avif_item_ids_size;
> +    int *avif_extent_lengths;
> +    int64_t *avif_extent_offsets;

Why are these three different buffers instead of one buffer of struct {
int avif_item_ids; int avif_extent_lengths; int64_t avif_extent_offsets;}?

>  } MOVContext;
>  
>  int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index a09a762d91..fc6a691da4 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4698,6 +4698,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      return ret;
>  }
>  
> +static int avif_add_stream(MOVContext *c, int item_id)
> +{
> +    MOVStreamContext *sc;
> +    AVStream *st;
> +    int item_index = -1;
> +    for (int i = 0; i < c->avif_item_ids_size; i++)
> +        if (c->avif_item_ids[i] == item_id) {
> +            item_index = i;
> +            break;
> +        }
> +    if (item_index < 0)
> +        return AVERROR_INVALIDDATA;
> +    st = avformat_new_stream(c->fc, NULL);
> +    if (!st)
> +        return AVERROR(ENOMEM);
> +    st->id = c->fc->nb_streams;
> +    sc = av_mallocz(sizeof(MOVStreamContext));
> +    if (!sc)
> +        return AVERROR(ENOMEM);
> +
> +    st->priv_data = sc;
> +    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> +    st->codecpar->codec_id = AV_CODEC_ID_AV1;
> +    sc->ffindex = st->index;
> +    c->trak_index = st->index;
> +    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> +    st->time_base.num = st->time_base.den = 1;
> +    st->nb_frames = 1;
> +    sc->time_scale = 1;
> +    sc = st->priv_data;
> +    sc->pb = c->fc->pb;
> +    sc->pb_is_copied = 1;
> +
> +    // Populate the necessary fields used by mov_build_index.
> +    sc->stsc_count = 1;
> +    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> +    if (!sc->stsc_data)
> +        return AVERROR(ENOMEM);
> +    sc->stsc_data[0].first = 1;
> +    sc->stsc_data[0].count = 1;
> +    sc->stsc_data[0].id = 1;
> +    sc->chunk_count = 1;
> +    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> +    if (!sc->chunk_offsets)
> +        return AVERROR(ENOMEM);
> +    sc->sample_count = 1;
> +    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> +    if (!sc->sample_sizes)
> +        return AVERROR(ENOMEM);
> +    sc->stts_count = 1;
> +    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> +    if (!sc->stts_data)
> +        return AVERROR(ENOMEM);
> +    sc->stts_data[0].count = 1;
> +    // Not used for still images. But needed by mov_build_index.
> +    sc->stts_data[0].duration = 0;
> +    sc->sample_sizes[0] = c->avif_extent_lengths[item_index];
> +    sc->chunk_offsets[0] = c->avif_extent_offsets[item_index];
> +
> +    mov_build_index(c, st);
> +    return 0;
> +}
> +
>  static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
>      while (atom.size > 8) {
> @@ -4707,9 +4770,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          tag = avio_rl32(pb);
>          atom.size -= 4;
>          if (tag == MKTAG('h','d','l','r')) {
> +            int ret;
>              avio_seek(pb, -8, SEEK_CUR);
>              atom.size += 8;
> -            return mov_read_default(c, pb, atom);
> +            if ((ret = mov_read_default(c, pb, atom)) < 0)
> +                return ret;
> +            if (c->is_still_picture_avif) {
> +                int ret;
> +                // Add a stream for the YUV planes (primary item).
> +                if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
> +                    return ret;
> +                // For still AVIF images, the meta box contains all the
> +                // necessary information that would generally be provided by the
> +                // moov box. So simply mark that we have found the moov box so
> +                // that parsing can continue.
> +                c->found_moov = 1;
> +            }
> +            return ret;
>          }
>      }
>      return 0;
> @@ -7478,8 +7555,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      int item_count, extent_count;
>      uint64_t base_offset, extent_offset, extent_length;
>      uint8_t value;
> -    AVStream *st;
> -    MOVStreamContext *sc;
>  
>      if (!c->is_still_picture_avif) {
>          // * For non-avif, we simply ignore the iloc box.
> @@ -7493,27 +7568,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          return 0;
>      }
>  
> -    st = avformat_new_stream(c->fc, NULL);
> -    if (!st)
> -        return AVERROR(ENOMEM);
> -    st->id = c->fc->nb_streams;
> -    sc = av_mallocz(sizeof(MOVStreamContext));
> -    if (!sc)
> -        return AVERROR(ENOMEM);
> -
> -    st->priv_data = sc;
> -    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> -    st->codecpar->codec_id = AV_CODEC_ID_AV1;
> -    sc->ffindex = st->index;
> -    c->trak_index = st->index;
> -    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> -    st->time_base.num = st->time_base.den = 1;
> -    st->nb_frames = 1;
> -    sc->time_scale = 1;
> -    sc = st->priv_data;
> -    sc->pb = c->fc->pb;
> -    sc->pb_is_copied = 1;
> -
>      version = avio_r8(pb);
>      avio_rb24(pb);  // flags.
>  
> @@ -7529,34 +7583,22 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      }
>      item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
>  
> -    // Populate the necessary fields used by mov_build_index.
> -    sc->stsc_count = 1;
> -    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> -    if (!sc->stsc_data)
> +    c->avif_item_ids = av_malloc_array(item_count, sizeof(*c->avif_item_ids));
> +    if (!c->avif_item_ids)
>          return AVERROR(ENOMEM);
> -    sc->stsc_data[0].first = 1;
> -    sc->stsc_data[0].count = 1;
> -    sc->stsc_data[0].id = 1;
> -    sc->chunk_count = 1;
> -    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> -    if (!sc->chunk_offsets)
> +    c->avif_item_ids_size = item_count;
> +    c->avif_extent_lengths = av_malloc_array(item_count, sizeof(*c->avif_extent_lengths));
> +    if (!c->avif_extent_lengths)
>          return AVERROR(ENOMEM);
> -    sc->sample_count = 1;
> -    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> -    if (!sc->sample_sizes)
> +    c->avif_extent_offsets = av_malloc_array(item_count, sizeof(*c->avif_extent_offsets));
> +    if (!c->avif_extent_offsets)
>          return AVERROR(ENOMEM);
> -    sc->stts_count = 1;
> -    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> -    if (!sc->stts_data)
> -        return AVERROR(ENOMEM);
> -    sc->stts_data[0].count = 1;
> -    // Not used for still images. But needed by mov_build_index.
> -    sc->stts_data[0].duration = 0;
>  
>      for (int i = 0; i < item_count; i++) {
>          int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
>          if (avio_feof(pb))
>              return AVERROR_INVALIDDATA;
> +        c->avif_item_ids[i] = item_id;
>          if (version > 0)
>              avio_rb16(pb);  // construction_method.
>          avio_rb16(pb);  // data_reference_index.
> @@ -7572,20 +7614,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>              if (rb_size(pb, &extent_offset, offset_size) < 0 ||
>                  rb_size(pb, &extent_length, length_size) < 0)
>                  return AVERROR_INVALIDDATA;
> -            if (item_id == c->primary_item_id) {
> -                sc->sample_sizes[0] = extent_length;
> -                sc->chunk_offsets[0] = base_offset + extent_offset;
> -            }
> +            c->avif_extent_lengths[i] = extent_length;
> +            c->avif_extent_offsets[i] = base_offset + extent_offset;
>          }
>      }
>  
> -    mov_build_index(c, st);
> -
> -    // For still AVIF images, the iloc box contains all the necessary
> -    // information that would generally be provided by the moov box. So simply
> -    // mark that we have found the moov box so that parsing can continue.
> -    c->found_moov = 1;
> -
>      return atom.size;
>  }
>  
> @@ -8189,6 +8222,9 @@ static int mov_read_close(AVFormatContext *s)
>  
>      av_freep(&mov->aes_decrypt);
>      av_freep(&mov->chapter_tracks);
> +    av_freep(&mov->avif_item_ids);
> +    av_freep(&mov->avif_extent_lengths);
> +    av_freep(&mov->avif_extent_offsets);
>  
>      return 0;
>  }

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

On Wed, Jul 27, 2022 at 12:40 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Vignesh Venkatasubramanian:
> > Stores the item ids of all the items found in the file and
> > processes the primary item at the end of the meta box. This patch
> > does not change any behavior. It sets up the code for parsing
> > alpha channel (and possibly images with 'grid') in follow up
> > patches.
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> >  libavformat/isom.h |   4 ++
> >  libavformat/mov.c  | 146 ++++++++++++++++++++++++++++-----------------
> >  2 files changed, 95 insertions(+), 55 deletions(-)
> >
> > diff --git a/libavformat/isom.h b/libavformat/isom.h
> > index f05c2d9c28..d8b262e915 100644
> > --- a/libavformat/isom.h
> > +++ b/libavformat/isom.h
> > @@ -318,6 +318,10 @@ typedef struct MOVContext {
> >      uint32_t max_stts_delta;
> >      int is_still_picture_avif;
> >      int primary_item_id;
> > +    int *avif_item_ids;
> > +    int avif_item_ids_size;
> > +    int *avif_extent_lengths;
> > +    int64_t *avif_extent_offsets;
>
> Why are these three different buffers instead of one buffer of struct {
> int avif_item_ids; int avif_extent_lengths; int64_t avif_extent_offsets;}?
>

Ah good point. Updated to use a struct and a size field.

> >  } MOVContext;
> >
> >  int ff_mp4_read_descr_len(AVIOContext *pb);
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index a09a762d91..fc6a691da4 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -4698,6 +4698,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >      return ret;
> >  }
> >
> > +static int avif_add_stream(MOVContext *c, int item_id)
> > +{
> > +    MOVStreamContext *sc;
> > +    AVStream *st;
> > +    int item_index = -1;
> > +    for (int i = 0; i < c->avif_item_ids_size; i++)
> > +        if (c->avif_item_ids[i] == item_id) {
> > +            item_index = i;
> > +            break;
> > +        }
> > +    if (item_index < 0)
> > +        return AVERROR_INVALIDDATA;
> > +    st = avformat_new_stream(c->fc, NULL);
> > +    if (!st)
> > +        return AVERROR(ENOMEM);
> > +    st->id = c->fc->nb_streams;
> > +    sc = av_mallocz(sizeof(MOVStreamContext));
> > +    if (!sc)
> > +        return AVERROR(ENOMEM);
> > +
> > +    st->priv_data = sc;
> > +    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> > +    st->codecpar->codec_id = AV_CODEC_ID_AV1;
> > +    sc->ffindex = st->index;
> > +    c->trak_index = st->index;
> > +    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> > +    st->time_base.num = st->time_base.den = 1;
> > +    st->nb_frames = 1;
> > +    sc->time_scale = 1;
> > +    sc = st->priv_data;
> > +    sc->pb = c->fc->pb;
> > +    sc->pb_is_copied = 1;
> > +
> > +    // Populate the necessary fields used by mov_build_index.
> > +    sc->stsc_count = 1;
> > +    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> > +    if (!sc->stsc_data)
> > +        return AVERROR(ENOMEM);
> > +    sc->stsc_data[0].first = 1;
> > +    sc->stsc_data[0].count = 1;
> > +    sc->stsc_data[0].id = 1;
> > +    sc->chunk_count = 1;
> > +    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> > +    if (!sc->chunk_offsets)
> > +        return AVERROR(ENOMEM);
> > +    sc->sample_count = 1;
> > +    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> > +    if (!sc->sample_sizes)
> > +        return AVERROR(ENOMEM);
> > +    sc->stts_count = 1;
> > +    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> > +    if (!sc->stts_data)
> > +        return AVERROR(ENOMEM);
> > +    sc->stts_data[0].count = 1;
> > +    // Not used for still images. But needed by mov_build_index.
> > +    sc->stts_data[0].duration = 0;
> > +    sc->sample_sizes[0] = c->avif_extent_lengths[item_index];
> > +    sc->chunk_offsets[0] = c->avif_extent_offsets[item_index];
> > +
> > +    mov_build_index(c, st);
> > +    return 0;
> > +}
> > +
> >  static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >  {
> >      while (atom.size > 8) {
> > @@ -4707,9 +4770,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >          tag = avio_rl32(pb);
> >          atom.size -= 4;
> >          if (tag == MKTAG('h','d','l','r')) {
> > +            int ret;
> >              avio_seek(pb, -8, SEEK_CUR);
> >              atom.size += 8;
> > -            return mov_read_default(c, pb, atom);
> > +            if ((ret = mov_read_default(c, pb, atom)) < 0)
> > +                return ret;
> > +            if (c->is_still_picture_avif) {
> > +                int ret;
> > +                // Add a stream for the YUV planes (primary item).
> > +                if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
> > +                    return ret;
> > +                // For still AVIF images, the meta box contains all the
> > +                // necessary information that would generally be provided by the
> > +                // moov box. So simply mark that we have found the moov box so
> > +                // that parsing can continue.
> > +                c->found_moov = 1;
> > +            }
> > +            return ret;
> >          }
> >      }
> >      return 0;
> > @@ -7478,8 +7555,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >      int item_count, extent_count;
> >      uint64_t base_offset, extent_offset, extent_length;
> >      uint8_t value;
> > -    AVStream *st;
> > -    MOVStreamContext *sc;
> >
> >      if (!c->is_still_picture_avif) {
> >          // * For non-avif, we simply ignore the iloc box.
> > @@ -7493,27 +7568,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >          return 0;
> >      }
> >
> > -    st = avformat_new_stream(c->fc, NULL);
> > -    if (!st)
> > -        return AVERROR(ENOMEM);
> > -    st->id = c->fc->nb_streams;
> > -    sc = av_mallocz(sizeof(MOVStreamContext));
> > -    if (!sc)
> > -        return AVERROR(ENOMEM);
> > -
> > -    st->priv_data = sc;
> > -    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> > -    st->codecpar->codec_id = AV_CODEC_ID_AV1;
> > -    sc->ffindex = st->index;
> > -    c->trak_index = st->index;
> > -    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> > -    st->time_base.num = st->time_base.den = 1;
> > -    st->nb_frames = 1;
> > -    sc->time_scale = 1;
> > -    sc = st->priv_data;
> > -    sc->pb = c->fc->pb;
> > -    sc->pb_is_copied = 1;
> > -
> >      version = avio_r8(pb);
> >      avio_rb24(pb);  // flags.
> >
> > @@ -7529,34 +7583,22 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >      }
> >      item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
> >
> > -    // Populate the necessary fields used by mov_build_index.
> > -    sc->stsc_count = 1;
> > -    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> > -    if (!sc->stsc_data)
> > +    c->avif_item_ids = av_malloc_array(item_count, sizeof(*c->avif_item_ids));
> > +    if (!c->avif_item_ids)
> >          return AVERROR(ENOMEM);
> > -    sc->stsc_data[0].first = 1;
> > -    sc->stsc_data[0].count = 1;
> > -    sc->stsc_data[0].id = 1;
> > -    sc->chunk_count = 1;
> > -    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> > -    if (!sc->chunk_offsets)
> > +    c->avif_item_ids_size = item_count;
> > +    c->avif_extent_lengths = av_malloc_array(item_count, sizeof(*c->avif_extent_lengths));
> > +    if (!c->avif_extent_lengths)
> >          return AVERROR(ENOMEM);
> > -    sc->sample_count = 1;
> > -    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> > -    if (!sc->sample_sizes)
> > +    c->avif_extent_offsets = av_malloc_array(item_count, sizeof(*c->avif_extent_offsets));
> > +    if (!c->avif_extent_offsets)
> >          return AVERROR(ENOMEM);
> > -    sc->stts_count = 1;
> > -    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> > -    if (!sc->stts_data)
> > -        return AVERROR(ENOMEM);
> > -    sc->stts_data[0].count = 1;
> > -    // Not used for still images. But needed by mov_build_index.
> > -    sc->stts_data[0].duration = 0;
> >
> >      for (int i = 0; i < item_count; i++) {
> >          int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
> >          if (avio_feof(pb))
> >              return AVERROR_INVALIDDATA;
> > +        c->avif_item_ids[i] = item_id;
> >          if (version > 0)
> >              avio_rb16(pb);  // construction_method.
> >          avio_rb16(pb);  // data_reference_index.
> > @@ -7572,20 +7614,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >              if (rb_size(pb, &extent_offset, offset_size) < 0 ||
> >                  rb_size(pb, &extent_length, length_size) < 0)
> >                  return AVERROR_INVALIDDATA;
> > -            if (item_id == c->primary_item_id) {
> > -                sc->sample_sizes[0] = extent_length;
> > -                sc->chunk_offsets[0] = base_offset + extent_offset;
> > -            }
> > +            c->avif_extent_lengths[i] = extent_length;
> > +            c->avif_extent_offsets[i] = base_offset + extent_offset;
> >          }
> >      }
> >
> > -    mov_build_index(c, st);
> > -
> > -    // For still AVIF images, the iloc box contains all the necessary
> > -    // information that would generally be provided by the moov box. So simply
> > -    // mark that we have found the moov box so that parsing can continue.
> > -    c->found_moov = 1;
> > -
> >      return atom.size;
> >  }
> >
> > @@ -8189,6 +8222,9 @@ static int mov_read_close(AVFormatContext *s)
> >
> >      av_freep(&mov->aes_decrypt);
> >      av_freep(&mov->chapter_tracks);
> > +    av_freep(&mov->avif_item_ids);
> > +    av_freep(&mov->avif_extent_lengths);
> > +    av_freep(&mov->avif_extent_offsets);
> >
> >      return 0;
> >  }
>
> _______________________________________________
> 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] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

Stores the item ids of all the items found in the file and
processes the primary item at the end of the meta box. This patch
does not change any behavior. It sets up the code for parsing
alpha channel (and possibly images with 'grid') in follow up
patches.

Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
---
 libavformat/isom.h |   6 ++
 libavformat/mov.c  | 143 +++++++++++++++++++++++++++------------------
 2 files changed, 92 insertions(+), 57 deletions(-)

diff --git a/libavformat/isom.h b/libavformat/isom.h
index f05c2d9c28..9d8646d2ea 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -318,6 +318,12 @@ typedef struct MOVContext {
     uint32_t max_stts_delta;
     int is_still_picture_avif;
     int primary_item_id;
+    struct {
+        int item_id;
+        int extent_length;
+        int64_t extent_offset;
+    } *avif_info;
+    int avif_info_size;
 } MOVContext;
 
 int ff_mp4_read_descr_len(AVIOContext *pb);
diff --git a/libavformat/mov.c b/libavformat/mov.c
index a09a762d91..6ee6ed0950 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4698,6 +4698,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return ret;
 }
 
+static int avif_add_stream(MOVContext *c, int item_id)
+{
+    MOVStreamContext *sc;
+    AVStream *st;
+    int item_index = -1;
+    for (int i = 0; i < c->avif_info_size; i++)
+        if (c->avif_info[i].item_id == item_id) {
+            item_index = i;
+            break;
+        }
+    if (item_index < 0)
+        return AVERROR_INVALIDDATA;
+    st = avformat_new_stream(c->fc, NULL);
+    if (!st)
+        return AVERROR(ENOMEM);
+    st->id = c->fc->nb_streams;
+    sc = av_mallocz(sizeof(MOVStreamContext));
+    if (!sc)
+        return AVERROR(ENOMEM);
+
+    st->priv_data = sc;
+    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
+    st->codecpar->codec_id = AV_CODEC_ID_AV1;
+    sc->ffindex = st->index;
+    c->trak_index = st->index;
+    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
+    st->time_base.num = st->time_base.den = 1;
+    st->nb_frames = 1;
+    sc->time_scale = 1;
+    sc = st->priv_data;
+    sc->pb = c->fc->pb;
+    sc->pb_is_copied = 1;
+
+    // Populate the necessary fields used by mov_build_index.
+    sc->stsc_count = 1;
+    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
+    if (!sc->stsc_data)
+        return AVERROR(ENOMEM);
+    sc->stsc_data[0].first = 1;
+    sc->stsc_data[0].count = 1;
+    sc->stsc_data[0].id = 1;
+    sc->chunk_count = 1;
+    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
+    if (!sc->chunk_offsets)
+        return AVERROR(ENOMEM);
+    sc->sample_count = 1;
+    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
+    if (!sc->sample_sizes)
+        return AVERROR(ENOMEM);
+    sc->stts_count = 1;
+    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
+    if (!sc->stts_data)
+        return AVERROR(ENOMEM);
+    sc->stts_data[0].count = 1;
+    // Not used for still images. But needed by mov_build_index.
+    sc->stts_data[0].duration = 0;
+    sc->sample_sizes[0] = c->avif_info[item_index].extent_length;
+    sc->chunk_offsets[0] = c->avif_info[item_index].extent_offset;
+
+    mov_build_index(c, st);
+    return 0;
+}
+
 static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     while (atom.size > 8) {
@@ -4707,9 +4770,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         tag = avio_rl32(pb);
         atom.size -= 4;
         if (tag == MKTAG('h','d','l','r')) {
+            int ret;
             avio_seek(pb, -8, SEEK_CUR);
             atom.size += 8;
-            return mov_read_default(c, pb, atom);
+            if ((ret = mov_read_default(c, pb, atom)) < 0)
+                return ret;
+            if (c->is_still_picture_avif) {
+                int ret;
+                // Add a stream for the YUV planes (primary item).
+                if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
+                    return ret;
+                // For still AVIF images, the meta box contains all the
+                // necessary information that would generally be provided by the
+                // moov box. So simply mark that we have found the moov box so
+                // that parsing can continue.
+                c->found_moov = 1;
+            }
+            return ret;
         }
     }
     return 0;
@@ -7478,8 +7555,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     int item_count, extent_count;
     uint64_t base_offset, extent_offset, extent_length;
     uint8_t value;
-    AVStream *st;
-    MOVStreamContext *sc;
 
     if (!c->is_still_picture_avif) {
         // * For non-avif, we simply ignore the iloc box.
@@ -7493,27 +7568,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         return 0;
     }
 
-    st = avformat_new_stream(c->fc, NULL);
-    if (!st)
-        return AVERROR(ENOMEM);
-    st->id = c->fc->nb_streams;
-    sc = av_mallocz(sizeof(MOVStreamContext));
-    if (!sc)
-        return AVERROR(ENOMEM);
-
-    st->priv_data = sc;
-    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id = AV_CODEC_ID_AV1;
-    sc->ffindex = st->index;
-    c->trak_index = st->index;
-    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
-    st->time_base.num = st->time_base.den = 1;
-    st->nb_frames = 1;
-    sc->time_scale = 1;
-    sc = st->priv_data;
-    sc->pb = c->fc->pb;
-    sc->pb_is_copied = 1;
-
     version = avio_r8(pb);
     avio_rb24(pb);  // flags.
 
@@ -7529,34 +7583,17 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     }
     item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
 
-    // Populate the necessary fields used by mov_build_index.
-    sc->stsc_count = 1;
-    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
-    if (!sc->stsc_data)
-        return AVERROR(ENOMEM);
-    sc->stsc_data[0].first = 1;
-    sc->stsc_data[0].count = 1;
-    sc->stsc_data[0].id = 1;
-    sc->chunk_count = 1;
-    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
-    if (!sc->chunk_offsets)
+    c->avif_info = av_malloc_array(item_count, sizeof(*c->avif_info));
+    if (!c->avif_info)
         return AVERROR(ENOMEM);
-    sc->sample_count = 1;
-    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
-    if (!sc->sample_sizes)
-        return AVERROR(ENOMEM);
-    sc->stts_count = 1;
-    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
-    if (!sc->stts_data)
-        return AVERROR(ENOMEM);
-    sc->stts_data[0].count = 1;
-    // Not used for still images. But needed by mov_build_index.
-    sc->stts_data[0].duration = 0;
+    c->avif_info_size = item_count;
 
     for (int i = 0; i < item_count; i++) {
         int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
         if (avio_feof(pb))
             return AVERROR_INVALIDDATA;
+        c->avif_info[i].item_id = item_id;
+
         if (version > 0)
             avio_rb16(pb);  // construction_method.
         avio_rb16(pb);  // data_reference_index.
@@ -7572,20 +7609,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
             if (rb_size(pb, &extent_offset, offset_size) < 0 ||
                 rb_size(pb, &extent_length, length_size) < 0)
                 return AVERROR_INVALIDDATA;
-            if (item_id == c->primary_item_id) {
-                sc->sample_sizes[0] = extent_length;
-                sc->chunk_offsets[0] = base_offset + extent_offset;
-            }
+            c->avif_info[i].extent_length = extent_length;
+            c->avif_info[i].extent_offset = base_offset + extent_offset;
         }
     }
 
-    mov_build_index(c, st);
-
-    // For still AVIF images, the iloc box contains all the necessary
-    // information that would generally be provided by the moov box. So simply
-    // mark that we have found the moov box so that parsing can continue.
-    c->found_moov = 1;
-
     return atom.size;
 }
 
@@ -8189,6 +8217,7 @@ static int mov_read_close(AVFormatContext *s)
 
     av_freep(&mov->aes_decrypt);
     av_freep(&mov->chapter_tracks);
+    av_freep(&mov->avif_info);
 
     return 0;
 }
-- 
2.37.1.455.g008518b4e5-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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[James Zern]

Andreas,

On Thu, Jul 28, 2022 at 11:25 AM Vignesh Venkatasubramanian
<vigneshv-at-google.com@ffmpeg.org> wrote:
>
> Stores the item ids of all the items found in the file and
> processes the primary item at the end of the meta box. This patch
> does not change any behavior. It sets up the code for parsing
> alpha channel (and possibly images with 'grid') in follow up
> patches.
>
> Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> ---
>  libavformat/isom.h |   6 ++
>  libavformat/mov.c  | 143 +++++++++++++++++++++++++++------------------
>  2 files changed, 92 insertions(+), 57 deletions(-)
>

Any more comments on this one?

> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index f05c2d9c28..9d8646d2ea 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -318,6 +318,12 @@ typedef struct MOVContext {
>      uint32_t max_stts_delta;
>      int is_still_picture_avif;
>      int primary_item_id;
> +    struct {
> +        int item_id;
> +        int extent_length;
> +        int64_t extent_offset;
> +    } *avif_info;
> +    int avif_info_size;
>  } MOVContext;
>
>  int ff_mp4_read_descr_len(AVIOContext *pb);
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index a09a762d91..6ee6ed0950 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -4698,6 +4698,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      return ret;
>  }
>
> +static int avif_add_stream(MOVContext *c, int item_id)
> +{
> +    MOVStreamContext *sc;
> +    AVStream *st;
> +    int item_index = -1;
> +    for (int i = 0; i < c->avif_info_size; i++)
> +        if (c->avif_info[i].item_id == item_id) {
> +            item_index = i;
> +            break;
> +        }
> +    if (item_index < 0)
> +        return AVERROR_INVALIDDATA;
> +    st = avformat_new_stream(c->fc, NULL);
> +    if (!st)
> +        return AVERROR(ENOMEM);
> +    st->id = c->fc->nb_streams;
> +    sc = av_mallocz(sizeof(MOVStreamContext));
> +    if (!sc)
> +        return AVERROR(ENOMEM);
> +
> +    st->priv_data = sc;
> +    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> +    st->codecpar->codec_id = AV_CODEC_ID_AV1;
> +    sc->ffindex = st->index;
> +    c->trak_index = st->index;
> +    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> +    st->time_base.num = st->time_base.den = 1;
> +    st->nb_frames = 1;
> +    sc->time_scale = 1;
> +    sc = st->priv_data;
> +    sc->pb = c->fc->pb;
> +    sc->pb_is_copied = 1;
> +
> +    // Populate the necessary fields used by mov_build_index.
> +    sc->stsc_count = 1;
> +    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> +    if (!sc->stsc_data)
> +        return AVERROR(ENOMEM);
> +    sc->stsc_data[0].first = 1;
> +    sc->stsc_data[0].count = 1;
> +    sc->stsc_data[0].id = 1;
> +    sc->chunk_count = 1;
> +    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> +    if (!sc->chunk_offsets)
> +        return AVERROR(ENOMEM);
> +    sc->sample_count = 1;
> +    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> +    if (!sc->sample_sizes)
> +        return AVERROR(ENOMEM);
> +    sc->stts_count = 1;
> +    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> +    if (!sc->stts_data)
> +        return AVERROR(ENOMEM);
> +    sc->stts_data[0].count = 1;
> +    // Not used for still images. But needed by mov_build_index.
> +    sc->stts_data[0].duration = 0;
> +    sc->sample_sizes[0] = c->avif_info[item_index].extent_length;
> +    sc->chunk_offsets[0] = c->avif_info[item_index].extent_offset;
> +
> +    mov_build_index(c, st);
> +    return 0;
> +}
> +
>  static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>  {
>      while (atom.size > 8) {
> @@ -4707,9 +4770,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          tag = avio_rl32(pb);
>          atom.size -= 4;
>          if (tag == MKTAG('h','d','l','r')) {
> +            int ret;
>              avio_seek(pb, -8, SEEK_CUR);
>              atom.size += 8;
> -            return mov_read_default(c, pb, atom);
> +            if ((ret = mov_read_default(c, pb, atom)) < 0)
> +                return ret;
> +            if (c->is_still_picture_avif) {
> +                int ret;
> +                // Add a stream for the YUV planes (primary item).
> +                if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
> +                    return ret;
> +                // For still AVIF images, the meta box contains all the
> +                // necessary information that would generally be provided by the
> +                // moov box. So simply mark that we have found the moov box so
> +                // that parsing can continue.
> +                c->found_moov = 1;
> +            }
> +            return ret;
>          }
>      }
>      return 0;
> @@ -7478,8 +7555,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      int item_count, extent_count;
>      uint64_t base_offset, extent_offset, extent_length;
>      uint8_t value;
> -    AVStream *st;
> -    MOVStreamContext *sc;
>
>      if (!c->is_still_picture_avif) {
>          // * For non-avif, we simply ignore the iloc box.
> @@ -7493,27 +7568,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          return 0;
>      }
>
> -    st = avformat_new_stream(c->fc, NULL);
> -    if (!st)
> -        return AVERROR(ENOMEM);
> -    st->id = c->fc->nb_streams;
> -    sc = av_mallocz(sizeof(MOVStreamContext));
> -    if (!sc)
> -        return AVERROR(ENOMEM);
> -
> -    st->priv_data = sc;
> -    st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> -    st->codecpar->codec_id = AV_CODEC_ID_AV1;
> -    sc->ffindex = st->index;
> -    c->trak_index = st->index;
> -    st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> -    st->time_base.num = st->time_base.den = 1;
> -    st->nb_frames = 1;
> -    sc->time_scale = 1;
> -    sc = st->priv_data;
> -    sc->pb = c->fc->pb;
> -    sc->pb_is_copied = 1;
> -
>      version = avio_r8(pb);
>      avio_rb24(pb);  // flags.
>
> @@ -7529,34 +7583,17 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      }
>      item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
>
> -    // Populate the necessary fields used by mov_build_index.
> -    sc->stsc_count = 1;
> -    sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> -    if (!sc->stsc_data)
> -        return AVERROR(ENOMEM);
> -    sc->stsc_data[0].first = 1;
> -    sc->stsc_data[0].count = 1;
> -    sc->stsc_data[0].id = 1;
> -    sc->chunk_count = 1;
> -    sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> -    if (!sc->chunk_offsets)
> +    c->avif_info = av_malloc_array(item_count, sizeof(*c->avif_info));
> +    if (!c->avif_info)
>          return AVERROR(ENOMEM);
> -    sc->sample_count = 1;
> -    sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> -    if (!sc->sample_sizes)
> -        return AVERROR(ENOMEM);
> -    sc->stts_count = 1;
> -    sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> -    if (!sc->stts_data)
> -        return AVERROR(ENOMEM);
> -    sc->stts_data[0].count = 1;
> -    // Not used for still images. But needed by mov_build_index.
> -    sc->stts_data[0].duration = 0;
> +    c->avif_info_size = item_count;
>
>      for (int i = 0; i < item_count; i++) {
>          int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
>          if (avio_feof(pb))
>              return AVERROR_INVALIDDATA;
> +        c->avif_info[i].item_id = item_id;
> +
>          if (version > 0)
>              avio_rb16(pb);  // construction_method.
>          avio_rb16(pb);  // data_reference_index.
> @@ -7572,20 +7609,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>              if (rb_size(pb, &extent_offset, offset_size) < 0 ||
>                  rb_size(pb, &extent_length, length_size) < 0)
>                  return AVERROR_INVALIDDATA;
> -            if (item_id == c->primary_item_id) {
> -                sc->sample_sizes[0] = extent_length;
> -                sc->chunk_offsets[0] = base_offset + extent_offset;
> -            }
> +            c->avif_info[i].extent_length = extent_length;
> +            c->avif_info[i].extent_offset = base_offset + extent_offset;
>          }
>      }
>
> -    mov_build_index(c, st);
> -
> -    // For still AVIF images, the iloc box contains all the necessary
> -    // information that would generally be provided by the moov box. So simply
> -    // mark that we have found the moov box so that parsing can continue.
> -    c->found_moov = 1;
> -
>      return atom.size;
>  }
>
> @@ -8189,6 +8217,7 @@ static int mov_read_close(AVFormatContext *s)
>
>      av_freep(&mov->aes_decrypt);
>      av_freep(&mov->chapter_tracks);
> +    av_freep(&mov->avif_info);
>
>      return 0;
>  }
> --
> 2.37.1.455.g008518b4e5-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".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[James Zern]

On Tue, Aug 2, 2022 at 9:54 AM James Zern <jzern@google.com> wrote:
>
> Andreas,
>
> On Thu, Jul 28, 2022 at 11:25 AM Vignesh Venkatasubramanian
> <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > Stores the item ids of all the items found in the file and
> > processes the primary item at the end of the meta box. This patch
> > does not change any behavior. It sets up the code for parsing
> > alpha channel (and possibly images with 'grid') in follow up
> > patches.
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> >  libavformat/isom.h |   6 ++
> >  libavformat/mov.c  | 143 +++++++++++++++++++++++++++------------------
> >  2 files changed, 92 insertions(+), 57 deletions(-)
> >
>
> Any more comments on this one?
>

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

Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items

[Vignesh Venkatasubramanian]

On Tue, Jul 26, 2022 at 12:02 PM James Zern
<jzern-at-google.com@ffmpeg.org> wrote:
>
> On Fri, Jul 22, 2022 at 11:21 AM Vignesh Venkatasubramanian
> <vigneshv-at-google.com@ffmpeg.org> wrote:
> >
> > On Wed, Jul 13, 2022 at 9:12 AM Vignesh Venkatasubramanian
> > <vigneshv@google.com> wrote:
> > >
> > > On Mon, Jul 11, 2022 at 3:25 PM James Zern
> > > <jzern-at-google.com@ffmpeg.org> wrote:
> > > >
> > > > On Thu, Jun 30, 2022 at 2:04 PM Vignesh Venkatasubramanian
> > > > <vigneshv-at-google.com@ffmpeg.org> wrote:
> > > > >
> > > > > Stores the item ids of all the items found in the file and
> > > > > processes the primary item at the end of the meta box. This patch
> > > > > does not change any behavior. It sets up the code for parsing
> > > > > alpha channel (and possibly images with 'grid') in follow up
> > > > > patches.
> > > > >
> > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > > > > ---
> > > > >  libavformat/isom.h |   4 ++
> > > > >  libavformat/mov.c  | 148 ++++++++++++++++++++++++++++-----------------
> > > > >  2 files changed, 97 insertions(+), 55 deletions(-)
> > > > >
> > > > > [...]
> > > >
> > > > @@ -4692,9 +4755,25 @@ static int mov_read_meta(MOVContext *c,
> > > > AVIOContext *pb, MOVAtom atom)
> > > >          tag = avio_rl32(pb);
> > > >          atom.size -= 4;
> > > >          if (tag == MKTAG('h','d','l','r')) {
> > > > +            int ret;
> > > >              avio_seek(pb, -8, SEEK_CUR);
> > > >              atom.size += 8;
> > > > -            return mov_read_default(c, pb, atom);
> > > > +            ret = mov_read_default(c, pb, atom);
> > > > +            if (ret < 0)
> > > >
> > > > In some other cases these two lines are combined, if ((ret = ...
> > > >
> > >
> > > Done.
> > >
> > > > +                return ret;
> > > > +            if (c->is_still_picture_avif) {
> > > > +                int ret;
> > > > +                // Add a stream for the YUV planes (primary item).
> > > > +                ret = avif_add_stream(c, c->primary_item_id);
> > > > +                if (ret)
> > > >
> > > > This could be updated too and use '< 0' to match other code.
> > > >
> > >
> > > Done.
> > >
> > > > +                    return ret;
> > > > +                // For still AVIF images, the meta box contains all the
> > > > +                // necessary information that would generally be
> > > > provided by the
> > > > +                // moov box. So simply mark that we have found the moov box so
> > > > +                // that parsing can continue.
> > > > +                c->found_moov = 1;
> > > > +            }
> > > > +            return ret;
> > > >          }
> > > > _______________________________________________
> > > > 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
> >
> > Any further comments on this one?
> >
> > Please note that i have abandoned the second patch in this list. But
> > this one is still up for merging.
> >
>
> This looks like it needs to be rebased. I'll take a look after that.

Done.

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