* [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF @ 2022-04-24 18:35 Vignesh Venkatasubramanian 2022-05-02 21:39 ` Vignesh Venkatasubramanian 2022-06-01 17:30 ` James Zern 0 siblings, 2 replies; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-04-24 18:35 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian Update the still AVIF parser to only read the primary item. With this patch, AVIF still images with exif/icc/alpha channel will no longer fail to parse. For example, this patch enables parsing of files in: https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft Partially fixes trac ticket #7621 Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> --- libavformat/isom.h | 1 + libavformat/mov.c | 41 +++++++++++++++++++++-------------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/libavformat/isom.h b/libavformat/isom.h index cf36f04d5b..f05c2d9c28 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -317,6 +317,7 @@ typedef struct MOVContext { uint32_t mfra_size; uint32_t max_stts_delta; int is_still_picture_avif; + int primary_item_id; } MOVContext; int ff_mp4_read_descr_len(AVIOContext *pb); diff --git a/libavformat/mov.c b/libavformat/mov.c index 3e83e54a77..6be0f317ca 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7449,6 +7449,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) return size; } +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) +{ + avio_rb32(pb); // version & flags. + c->primary_item_id = avio_rb16(pb); + 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; @@ -7505,34 +7512,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) return AVERROR_PATCHWELCOME; } item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); - if (item_count > 1) { - // For still AVIF images, we only support one item. Second item will - // generally be found for AVIF images with alpha channel. We don't - // support them as of now. - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); - return AVERROR_PATCHWELCOME; - } // Populate the necessary fields used by mov_build_index. - sc->stsc_count = item_count; - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); + 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 = item_count; - sc->chunk_offsets = - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); + 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 = item_count; - sc->sample_sizes = - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); + 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 = item_count; - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); + 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; @@ -7540,7 +7538,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stts_data[0].duration = 0; for (int i = 0; i < item_count; i++) { - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); if (version > 0) avio_rb16(pb); // construction_method. avio_rb16(pb); // data_reference_index. @@ -7556,8 +7554,10 @@ 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; - sc->sample_sizes[0] = extent_length; - sc->chunk_offsets[0] = base_offset + extent_offset; + if (item_id == c->primary_item_id) { + sc->sample_sizes[0] = extent_length; + sc->chunk_offsets[0] = base_offset + extent_offset; + } } } @@ -7674,6 +7674,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ { MKTAG('i','l','o','c'), mov_read_iloc }, +{ MKTAG('p','i','t','m'), mov_read_pitm }, { 0, NULL } }; -- 2.36.0.rc2.479.g8af0fa9b8e-goog _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-04-24 18:35 [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF Vignesh Venkatasubramanian @ 2022-05-02 21:39 ` Vignesh Venkatasubramanian 2022-05-09 16:32 ` Vignesh Venkatasubramanian 2022-06-01 17:30 ` James Zern 1 sibling, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-05-02 21:39 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > Update the still AVIF parser to only read the primary item. With this > patch, AVIF still images with exif/icc/alpha channel will no longer > fail to parse. > > For example, this patch enables parsing of files in: > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > Partially fixes trac ticket #7621 > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > --- > libavformat/isom.h | 1 + > libavformat/mov.c | 41 +++++++++++++++++++++-------------------- > 2 files changed, 22 insertions(+), 20 deletions(-) > > diff --git a/libavformat/isom.h b/libavformat/isom.h > index cf36f04d5b..f05c2d9c28 100644 > --- a/libavformat/isom.h > +++ b/libavformat/isom.h > @@ -317,6 +317,7 @@ typedef struct MOVContext { > uint32_t mfra_size; > uint32_t max_stts_delta; > int is_still_picture_avif; > + int primary_item_id; > } MOVContext; > > int ff_mp4_read_descr_len(AVIOContext *pb); > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 3e83e54a77..6be0f317ca 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -7449,6 +7449,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) > return size; > } > > +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) > +{ > + avio_rb32(pb); // version & flags. > + c->primary_item_id = avio_rb16(pb); > + 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; > @@ -7505,34 +7512,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > return AVERROR_PATCHWELCOME; > } > item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > - if (item_count > 1) { > - // For still AVIF images, we only support one item. Second item will > - // generally be found for AVIF images with alpha channel. We don't > - // support them as of now. > - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); > - return AVERROR_PATCHWELCOME; > - } > > // Populate the necessary fields used by mov_build_index. > - sc->stsc_count = item_count; > - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); > + 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 = item_count; > - sc->chunk_offsets = > - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); > + 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 = item_count; > - sc->sample_sizes = > - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); > + 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 = item_count; > - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); > + 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; > @@ -7540,7 +7538,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > sc->stts_data[0].duration = 0; > > for (int i = 0; i < item_count; i++) { > - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; > + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > if (version > 0) > avio_rb16(pb); // construction_method. > avio_rb16(pb); // data_reference_index. > @@ -7556,8 +7554,10 @@ 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; > - sc->sample_sizes[0] = extent_length; > - sc->chunk_offsets[0] = base_offset + extent_offset; > + if (item_id == c->primary_item_id) { > + sc->sample_sizes[0] = extent_length; > + sc->chunk_offsets[0] = base_offset + extent_offset; > + } > } > } > > @@ -7674,6 +7674,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { > { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ > { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ > { MKTAG('i','l','o','c'), mov_read_iloc }, > +{ MKTAG('p','i','t','m'), mov_read_pitm }, > { 0, NULL } > }; > > -- > 2.36.0.rc2.479.g8af0fa9b8e-goog > Any comments on this one? If not, can this be merged please? :) -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-05-02 21:39 ` Vignesh Venkatasubramanian @ 2022-05-09 16:32 ` Vignesh Venkatasubramanian 2022-05-16 16:59 ` Vignesh Venkatasubramanian 0 siblings, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-05-09 16:32 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, May 2, 2022 at 2:39 PM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > <vigneshv@google.com> wrote: > > > > Update the still AVIF parser to only read the primary item. With this > > patch, AVIF still images with exif/icc/alpha channel will no longer > > fail to parse. > > > > For example, this patch enables parsing of files in: > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > Partially fixes trac ticket #7621 > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > > --- > > libavformat/isom.h | 1 + > > libavformat/mov.c | 41 +++++++++++++++++++++-------------------- > > 2 files changed, 22 insertions(+), 20 deletions(-) > > > > diff --git a/libavformat/isom.h b/libavformat/isom.h > > index cf36f04d5b..f05c2d9c28 100644 > > --- a/libavformat/isom.h > > +++ b/libavformat/isom.h > > @@ -317,6 +317,7 @@ typedef struct MOVContext { > > uint32_t mfra_size; > > uint32_t max_stts_delta; > > int is_still_picture_avif; > > + int primary_item_id; > > } MOVContext; > > > > int ff_mp4_read_descr_len(AVIOContext *pb); > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 3e83e54a77..6be0f317ca 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -7449,6 +7449,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) > > return size; > > } > > > > +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > +{ > > + avio_rb32(pb); // version & flags. > > + c->primary_item_id = avio_rb16(pb); > > + 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; > > @@ -7505,34 +7512,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > return AVERROR_PATCHWELCOME; > > } > > item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > > - if (item_count > 1) { > > - // For still AVIF images, we only support one item. Second item will > > - // generally be found for AVIF images with alpha channel. We don't > > - // support them as of now. > > - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); > > - return AVERROR_PATCHWELCOME; > > - } > > > > // Populate the necessary fields used by mov_build_index. > > - sc->stsc_count = item_count; > > - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); > > + 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 = item_count; > > - sc->chunk_offsets = > > - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); > > + 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 = item_count; > > - sc->sample_sizes = > > - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); > > + 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 = item_count; > > - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); > > + 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; > > @@ -7540,7 +7538,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > sc->stts_data[0].duration = 0; > > > > for (int i = 0; i < item_count; i++) { > > - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; > > + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > > if (version > 0) > > avio_rb16(pb); // construction_method. > > avio_rb16(pb); // data_reference_index. > > @@ -7556,8 +7554,10 @@ 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; > > - sc->sample_sizes[0] = extent_length; > > - sc->chunk_offsets[0] = base_offset + extent_offset; > > + if (item_id == c->primary_item_id) { > > + sc->sample_sizes[0] = extent_length; > > + sc->chunk_offsets[0] = base_offset + extent_offset; > > + } > > } > > } > > > > @@ -7674,6 +7674,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { > > { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ > > { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ > > { MKTAG('i','l','o','c'), mov_read_iloc }, > > +{ MKTAG('p','i','t','m'), mov_read_pitm }, > > { 0, NULL } > > }; > > > > -- > > 2.36.0.rc2.479.g8af0fa9b8e-goog > > > > Any comments on this one? If not, can this be merged please? :) > Another ping on this. > -- > Vignesh -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-05-09 16:32 ` Vignesh Venkatasubramanian @ 2022-05-16 16:59 ` Vignesh Venkatasubramanian 2022-05-19 16:13 ` Vignesh Venkatasubramanian 0 siblings, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-05-16 16:59 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian Update the still AVIF parser to only read the primary item. With this patch, AVIF still images with exif/icc/alpha channel will no longer fail to parse. For example, this patch enables parsing of files in: https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft Partially fixes trac ticket #7621 Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> --- libavformat/isom.h | 1 + libavformat/mov.c | 41 +++++++++++++++++++++-------------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/libavformat/isom.h b/libavformat/isom.h index cf36f04d5b..f05c2d9c28 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -317,6 +317,7 @@ typedef struct MOVContext { uint32_t mfra_size; uint32_t max_stts_delta; int is_still_picture_avif; + int primary_item_id; } MOVContext; int ff_mp4_read_descr_len(AVIOContext *pb); diff --git a/libavformat/mov.c b/libavformat/mov.c index d7be593a86..9310a393fe 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7445,6 +7445,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) return size; } +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) +{ + avio_rb32(pb); // version & flags. + c->primary_item_id = avio_rb16(pb); + 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; @@ -7501,34 +7508,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) return AVERROR_PATCHWELCOME; } item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); - if (item_count > 1) { - // For still AVIF images, we only support one item. Second item will - // generally be found for AVIF images with alpha channel. We don't - // support them as of now. - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); - return AVERROR_PATCHWELCOME; - } // Populate the necessary fields used by mov_build_index. - sc->stsc_count = item_count; - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); + 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 = item_count; - sc->chunk_offsets = - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); + 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 = item_count; - sc->sample_sizes = - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); + 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 = item_count; - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); + 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; @@ -7536,7 +7534,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stts_data[0].duration = 0; for (int i = 0; i < item_count; i++) { - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); if (version > 0) avio_rb16(pb); // construction_method. avio_rb16(pb); // data_reference_index. @@ -7552,8 +7550,10 @@ 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; - sc->sample_sizes[0] = extent_length; - sc->chunk_offsets[0] = base_offset + extent_offset; + if (item_id == c->primary_item_id) { + sc->sample_sizes[0] = extent_length; + sc->chunk_offsets[0] = base_offset + extent_offset; + } } } @@ -7670,6 +7670,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ { MKTAG('i','l','o','c'), mov_read_iloc }, +{ MKTAG('p','i','t','m'), mov_read_pitm }, { 0, NULL } }; -- 2.36.0.550.gb090851708-goog _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-05-16 16:59 ` Vignesh Venkatasubramanian @ 2022-05-19 16:13 ` Vignesh Venkatasubramanian 2022-05-31 19:11 ` Vignesh Venkatasubramanian 0 siblings, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-05-19 16:13 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, May 16, 2022 at 9:59 AM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > Update the still AVIF parser to only read the primary item. With this > patch, AVIF still images with exif/icc/alpha channel will no longer > fail to parse. > > For example, this patch enables parsing of files in: > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > Partially fixes trac ticket #7621 > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > --- > libavformat/isom.h | 1 + > libavformat/mov.c | 41 +++++++++++++++++++++-------------------- > 2 files changed, 22 insertions(+), 20 deletions(-) > > diff --git a/libavformat/isom.h b/libavformat/isom.h > index cf36f04d5b..f05c2d9c28 100644 > --- a/libavformat/isom.h > +++ b/libavformat/isom.h > @@ -317,6 +317,7 @@ typedef struct MOVContext { > uint32_t mfra_size; > uint32_t max_stts_delta; > int is_still_picture_avif; > + int primary_item_id; > } MOVContext; > > int ff_mp4_read_descr_len(AVIOContext *pb); > diff --git a/libavformat/mov.c b/libavformat/mov.c > index d7be593a86..9310a393fe 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -7445,6 +7445,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) > return size; > } > > +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) > +{ > + avio_rb32(pb); // version & flags. > + c->primary_item_id = avio_rb16(pb); > + 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; > @@ -7501,34 +7508,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > return AVERROR_PATCHWELCOME; > } > item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > - if (item_count > 1) { > - // For still AVIF images, we only support one item. Second item will > - // generally be found for AVIF images with alpha channel. We don't > - // support them as of now. > - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); > - return AVERROR_PATCHWELCOME; > - } > > // Populate the necessary fields used by mov_build_index. > - sc->stsc_count = item_count; > - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); > + 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 = item_count; > - sc->chunk_offsets = > - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); > + 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 = item_count; > - sc->sample_sizes = > - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); > + 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 = item_count; > - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); > + 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; > @@ -7536,7 +7534,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > sc->stts_data[0].duration = 0; > > for (int i = 0; i < item_count; i++) { > - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; > + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > if (version > 0) > avio_rb16(pb); // construction_method. > avio_rb16(pb); // data_reference_index. > @@ -7552,8 +7550,10 @@ 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; > - sc->sample_sizes[0] = extent_length; > - sc->chunk_offsets[0] = base_offset + extent_offset; > + if (item_id == c->primary_item_id) { > + sc->sample_sizes[0] = extent_length; > + sc->chunk_offsets[0] = base_offset + extent_offset; > + } > } > } > > @@ -7670,6 +7670,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { > { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ > { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ > { MKTAG('i','l','o','c'), mov_read_iloc }, > +{ MKTAG('p','i','t','m'), mov_read_pitm }, > { 0, NULL } > }; > > -- > 2.36.0.550.gb090851708-goog > Any comments on this one? If not, can this be merged please? -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-05-19 16:13 ` Vignesh Venkatasubramanian @ 2022-05-31 19:11 ` Vignesh Venkatasubramanian 0 siblings, 0 replies; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-05-31 19:11 UTC (permalink / raw) To: FFmpeg development discussions and patches On Thu, May 19, 2022 at 9:13 AM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > On Mon, May 16, 2022 at 9:59 AM Vignesh Venkatasubramanian > <vigneshv@google.com> wrote: > > > > Update the still AVIF parser to only read the primary item. With this > > patch, AVIF still images with exif/icc/alpha channel will no longer > > fail to parse. > > > > For example, this patch enables parsing of files in: > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > Partially fixes trac ticket #7621 > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > > --- > > libavformat/isom.h | 1 + > > libavformat/mov.c | 41 +++++++++++++++++++++-------------------- > > 2 files changed, 22 insertions(+), 20 deletions(-) > > > > diff --git a/libavformat/isom.h b/libavformat/isom.h > > index cf36f04d5b..f05c2d9c28 100644 > > --- a/libavformat/isom.h > > +++ b/libavformat/isom.h > > @@ -317,6 +317,7 @@ typedef struct MOVContext { > > uint32_t mfra_size; > > uint32_t max_stts_delta; > > int is_still_picture_avif; > > + int primary_item_id; > > } MOVContext; > > > > int ff_mp4_read_descr_len(AVIOContext *pb); > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index d7be593a86..9310a393fe 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -7445,6 +7445,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) > > return size; > > } > > > > +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > +{ > > + avio_rb32(pb); // version & flags. > > + c->primary_item_id = avio_rb16(pb); > > + 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; > > @@ -7501,34 +7508,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > return AVERROR_PATCHWELCOME; > > } > > item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > > - if (item_count > 1) { > > - // For still AVIF images, we only support one item. Second item will > > - // generally be found for AVIF images with alpha channel. We don't > > - // support them as of now. > > - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); > > - return AVERROR_PATCHWELCOME; > > - } > > > > // Populate the necessary fields used by mov_build_index. > > - sc->stsc_count = item_count; > > - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); > > + 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 = item_count; > > - sc->chunk_offsets = > > - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); > > + 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 = item_count; > > - sc->sample_sizes = > > - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); > > + 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 = item_count; > > - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); > > + 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; > > @@ -7536,7 +7534,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) > > sc->stts_data[0].duration = 0; > > > > for (int i = 0; i < item_count; i++) { > > - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; > > + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); > > if (version > 0) > > avio_rb16(pb); // construction_method. > > avio_rb16(pb); // data_reference_index. > > @@ -7552,8 +7550,10 @@ 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; > > - sc->sample_sizes[0] = extent_length; > > - sc->chunk_offsets[0] = base_offset + extent_offset; > > + if (item_id == c->primary_item_id) { > > + sc->sample_sizes[0] = extent_length; > > + sc->chunk_offsets[0] = base_offset + extent_offset; > > + } > > } > > } > > > > @@ -7670,6 +7670,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { > > { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ > > { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ > > { MKTAG('i','l','o','c'), mov_read_iloc }, > > +{ MKTAG('p','i','t','m'), mov_read_pitm }, > > { 0, NULL } > > }; > > > > -- > > 2.36.0.550.gb090851708-goog > > > > Any comments on this one? If not, can this be merged please? > Another ping on this please? > -- > Vignesh -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-04-24 18:35 [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF Vignesh Venkatasubramanian 2022-05-02 21:39 ` Vignesh Venkatasubramanian @ 2022-06-01 17:30 ` James Zern 2022-06-01 20:38 ` Vignesh Venkatasubramanian 1 sibling, 1 reply; 23+ messages in thread From: James Zern @ 2022-06-01 17:30 UTC (permalink / raw) To: FFmpeg development discussions and patches; +Cc: Vignesh Venkatasubramanian On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote: > > Update the still AVIF parser to only read the primary item. With this > patch, AVIF still images with exif/icc/alpha channel will no longer > fail to parse. > > For example, this patch enables parsing of files in: > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > Can some of the failing files or similar ones be added to fate? _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-01 17:30 ` James Zern @ 2022-06-01 20:38 ` Vignesh Venkatasubramanian 2022-06-02 20:34 ` James Zern 0 siblings, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-01 20:38 UTC (permalink / raw) To: James Zern; +Cc: FFmpeg development discussions and patches On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > Update the still AVIF parser to only read the primary item. With this > > patch, AVIF still images with exif/icc/alpha channel will no longer > > fail to parse. > > > > For example, this patch enables parsing of files in: > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > Can some of the failing files or similar ones be added to fate? There are no fate tests for AVIF parsing as of now. I was thinking about adding some for the muxing. But i am not sure what can be done here for the parsing. -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-01 20:38 ` Vignesh Venkatasubramanian @ 2022-06-02 20:34 ` James Zern 2022-06-08 17:21 ` Vignesh Venkatasubramanian 0 siblings, 1 reply; 23+ messages in thread From: James Zern @ 2022-06-02 20:34 UTC (permalink / raw) To: Vignesh Venkatasubramanian; +Cc: FFmpeg development discussions and patches On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > > > On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > > > Update the still AVIF parser to only read the primary item. With this > > > patch, AVIF still images with exif/icc/alpha channel will no longer > > > fail to parse. > > > > > > For example, this patch enables parsing of files in: > > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > > > > Can some of the failing files or similar ones be added to fate? > > There are no fate tests for AVIF parsing as of now. I was thinking > about adding some for the muxing. But i am not sure what can be done > here for the parsing. > Thanks. Are there any for general mov/mp4 parsing that could be extended? This looks good otherwise. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-02 20:34 ` James Zern @ 2022-06-08 17:21 ` Vignesh Venkatasubramanian 2022-06-09 7:50 ` Gyan Doshi 0 siblings, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-08 17:21 UTC (permalink / raw) To: James Zern; +Cc: FFmpeg development discussions and patches On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > > On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > <vigneshv@google.com> wrote: > > > > On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > > > > > On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > > <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > > > > > Update the still AVIF parser to only read the primary item. With this > > > > patch, AVIF still images with exif/icc/alpha channel will no longer > > > > fail to parse. > > > > > > > > For example, this patch enables parsing of files in: > > > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > > > > > > > Can some of the failing files or similar ones be added to fate? > > > > There are no fate tests for AVIF parsing as of now. I was thinking > > about adding some for the muxing. But i am not sure what can be done > > here for the parsing. > > > > Thanks. Are there any for general mov/mp4 parsing that could be > extended? This looks good otherwise. From what i see, most of the mov tests only seem to be for muxing. I'm not entirely certain about how to add a test for AVIF parsing. If anybody has an idea, i'd be open to adding it. -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-08 17:21 ` Vignesh Venkatasubramanian @ 2022-06-09 7:50 ` Gyan Doshi 2022-06-10 17:30 ` Vignesh Venkatasubramanian 2022-06-10 17:34 ` Vignesh Venkatasubramanian 0 siblings, 2 replies; 23+ messages in thread From: Gyan Doshi @ 2022-06-09 7:50 UTC (permalink / raw) To: ffmpeg-devel On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian >> <vigneshv@google.com> wrote: >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: >>>>> Update the still AVIF parser to only read the primary item. With this >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer >>>>> fail to parse. >>>>> >>>>> For example, this patch enables parsing of files in: >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft >>>>> >>>> Can some of the failing files or similar ones be added to fate? >>> There are no fate tests for AVIF parsing as of now. I was thinking >>> about adding some for the muxing. But i am not sure what can be done >>> here for the parsing. >>> >> Thanks. Are there any for general mov/mp4 parsing that could be >> extended? This looks good otherwise. > From what i see, most of the mov tests only seem to be for muxing. I'm > not entirely certain about how to add a test for AVIF parsing. If > anybody has an idea, i'd be open to adding it. Basic test would use the framemd5 muxer to compare demuxed packets with a reference. See fate-ffmpeg-streamloop Regards, Gyan _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-09 7:50 ` Gyan Doshi @ 2022-06-10 17:30 ` Vignesh Venkatasubramanian 2022-06-10 17:33 ` Andreas Rheinhardt 2022-06-10 17:34 ` Vignesh Venkatasubramanian 1 sibling, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-10 17:30 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian Update the still AVIF parser to only read the primary item. With this patch, AVIF still images with exif/icc/alpha channel will no longer fail to parse. For example, this patch enables parsing of files in: https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft Adding two fate tests: 1) demuxing of still image with 1 item - this test will pass regardlesss of this patch. 2) demuxing of still image with 2 items - this test will fail without this patch and will pass with patch applied. Partially fixes trac ticket #7621 Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> --- libavformat/isom.h | 1 + libavformat/mov.c | 41 ++++++++++--------- tests/fate/mov.mak | 9 ++++ .../fate/mov-avif-demux-still-image-1-item | 11 +++++ .../mov-avif-demux-still-image-multiple-items | 11 +++++ 5 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 tests/ref/fate/mov-avif-demux-still-image-1-item create mode 100644 tests/ref/fate/mov-avif-demux-still-image-multiple-items diff --git a/libavformat/isom.h b/libavformat/isom.h index cf36f04d5b..f05c2d9c28 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -317,6 +317,7 @@ typedef struct MOVContext { uint32_t mfra_size; uint32_t max_stts_delta; int is_still_picture_avif; + int primary_item_id; } MOVContext; int ff_mp4_read_descr_len(AVIOContext *pb); diff --git a/libavformat/mov.c b/libavformat/mov.c index d7be593a86..9310a393fe 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7445,6 +7445,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) return size; } +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) +{ + avio_rb32(pb); // version & flags. + c->primary_item_id = avio_rb16(pb); + 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; @@ -7501,34 +7508,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) return AVERROR_PATCHWELCOME; } item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); - if (item_count > 1) { - // For still AVIF images, we only support one item. Second item will - // generally be found for AVIF images with alpha channel. We don't - // support them as of now. - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); - return AVERROR_PATCHWELCOME; - } // Populate the necessary fields used by mov_build_index. - sc->stsc_count = item_count; - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); + 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 = item_count; - sc->chunk_offsets = - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); + 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 = item_count; - sc->sample_sizes = - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); + 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 = item_count; - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); + 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; @@ -7536,7 +7534,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stts_data[0].duration = 0; for (int i = 0; i < item_count; i++) { - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); if (version > 0) avio_rb16(pb); // construction_method. avio_rb16(pb); // data_reference_index. @@ -7552,8 +7550,10 @@ 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; - sc->sample_sizes[0] = extent_length; - sc->chunk_offsets[0] = base_offset + extent_offset; + if (item_id == c->primary_item_id) { + sc->sample_sizes[0] = extent_length; + sc->chunk_offsets[0] = base_offset + extent_offset; + } } } @@ -7670,6 +7670,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { { MKTAG('S','A','3','D'), mov_read_SA3D }, /* ambisonic audio box */ { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ { MKTAG('i','l','o','c'), mov_read_iloc }, +{ MKTAG('p','i','t','m'), mov_read_pitm }, { 0, NULL } }; diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 2fae054423..842c7f9aa1 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -17,6 +17,8 @@ FATE_MOV = fate-mov-3elist \ fate-mov-bbi-elst-starts-b \ fate-mov-neg-firstpts-discard-frames \ fate-mov-stream-shorter-than-movie \ + fate-mov-avif-demux-still-image-1-item \ + fate-mov-avif-demux-still-image-multiple-items \ FATE_MOV_FFPROBE = fate-mov-neg-firstpts-discard \ fate-mov-neg-firstpts-discard-vorbis \ @@ -138,6 +140,13 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV, SRT_DEMUXER TTML fate-mov-mp4-ttml-stpp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" fate-mov-mp4-ttml-dfxp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" +# avif demuxing - still image with 1 item. +fate-mov-avif-demux-still-image-1-item: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image.avif -c:v copy + +# avif demuxing - still image with multiple items. only the primary item will be +# parsed. +fate-mov-avif-demux-still-image-multiple-items: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image_exif.avif -c:v copy + # Resulting remux should have: # 1. first audio stream with AV_DISPOSITION_HEARING_IMPAIRED # 2. second audio stream with AV_DISPOSITION_VISUAL_IMPAIRED | DESCRIPTIONS diff --git a/tests/ref/fate/mov-avif-demux-still-image-1-item b/tests/ref/fate/mov-avif-demux-still-image-1-item new file mode 100644 index 0000000000..93773afd4e --- /dev/null +++ b/tests/ref/fate/mov-avif-demux-still-image-1-item @@ -0,0 +1,11 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#extradata 0, 13, b52ae298d37128862ef1918cf916239c +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: av1 +#dimensions 0: 352x288 +#sar 0: 1/1 +#stream#, dts, pts, duration, size, hash +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 diff --git a/tests/ref/fate/mov-avif-demux-still-image-multiple-items b/tests/ref/fate/mov-avif-demux-still-image-multiple-items new file mode 100644 index 0000000000..93773afd4e --- /dev/null +++ b/tests/ref/fate/mov-avif-demux-still-image-multiple-items @@ -0,0 +1,11 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#extradata 0, 13, b52ae298d37128862ef1918cf916239c +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: av1 +#dimensions 0: 352x288 +#sar 0: 1/1 +#stream#, dts, pts, duration, size, hash +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 -- 2.36.1.476.g0c4daa206d-goog _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-10 17:30 ` Vignesh Venkatasubramanian @ 2022-06-10 17:33 ` Andreas Rheinhardt 2022-06-10 17:37 ` Vignesh Venkatasubramanian 0 siblings, 1 reply; 23+ messages in thread From: Andreas Rheinhardt @ 2022-06-10 17:33 UTC (permalink / raw) To: ffmpeg-devel Vignesh Venkatasubramanian: > Update the still AVIF parser to only read the primary item. With this > patch, AVIF still images with exif/icc/alpha channel will no longer > fail to parse. > > For example, this patch enables parsing of files in: > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > Adding two fate tests: > 1) demuxing of still image with 1 item - this test will pass regardlesss > of this patch. > 2) demuxing of still image with 2 items - this test will fail without > this patch and will pass with patch applied. > > Partially fixes trac ticket #7621 > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > --- > libavformat/isom.h | 1 + > libavformat/mov.c | 41 ++++++++++--------- > tests/fate/mov.mak | 9 ++++ > .../fate/mov-avif-demux-still-image-1-item | 11 +++++ > .../mov-avif-demux-still-image-multiple-items | 11 +++++ > 5 files changed, 53 insertions(+), 20 deletions(-) > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-1-item > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-multiple-items > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > index 2fae054423..842c7f9aa1 100644 > --- a/tests/fate/mov.mak > +++ b/tests/fate/mov.mak > @@ -17,6 +17,8 @@ FATE_MOV = fate-mov-3elist \ > fate-mov-bbi-elst-starts-b \ > fate-mov-neg-firstpts-discard-frames \ > fate-mov-stream-shorter-than-movie \ > + fate-mov-avif-demux-still-image-1-item \ > + fate-mov-avif-demux-still-image-multiple-items \ > > FATE_MOV_FFPROBE = fate-mov-neg-firstpts-discard \ > fate-mov-neg-firstpts-discard-vorbis \ > @@ -138,6 +140,13 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV, SRT_DEMUXER TTML > fate-mov-mp4-ttml-stpp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" > fate-mov-mp4-ttml-dfxp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" > > +# avif demuxing - still image with 1 item. > +fate-mov-avif-demux-still-image-1-item: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image.avif -c:v copy > + > +# avif demuxing - still image with multiple items. only the primary item will be > +# parsed. > +fate-mov-avif-demux-still-image-multiple-items: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image_exif.avif -c:v copy Can we create such files with our muxer? In this case one could use a remux test, i.e. a test that creates the file and the demuxes the just created file. > + > # Resulting remux should have: > # 1. first audio stream with AV_DISPOSITION_HEARING_IMPAIRED > # 2. second audio stream with AV_DISPOSITION_VISUAL_IMPAIRED | DESCRIPTIONS > diff --git a/tests/ref/fate/mov-avif-demux-still-image-1-item b/tests/ref/fate/mov-avif-demux-still-image-1-item > new file mode 100644 > index 0000000000..93773afd4e > --- /dev/null > +++ b/tests/ref/fate/mov-avif-demux-still-image-1-item > @@ -0,0 +1,11 @@ > +#format: frame checksums > +#version: 2 > +#hash: MD5 > +#extradata 0, 13, b52ae298d37128862ef1918cf916239c > +#tb 0: 1/1 > +#media_type 0: video > +#codec_id 0: av1 > +#dimensions 0: 352x288 > +#sar 0: 1/1 > +#stream#, dts, pts, duration, size, hash > +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 > diff --git a/tests/ref/fate/mov-avif-demux-still-image-multiple-items b/tests/ref/fate/mov-avif-demux-still-image-multiple-items > new file mode 100644 > index 0000000000..93773afd4e > --- /dev/null > +++ b/tests/ref/fate/mov-avif-demux-still-image-multiple-items > @@ -0,0 +1,11 @@ > +#format: frame checksums > +#version: 2 > +#hash: MD5 > +#extradata 0, 13, b52ae298d37128862ef1918cf916239c > +#tb 0: 1/1 > +#media_type 0: video > +#codec_id 0: av1 > +#dimensions 0: 352x288 > +#sar 0: 1/1 > +#stream#, dts, pts, duration, size, hash > +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-10 17:33 ` Andreas Rheinhardt @ 2022-06-10 17:37 ` Vignesh Venkatasubramanian 0 siblings, 0 replies; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-10 17:37 UTC (permalink / raw) To: FFmpeg development discussions and patches On Fri, Jun 10, 2022 at 10:33 AM Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > Vignesh Venkatasubramanian: > > Update the still AVIF parser to only read the primary item. With this > > patch, AVIF still images with exif/icc/alpha channel will no longer > > fail to parse. > > > > For example, this patch enables parsing of files in: > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > Adding two fate tests: > > 1) demuxing of still image with 1 item - this test will pass regardlesss > > of this patch. > > 2) demuxing of still image with 2 items - this test will fail without > > this patch and will pass with patch applied. > > > > Partially fixes trac ticket #7621 > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > > --- > > libavformat/isom.h | 1 + > > libavformat/mov.c | 41 ++++++++++--------- > > tests/fate/mov.mak | 9 ++++ > > .../fate/mov-avif-demux-still-image-1-item | 11 +++++ > > .../mov-avif-demux-still-image-multiple-items | 11 +++++ > > 5 files changed, 53 insertions(+), 20 deletions(-) > > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-1-item > > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-multiple-items > > > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak > > index 2fae054423..842c7f9aa1 100644 > > --- a/tests/fate/mov.mak > > +++ b/tests/fate/mov.mak > > @@ -17,6 +17,8 @@ FATE_MOV = fate-mov-3elist \ > > fate-mov-bbi-elst-starts-b \ > > fate-mov-neg-firstpts-discard-frames \ > > fate-mov-stream-shorter-than-movie \ > > + fate-mov-avif-demux-still-image-1-item \ > > + fate-mov-avif-demux-still-image-multiple-items \ > > > > FATE_MOV_FFPROBE = fate-mov-neg-firstpts-discard \ > > fate-mov-neg-firstpts-discard-vorbis \ > > @@ -138,6 +140,13 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV, SRT_DEMUXER TTML > > fate-mov-mp4-ttml-stpp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" > > fate-mov-mp4-ttml-dfxp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" > > > > +# avif demuxing - still image with 1 item. > > +fate-mov-avif-demux-still-image-1-item: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image.avif -c:v copy > > + > > +# avif demuxing - still image with multiple items. only the primary item will be > > +# parsed. > > +fate-mov-avif-demux-still-image-multiple-items: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image_exif.avif -c:v copy > > Can we create such files with our muxer? In this case one could use a > remux test, i.e. a test that creates the file and the demuxes the just > created file. > Unfortunately, the file with exif cannot be created using the ffmpeg's muxer as of now. So we will have to get that externally (i made that with MP4Box). The other still image file however can be creating using our muxer. I will add another test that does remuxing for the 1-item case in a separate patch if that's alright (since it's not directly related to what this patch is doing). I was also going to add some fate tests for animated AVIF muxing/parsing, so i will include that in the other patch as well. > > + > > # Resulting remux should have: > > # 1. first audio stream with AV_DISPOSITION_HEARING_IMPAIRED > > # 2. second audio stream with AV_DISPOSITION_VISUAL_IMPAIRED | DESCRIPTIONS > > diff --git a/tests/ref/fate/mov-avif-demux-still-image-1-item b/tests/ref/fate/mov-avif-demux-still-image-1-item > > new file mode 100644 > > index 0000000000..93773afd4e > > --- /dev/null > > +++ b/tests/ref/fate/mov-avif-demux-still-image-1-item > > @@ -0,0 +1,11 @@ > > +#format: frame checksums > > +#version: 2 > > +#hash: MD5 > > +#extradata 0, 13, b52ae298d37128862ef1918cf916239c > > +#tb 0: 1/1 > > +#media_type 0: video > > +#codec_id 0: av1 > > +#dimensions 0: 352x288 > > +#sar 0: 1/1 > > +#stream#, dts, pts, duration, size, hash > > +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 > > diff --git a/tests/ref/fate/mov-avif-demux-still-image-multiple-items b/tests/ref/fate/mov-avif-demux-still-image-multiple-items > > new file mode 100644 > > index 0000000000..93773afd4e > > --- /dev/null > > +++ b/tests/ref/fate/mov-avif-demux-still-image-multiple-items > > @@ -0,0 +1,11 @@ > > +#format: frame checksums > > +#version: 2 > > +#hash: MD5 > > +#extradata 0, 13, b52ae298d37128862ef1918cf916239c > > +#tb 0: 1/1 > > +#media_type 0: video > > +#codec_id 0: av1 > > +#dimensions 0: 352x288 > > +#sar 0: 1/1 > > +#stream#, dts, pts, duration, size, hash > > +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-09 7:50 ` Gyan Doshi 2022-06-10 17:30 ` Vignesh Venkatasubramanian @ 2022-06-10 17:34 ` Vignesh Venkatasubramanian 2022-06-13 16:32 ` Vignesh Venkatasubramanian 1 sibling, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-10 17:34 UTC (permalink / raw) To: FFmpeg development discussions and patches On Thu, Jun 9, 2022 at 12:50 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > >> <vigneshv@google.com> wrote: > >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: > >>>>> Update the still AVIF parser to only read the primary item. With this > >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer > >>>>> fail to parse. > >>>>> > >>>>> For example, this patch enables parsing of files in: > >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > >>>>> > >>>> Can some of the failing files or similar ones be added to fate? > >>> There are no fate tests for AVIF parsing as of now. I was thinking > >>> about adding some for the muxing. But i am not sure what can be done > >>> here for the parsing. > >>> > >> Thanks. Are there any for general mov/mp4 parsing that could be > >> extended? This looks good otherwise. > > From what i see, most of the mov tests only seem to be for muxing. I'm > > not entirely certain about how to add a test for AVIF parsing. If > > anybody has an idea, i'd be open to adding it. > > Basic test would use the framemd5 muxer to compare demuxed packets with > a reference. > See fate-ffmpeg-streamloop > Thank you i have added a couple of fate tests. I am not sure how to add the AVIF files to the fate test server. I have them on Google Drive here: https://drive.google.com/file/d/1diZoM0Ew7Co3Yh5w5y1J-3IiBYVmUv9J/view?usp=sharing https://drive.google.com/file/d/1DdrD1mW36evt40a4RkeLYpx-oojmbc3z/view?usp=sharing These links should be publicly available without any sign in required. Please let me know if there is another preferred way of sharing test files and i can share it that way. Also for the record, these files were created by remuxing an existing file in the fate suite. still_image.avif - contains the first frame from av1-test-vectors/av1-1-b8-02-allintra.ivf still_image_exif.avif - contains the first frame from av1-test-vectors/av1-1-b8-02-allintra.ivf with dummy exif data. > Regards, > Gyan > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-10 17:34 ` Vignesh Venkatasubramanian @ 2022-06-13 16:32 ` Vignesh Venkatasubramanian 2022-06-21 17:12 ` Vignesh Venkatasubramanian 2022-06-28 18:21 ` James Zern 0 siblings, 2 replies; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-13 16:32 UTC (permalink / raw) To: FFmpeg development discussions and patches On Fri, Jun 10, 2022 at 10:34 AM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > On Thu, Jun 9, 2022 at 12:50 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > > > > > On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > > > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > > >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > > >> <vigneshv@google.com> wrote: > > >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: > > >>>>> Update the still AVIF parser to only read the primary item. With this > > >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer > > >>>>> fail to parse. > > >>>>> > > >>>>> For example, this patch enables parsing of files in: > > >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > >>>>> > > >>>> Can some of the failing files or similar ones be added to fate? > > >>> There are no fate tests for AVIF parsing as of now. I was thinking > > >>> about adding some for the muxing. But i am not sure what can be done > > >>> here for the parsing. > > >>> > > >> Thanks. Are there any for general mov/mp4 parsing that could be > > >> extended? This looks good otherwise. > > > From what i see, most of the mov tests only seem to be for muxing. I'm > > > not entirely certain about how to add a test for AVIF parsing. If > > > anybody has an idea, i'd be open to adding it. > > > > Basic test would use the framemd5 muxer to compare demuxed packets with > > a reference. > > See fate-ffmpeg-streamloop > > > > Thank you i have added a couple of fate tests. > > I am not sure how to add the AVIF files to the fate test server. I > have them on Google Drive here: > > https://drive.google.com/file/d/1diZoM0Ew7Co3Yh5w5y1J-3IiBYVmUv9J/view?usp=sharing > https://drive.google.com/file/d/1DdrD1mW36evt40a4RkeLYpx-oojmbc3z/view?usp=sharing > > These links should be publicly available without any sign in required. > Please let me know if there is another preferred way of sharing test > files and i can share it that way. > Also, i forgot to mention that these files have to be uploaded under a new directory called "avif" in the fate server. > Also for the record, these files were created by remuxing an existing > file in the fate suite. > > still_image.avif - contains the first frame from > av1-test-vectors/av1-1-b8-02-allintra.ivf > still_image_exif.avif - contains the first frame from > av1-test-vectors/av1-1-b8-02-allintra.ivf with dummy exif data. > > > Regards, > > Gyan > > _______________________________________________ > > 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 -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-13 16:32 ` Vignesh Venkatasubramanian @ 2022-06-21 17:12 ` Vignesh Venkatasubramanian 2022-06-27 16:44 ` Vignesh Venkatasubramanian 2022-06-28 18:21 ` James Zern 1 sibling, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-21 17:12 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, Jun 13, 2022 at 9:32 AM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > On Fri, Jun 10, 2022 at 10:34 AM Vignesh Venkatasubramanian > <vigneshv@google.com> wrote: > > > > On Thu, Jun 9, 2022 at 12:50 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > > > > > > > > > On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > > > > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > > > >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > > > >> <vigneshv@google.com> wrote: > > > >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > > >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > > >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: > > > >>>>> Update the still AVIF parser to only read the primary item. With this > > > >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer > > > >>>>> fail to parse. > > > >>>>> > > > >>>>> For example, this patch enables parsing of files in: > > > >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > >>>>> > > > >>>> Can some of the failing files or similar ones be added to fate? > > > >>> There are no fate tests for AVIF parsing as of now. I was thinking > > > >>> about adding some for the muxing. But i am not sure what can be done > > > >>> here for the parsing. > > > >>> > > > >> Thanks. Are there any for general mov/mp4 parsing that could be > > > >> extended? This looks good otherwise. > > > > From what i see, most of the mov tests only seem to be for muxing. I'm > > > > not entirely certain about how to add a test for AVIF parsing. If > > > > anybody has an idea, i'd be open to adding it. > > > > > > Basic test would use the framemd5 muxer to compare demuxed packets with > > > a reference. > > > See fate-ffmpeg-streamloop > > > > > > > Thank you i have added a couple of fate tests. > > > > I am not sure how to add the AVIF files to the fate test server. I > > have them on Google Drive here: > > > > https://drive.google.com/file/d/1diZoM0Ew7Co3Yh5w5y1J-3IiBYVmUv9J/view?usp=sharing > > https://drive.google.com/file/d/1DdrD1mW36evt40a4RkeLYpx-oojmbc3z/view?usp=sharing > > > > These links should be publicly available without any sign in required. > > Please let me know if there is another preferred way of sharing test > > files and i can share it that way. > > > > Also, i forgot to mention that these files have to be uploaded under a > new directory called "avif" in the fate server. > > > Also for the record, these files were created by remuxing an existing > > file in the fate suite. > > > > still_image.avif - contains the first frame from > > av1-test-vectors/av1-1-b8-02-allintra.ivf > > still_image_exif.avif - contains the first frame from > > av1-test-vectors/av1-1-b8-02-allintra.ivf with dummy exif data. > > > > > Regards, > > > Gyan > > > _______________________________________________ > > > 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 > > > > -- > Vignesh Ping on this please. -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-21 17:12 ` Vignesh Venkatasubramanian @ 2022-06-27 16:44 ` Vignesh Venkatasubramanian 0 siblings, 0 replies; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-27 16:44 UTC (permalink / raw) To: FFmpeg development discussions and patches On Tue, Jun 21, 2022 at 10:12 AM Vignesh Venkatasubramanian <vigneshv@google.com> wrote: > > On Mon, Jun 13, 2022 at 9:32 AM Vignesh Venkatasubramanian > <vigneshv@google.com> wrote: > > > > On Fri, Jun 10, 2022 at 10:34 AM Vignesh Venkatasubramanian > > <vigneshv@google.com> wrote: > > > > > > On Thu, Jun 9, 2022 at 12:50 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > > > > > > > > > > > > > On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > > > > > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > > > > >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > > > > >> <vigneshv@google.com> wrote: > > > > >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > > > >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > > > >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > >>>>> Update the still AVIF parser to only read the primary item. With this > > > > >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer > > > > >>>>> fail to parse. > > > > >>>>> > > > > >>>>> For example, this patch enables parsing of files in: > > > > >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > >>>>> > > > > >>>> Can some of the failing files or similar ones be added to fate? > > > > >>> There are no fate tests for AVIF parsing as of now. I was thinking > > > > >>> about adding some for the muxing. But i am not sure what can be done > > > > >>> here for the parsing. > > > > >>> > > > > >> Thanks. Are there any for general mov/mp4 parsing that could be > > > > >> extended? This looks good otherwise. > > > > > From what i see, most of the mov tests only seem to be for muxing. I'm > > > > > not entirely certain about how to add a test for AVIF parsing. If > > > > > anybody has an idea, i'd be open to adding it. > > > > > > > > Basic test would use the framemd5 muxer to compare demuxed packets with > > > > a reference. > > > > See fate-ffmpeg-streamloop > > > > > > > > > > Thank you i have added a couple of fate tests. > > > > > > I am not sure how to add the AVIF files to the fate test server. I > > > have them on Google Drive here: > > > > > > https://drive.google.com/file/d/1diZoM0Ew7Co3Yh5w5y1J-3IiBYVmUv9J/view?usp=sharing > > > https://drive.google.com/file/d/1DdrD1mW36evt40a4RkeLYpx-oojmbc3z/view?usp=sharing > > > > > > These links should be publicly available without any sign in required. > > > Please let me know if there is another preferred way of sharing test > > > files and i can share it that way. > > > > > > > Also, i forgot to mention that these files have to be uploaded under a > > new directory called "avif" in the fate server. > > > > > Also for the record, these files were created by remuxing an existing > > > file in the fate suite. > > > > > > still_image.avif - contains the first frame from > > > av1-test-vectors/av1-1-b8-02-allintra.ivf > > > still_image_exif.avif - contains the first frame from > > > av1-test-vectors/av1-1-b8-02-allintra.ivf with dummy exif data. > > > > > > > Regards, > > > > Gyan > > > > _______________________________________________ > > > > 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 > > > > > > > > -- > > Vignesh > > Ping on this please. > > -- > Vignesh Another ping on this please. :) -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-13 16:32 ` Vignesh Venkatasubramanian 2022-06-21 17:12 ` Vignesh Venkatasubramanian @ 2022-06-28 18:21 ` James Zern 2022-06-28 18:56 ` Vignesh Venkatasubramanian 2022-06-28 18:58 ` Vignesh Venkatasubramanian 1 sibling, 2 replies; 23+ messages in thread From: James Zern @ 2022-06-28 18:21 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, Jun 13, 2022 at 9:32 AM Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote: > > On Fri, Jun 10, 2022 at 10:34 AM Vignesh Venkatasubramanian > <vigneshv@google.com> wrote: > > > > On Thu, Jun 9, 2022 at 12:50 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > > > > > > > > > On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > > > > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > > > >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > > > >> <vigneshv@google.com> wrote: > > > >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > > >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > > >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: > > > >>>>> Update the still AVIF parser to only read the primary item. With this > > > >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer > > > >>>>> fail to parse. > > > >>>>> > > > >>>>> For example, this patch enables parsing of files in: > > > >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > >>>>> > > > >>>> Can some of the failing files or similar ones be added to fate? > > > >>> There are no fate tests for AVIF parsing as of now. I was thinking > > > >>> about adding some for the muxing. But i am not sure what can be done > > > >>> here for the parsing. > > > >>> > > > >> Thanks. Are there any for general mov/mp4 parsing that could be > > > >> extended? This looks good otherwise. > > > > From what i see, most of the mov tests only seem to be for muxing. I'm > > > > not entirely certain about how to add a test for AVIF parsing. If > > > > anybody has an idea, i'd be open to adding it. > > > > > > Basic test would use the framemd5 muxer to compare demuxed packets with > > > a reference. > > > See fate-ffmpeg-streamloop > > > > > > > Thank you i have added a couple of fate tests. > > > > I am not sure how to add the AVIF files to the fate test server. I > > have them on Google Drive here: > > > > https://drive.google.com/file/d/1diZoM0Ew7Co3Yh5w5y1J-3IiBYVmUv9J/view?usp=sharing > > https://drive.google.com/file/d/1DdrD1mW36evt40a4RkeLYpx-oojmbc3z/view?usp=sharing > > > > These links should be publicly available without any sign in required. > > Please let me know if there is another preferred way of sharing test > > files and i can share it that way. > > > > Also, i forgot to mention that these files have to be uploaded under a > new directory called "avif" in the fate server. > Can you send a separate patch with the tests disabled and marked by a TODO to enable them after the files are uploaded? https://ffmpeg.org/developer.html#Adding-files-to-the-fate_002dsuite-dataset _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-28 18:21 ` James Zern @ 2022-06-28 18:56 ` Vignesh Venkatasubramanian 2022-06-28 19:02 ` James Zern 2022-06-28 18:58 ` Vignesh Venkatasubramanian 1 sibling, 1 reply; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-28 18:56 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Vignesh Venkatasubramanian Update the still AVIF parser to only read the primary item. With this patch, AVIF still images with exif/icc/alpha channel will no longer fail to parse. For example, this patch enables parsing of files in: https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft Adding two fate tests: 1) demuxing of still image with 1 item - this test will pass regardlesss of this patch. 2) demuxing of still image with 2 items - this test will fail without this patch and will pass with patch applied. Partially fixes trac ticket #7621 Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> --- libavformat/isom.h | 1 + libavformat/mov.c | 41 ++++++++++--------- tests/fate/mov.mak | 13 ++++++ .../fate/mov-avif-demux-still-image-1-item | 11 +++++ .../mov-avif-demux-still-image-multiple-items | 11 +++++ 5 files changed, 57 insertions(+), 20 deletions(-) create mode 100644 tests/ref/fate/mov-avif-demux-still-image-1-item create mode 100644 tests/ref/fate/mov-avif-demux-still-image-multiple-items diff --git a/libavformat/isom.h b/libavformat/isom.h index cf36f04d5b..f05c2d9c28 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -317,6 +317,7 @@ typedef struct MOVContext { uint32_t mfra_size; uint32_t max_stts_delta; int is_still_picture_avif; + int primary_item_id; } MOVContext; int ff_mp4_read_descr_len(AVIOContext *pb); diff --git a/libavformat/mov.c b/libavformat/mov.c index c6fbe511c0..88669faa70 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7470,6 +7470,13 @@ static int rb_size(AVIOContext *pb, uint64_t* value, int size) return size; } +static int mov_read_pitm(MOVContext *c, AVIOContext *pb, MOVAtom atom) +{ + avio_rb32(pb); // version & flags. + c->primary_item_id = avio_rb16(pb); + 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; @@ -7526,34 +7533,25 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) return AVERROR_PATCHWELCOME; } item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); - if (item_count > 1) { - // For still AVIF images, we only support one item. Second item will - // generally be found for AVIF images with alpha channel. We don't - // support them as of now. - av_log(c->fc, AV_LOG_ERROR, "iloc: item_count > 1 not supported.\n"); - return AVERROR_PATCHWELCOME; - } // Populate the necessary fields used by mov_build_index. - sc->stsc_count = item_count; - sc->stsc_data = av_malloc_array(item_count, sizeof(*sc->stsc_data)); + 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 = item_count; - sc->chunk_offsets = - av_malloc_array(item_count, sizeof(*sc->chunk_offsets)); + 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 = item_count; - sc->sample_sizes = - av_malloc_array(item_count, sizeof(*sc->sample_sizes)); + 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 = item_count; - sc->stts_data = av_malloc_array(item_count, sizeof(*sc->stts_data)); + 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; @@ -7561,7 +7559,7 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom) sc->stts_data[0].duration = 0; for (int i = 0; i < item_count; i++) { - (version < 2) ? avio_rb16(pb) : avio_rb32(pb); // item_id; + int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb); if (version > 0) avio_rb16(pb); // construction_method. avio_rb16(pb); // data_reference_index. @@ -7577,8 +7575,10 @@ 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; - sc->sample_sizes[0] = extent_length; - sc->chunk_offsets[0] = base_offset + extent_offset; + if (item_id == c->primary_item_id) { + sc->sample_sizes[0] = extent_length; + sc->chunk_offsets[0] = base_offset + extent_offset; + } } } @@ -7696,6 +7696,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = { { MKTAG('S','A','N','D'), mov_read_SAND }, /* non diegetic audio box */ { 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 }, { 0, NULL } }; diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak index 2fae054423..8a7218a215 100644 --- a/tests/fate/mov.mak +++ b/tests/fate/mov.mak @@ -17,6 +17,10 @@ FATE_MOV = fate-mov-3elist \ fate-mov-bbi-elst-starts-b \ fate-mov-neg-firstpts-discard-frames \ fate-mov-stream-shorter-than-movie \ +# FIXME: Uncomment these two lines once the test files are uploaded to the fate +# server. +# fate-mov-avif-demux-still-image-1-item \ +# fate-mov-avif-demux-still-image-multiple-items \ FATE_MOV_FFPROBE = fate-mov-neg-firstpts-discard \ fate-mov-neg-firstpts-discard-vorbis \ @@ -138,6 +142,15 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV, SRT_DEMUXER TTML fate-mov-mp4-ttml-stpp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" fate-mov-mp4-ttml-dfxp: CMD = transcode srt $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" "-of json -show_entries packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags" +# FIXME: Uncomment these two tests once the test files are uploaded to the fate +# server. +# avif demuxing - still image with 1 item. +#fate-mov-avif-demux-still-image-1-item: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image.avif -c:v copy + +# avif demuxing - still image with multiple items. only the primary item will be +# parsed. +#fate-mov-avif-demux-still-image-multiple-items: CMD = framemd5 -i $(TARGET_SAMPLES)/avif/still_image_exif.avif -c:v copy + # Resulting remux should have: # 1. first audio stream with AV_DISPOSITION_HEARING_IMPAIRED # 2. second audio stream with AV_DISPOSITION_VISUAL_IMPAIRED | DESCRIPTIONS diff --git a/tests/ref/fate/mov-avif-demux-still-image-1-item b/tests/ref/fate/mov-avif-demux-still-image-1-item new file mode 100644 index 0000000000..93773afd4e --- /dev/null +++ b/tests/ref/fate/mov-avif-demux-still-image-1-item @@ -0,0 +1,11 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#extradata 0, 13, b52ae298d37128862ef1918cf916239c +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: av1 +#dimensions 0: 352x288 +#sar 0: 1/1 +#stream#, dts, pts, duration, size, hash +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 diff --git a/tests/ref/fate/mov-avif-demux-still-image-multiple-items b/tests/ref/fate/mov-avif-demux-still-image-multiple-items new file mode 100644 index 0000000000..93773afd4e --- /dev/null +++ b/tests/ref/fate/mov-avif-demux-still-image-multiple-items @@ -0,0 +1,11 @@ +#format: frame checksums +#version: 2 +#hash: MD5 +#extradata 0, 13, b52ae298d37128862ef1918cf916239c +#tb 0: 1/1 +#media_type 0: video +#codec_id 0: av1 +#dimensions 0: 352x288 +#sar 0: 1/1 +#stream#, dts, pts, duration, size, hash +0, 0, 0, 1, 36265, 235b0c6e389c4084845981e08d60db04 -- 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". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-28 18:56 ` Vignesh Venkatasubramanian @ 2022-06-28 19:02 ` James Zern 2022-06-29 19:20 ` James Zern 0 siblings, 1 reply; 23+ messages in thread From: James Zern @ 2022-06-28 19:02 UTC (permalink / raw) To: FFmpeg development discussions and patches On Tue, Jun 28, 2022 at 11:56 AM Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org> wrote: > > Update the still AVIF parser to only read the primary item. With this > patch, AVIF still images with exif/icc/alpha channel will no longer > fail to parse. > > For example, this patch enables parsing of files in: > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > Adding two fate tests: > 1) demuxing of still image with 1 item - this test will pass regardlesss > of this patch. > 2) demuxing of still image with 2 items - this test will fail without > this patch and will pass with patch applied. > > Partially fixes trac ticket #7621 > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > --- > libavformat/isom.h | 1 + > libavformat/mov.c | 41 ++++++++++--------- > tests/fate/mov.mak | 13 ++++++ > .../fate/mov-avif-demux-still-image-1-item | 11 +++++ > .../mov-avif-demux-still-image-multiple-items | 11 +++++ > 5 files changed, 57 insertions(+), 20 deletions(-) > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-1-item > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-multiple-items > lgtm. I'll submit this soon if there aren't any 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". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-28 19:02 ` James Zern @ 2022-06-29 19:20 ` James Zern 0 siblings, 0 replies; 23+ messages in thread From: James Zern @ 2022-06-29 19:20 UTC (permalink / raw) To: FFmpeg development discussions and patches On Tue, Jun 28, 2022 at 12:02 PM James Zern <jzern@google.com> wrote: > > On Tue, Jun 28, 2022 at 11:56 AM Vignesh Venkatasubramanian > <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > Update the still AVIF parser to only read the primary item. With this > > patch, AVIF still images with exif/icc/alpha channel will no longer > > fail to parse. > > > > For example, this patch enables parsing of files in: > > https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > Adding two fate tests: > > 1) demuxing of still image with 1 item - this test will pass regardlesss I fixed the 'regardless' typo. > > of this patch. > > 2) demuxing of still image with 2 items - this test will fail without > > this patch and will pass with patch applied. > > > > Partially fixes trac ticket #7621 > > > > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com> > > --- > > libavformat/isom.h | 1 + > > libavformat/mov.c | 41 ++++++++++--------- > > tests/fate/mov.mak | 13 ++++++ > > .../fate/mov-avif-demux-still-image-1-item | 11 +++++ > > .../mov-avif-demux-still-image-multiple-items | 11 +++++ > > 5 files changed, 57 insertions(+), 20 deletions(-) > > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-1-item > > create mode 100644 tests/ref/fate/mov-avif-demux-still-image-multiple-items > > > > lgtm. I'll submit this soon if there aren't any comments. applied, thanks for the patch. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF 2022-06-28 18:21 ` James Zern 2022-06-28 18:56 ` Vignesh Venkatasubramanian @ 2022-06-28 18:58 ` Vignesh Venkatasubramanian 1 sibling, 0 replies; 23+ messages in thread From: Vignesh Venkatasubramanian @ 2022-06-28 18:58 UTC (permalink / raw) To: FFmpeg development discussions and patches On Tue, Jun 28, 2022 at 11:21 AM James Zern <jzern-at-google.com@ffmpeg.org> wrote: > > On Mon, Jun 13, 2022 at 9:32 AM Vignesh Venkatasubramanian > <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > On Fri, Jun 10, 2022 at 10:34 AM Vignesh Venkatasubramanian > > <vigneshv@google.com> wrote: > > > > > > On Thu, Jun 9, 2022 at 12:50 AM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > > > > > > > > > > > > > On 2022-06-08 10:51 pm, Vignesh Venkatasubramanian wrote: > > > > > On Thu, Jun 2, 2022 at 1:35 PM James Zern <jzern@google.com> wrote: > > > > >> On Wed, Jun 1, 2022 at 1:38 PM Vignesh Venkatasubramanian > > > > >> <vigneshv@google.com> wrote: > > > > >>> On Wed, Jun 1, 2022 at 10:30 AM James Zern <jzern@google.com> wrote: > > > > >>>> On Sun, Apr 24, 2022 at 11:35 AM Vignesh Venkatasubramanian > > > > >>>> <vigneshv-at-google.com@ffmpeg.org> wrote: > > > > >>>>> Update the still AVIF parser to only read the primary item. With this > > > > >>>>> patch, AVIF still images with exif/icc/alpha channel will no longer > > > > >>>>> fail to parse. > > > > >>>>> > > > > >>>>> For example, this patch enables parsing of files in: > > > > >>>>> https://github.com/AOMediaCodec/av1-avif/tree/master/testFiles/Microsoft > > > > >>>>> > > > > >>>> Can some of the failing files or similar ones be added to fate? > > > > >>> There are no fate tests for AVIF parsing as of now. I was thinking > > > > >>> about adding some for the muxing. But i am not sure what can be done > > > > >>> here for the parsing. > > > > >>> > > > > >> Thanks. Are there any for general mov/mp4 parsing that could be > > > > >> extended? This looks good otherwise. > > > > > From what i see, most of the mov tests only seem to be for muxing. I'm > > > > > not entirely certain about how to add a test for AVIF parsing. If > > > > > anybody has an idea, i'd be open to adding it. > > > > > > > > Basic test would use the framemd5 muxer to compare demuxed packets with > > > > a reference. > > > > See fate-ffmpeg-streamloop > > > > > > > > > > Thank you i have added a couple of fate tests. > > > > > > I am not sure how to add the AVIF files to the fate test server. I > > > have them on Google Drive here: > > > > > > https://drive.google.com/file/d/1diZoM0Ew7Co3Yh5w5y1J-3IiBYVmUv9J/view?usp=sharing > > > https://drive.google.com/file/d/1DdrD1mW36evt40a4RkeLYpx-oojmbc3z/view?usp=sharing > > > > > > These links should be publicly available without any sign in required. > > > Please let me know if there is another preferred way of sharing test > > > files and i can share it that way. > > > > > > > Also, i forgot to mention that these files have to be uploaded under a > > new directory called "avif" in the fate server. > > > > Can you send a separate patch with the tests disabled and marked by a > TODO to enable them after the files are uploaded? > https://ffmpeg.org/developer.html#Adding-files-to-the-fate_002dsuite-dataset I have updated this patch to comment out the actual tests (with a FIXME note). I can send separate patch once the files are added to the server to uncomment the tests and remove the FIXME. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". -- Vignesh _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-06-29 19:21 UTC | newest] Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-24 18:35 [FFmpeg-devel] [PATCH] avformat/mov: Only read the primary item for AVIF Vignesh Venkatasubramanian 2022-05-02 21:39 ` Vignesh Venkatasubramanian 2022-05-09 16:32 ` Vignesh Venkatasubramanian 2022-05-16 16:59 ` Vignesh Venkatasubramanian 2022-05-19 16:13 ` Vignesh Venkatasubramanian 2022-05-31 19:11 ` Vignesh Venkatasubramanian 2022-06-01 17:30 ` James Zern 2022-06-01 20:38 ` Vignesh Venkatasubramanian 2022-06-02 20:34 ` James Zern 2022-06-08 17:21 ` Vignesh Venkatasubramanian 2022-06-09 7:50 ` Gyan Doshi 2022-06-10 17:30 ` Vignesh Venkatasubramanian 2022-06-10 17:33 ` Andreas Rheinhardt 2022-06-10 17:37 ` Vignesh Venkatasubramanian 2022-06-10 17:34 ` Vignesh Venkatasubramanian 2022-06-13 16:32 ` Vignesh Venkatasubramanian 2022-06-21 17:12 ` Vignesh Venkatasubramanian 2022-06-27 16:44 ` Vignesh Venkatasubramanian 2022-06-28 18:21 ` James Zern 2022-06-28 18:56 ` Vignesh Venkatasubramanian 2022-06-28 19:02 ` James Zern 2022-06-29 19:20 ` James Zern 2022-06-28 18:58 ` Vignesh Venkatasubramanian
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git