| Message ID | 20220424183514.986374-1-vigneshv@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: Only read the primary item for AVIF | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
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? :)
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
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?
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.
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.
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
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
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".
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".
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
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.
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. :)
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
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".
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 } };
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(-)