From: Vignesh Venkatasubramanian <vigneshv-at-google.com@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items
Date: Thu, 28 Jul 2022 11:25:00 -0700
Message-ID: <CAOJaEP+QwOTeioQk=4z3fgKgZD8SZzncqphQuLB5TK=-zmpLTQ@mail.gmail.com> (raw)
In-Reply-To: <DB6PR0101MB22149E9944563EF16EC536938F979@DB6PR0101MB2214.eurprd01.prod.exchangelabs.com>
On Wed, Jul 27, 2022 at 12:40 PM Andreas Rheinhardt
<andreas.rheinhardt@outlook.com> wrote:
>
> Vignesh Venkatasubramanian:
> > Stores the item ids of all the items found in the file and
> > processes the primary item at the end of the meta box. This patch
> > does not change any behavior. It sets up the code for parsing
> > alpha channel (and possibly images with 'grid') in follow up
> > patches.
> >
> > Signed-off-by: Vignesh Venkatasubramanian <vigneshv@google.com>
> > ---
> > libavformat/isom.h | 4 ++
> > libavformat/mov.c | 146 ++++++++++++++++++++++++++++-----------------
> > 2 files changed, 95 insertions(+), 55 deletions(-)
> >
> > diff --git a/libavformat/isom.h b/libavformat/isom.h
> > index f05c2d9c28..d8b262e915 100644
> > --- a/libavformat/isom.h
> > +++ b/libavformat/isom.h
> > @@ -318,6 +318,10 @@ typedef struct MOVContext {
> > uint32_t max_stts_delta;
> > int is_still_picture_avif;
> > int primary_item_id;
> > + int *avif_item_ids;
> > + int avif_item_ids_size;
> > + int *avif_extent_lengths;
> > + int64_t *avif_extent_offsets;
>
> Why are these three different buffers instead of one buffer of struct {
> int avif_item_ids; int avif_extent_lengths; int64_t avif_extent_offsets;}?
>
Ah good point. Updated to use a struct and a size field.
> > } MOVContext;
> >
> > int ff_mp4_read_descr_len(AVIOContext *pb);
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index a09a762d91..fc6a691da4 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -4698,6 +4698,69 @@ static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > return ret;
> > }
> >
> > +static int avif_add_stream(MOVContext *c, int item_id)
> > +{
> > + MOVStreamContext *sc;
> > + AVStream *st;
> > + int item_index = -1;
> > + for (int i = 0; i < c->avif_item_ids_size; i++)
> > + if (c->avif_item_ids[i] == item_id) {
> > + item_index = i;
> > + break;
> > + }
> > + if (item_index < 0)
> > + return AVERROR_INVALIDDATA;
> > + st = avformat_new_stream(c->fc, NULL);
> > + if (!st)
> > + return AVERROR(ENOMEM);
> > + st->id = c->fc->nb_streams;
> > + sc = av_mallocz(sizeof(MOVStreamContext));
> > + if (!sc)
> > + return AVERROR(ENOMEM);
> > +
> > + st->priv_data = sc;
> > + st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> > + st->codecpar->codec_id = AV_CODEC_ID_AV1;
> > + sc->ffindex = st->index;
> > + c->trak_index = st->index;
> > + st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> > + st->time_base.num = st->time_base.den = 1;
> > + st->nb_frames = 1;
> > + sc->time_scale = 1;
> > + sc = st->priv_data;
> > + sc->pb = c->fc->pb;
> > + sc->pb_is_copied = 1;
> > +
> > + // Populate the necessary fields used by mov_build_index.
> > + sc->stsc_count = 1;
> > + sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> > + if (!sc->stsc_data)
> > + return AVERROR(ENOMEM);
> > + sc->stsc_data[0].first = 1;
> > + sc->stsc_data[0].count = 1;
> > + sc->stsc_data[0].id = 1;
> > + sc->chunk_count = 1;
> > + sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> > + if (!sc->chunk_offsets)
> > + return AVERROR(ENOMEM);
> > + sc->sample_count = 1;
> > + sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> > + if (!sc->sample_sizes)
> > + return AVERROR(ENOMEM);
> > + sc->stts_count = 1;
> > + sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> > + if (!sc->stts_data)
> > + return AVERROR(ENOMEM);
> > + sc->stts_data[0].count = 1;
> > + // Not used for still images. But needed by mov_build_index.
> > + sc->stts_data[0].duration = 0;
> > + sc->sample_sizes[0] = c->avif_extent_lengths[item_index];
> > + sc->chunk_offsets[0] = c->avif_extent_offsets[item_index];
> > +
> > + mov_build_index(c, st);
> > + return 0;
> > +}
> > +
> > static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > {
> > while (atom.size > 8) {
> > @@ -4707,9 +4770,23 @@ static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > tag = avio_rl32(pb);
> > atom.size -= 4;
> > if (tag == MKTAG('h','d','l','r')) {
> > + int ret;
> > avio_seek(pb, -8, SEEK_CUR);
> > atom.size += 8;
> > - return mov_read_default(c, pb, atom);
> > + if ((ret = mov_read_default(c, pb, atom)) < 0)
> > + return ret;
> > + if (c->is_still_picture_avif) {
> > + int ret;
> > + // Add a stream for the YUV planes (primary item).
> > + if ((ret = avif_add_stream(c, c->primary_item_id)) < 0)
> > + return ret;
> > + // For still AVIF images, the meta box contains all the
> > + // necessary information that would generally be provided by the
> > + // moov box. So simply mark that we have found the moov box so
> > + // that parsing can continue.
> > + c->found_moov = 1;
> > + }
> > + return ret;
> > }
> > }
> > return 0;
> > @@ -7478,8 +7555,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > int item_count, extent_count;
> > uint64_t base_offset, extent_offset, extent_length;
> > uint8_t value;
> > - AVStream *st;
> > - MOVStreamContext *sc;
> >
> > if (!c->is_still_picture_avif) {
> > // * For non-avif, we simply ignore the iloc box.
> > @@ -7493,27 +7568,6 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > return 0;
> > }
> >
> > - st = avformat_new_stream(c->fc, NULL);
> > - if (!st)
> > - return AVERROR(ENOMEM);
> > - st->id = c->fc->nb_streams;
> > - sc = av_mallocz(sizeof(MOVStreamContext));
> > - if (!sc)
> > - return AVERROR(ENOMEM);
> > -
> > - st->priv_data = sc;
> > - st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> > - st->codecpar->codec_id = AV_CODEC_ID_AV1;
> > - sc->ffindex = st->index;
> > - c->trak_index = st->index;
> > - st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
> > - st->time_base.num = st->time_base.den = 1;
> > - st->nb_frames = 1;
> > - sc->time_scale = 1;
> > - sc = st->priv_data;
> > - sc->pb = c->fc->pb;
> > - sc->pb_is_copied = 1;
> > -
> > version = avio_r8(pb);
> > avio_rb24(pb); // flags.
> >
> > @@ -7529,34 +7583,22 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > }
> > item_count = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
> >
> > - // Populate the necessary fields used by mov_build_index.
> > - sc->stsc_count = 1;
> > - sc->stsc_data = av_malloc_array(1, sizeof(*sc->stsc_data));
> > - if (!sc->stsc_data)
> > + c->avif_item_ids = av_malloc_array(item_count, sizeof(*c->avif_item_ids));
> > + if (!c->avif_item_ids)
> > return AVERROR(ENOMEM);
> > - sc->stsc_data[0].first = 1;
> > - sc->stsc_data[0].count = 1;
> > - sc->stsc_data[0].id = 1;
> > - sc->chunk_count = 1;
> > - sc->chunk_offsets = av_malloc_array(1, sizeof(*sc->chunk_offsets));
> > - if (!sc->chunk_offsets)
> > + c->avif_item_ids_size = item_count;
> > + c->avif_extent_lengths = av_malloc_array(item_count, sizeof(*c->avif_extent_lengths));
> > + if (!c->avif_extent_lengths)
> > return AVERROR(ENOMEM);
> > - sc->sample_count = 1;
> > - sc->sample_sizes = av_malloc_array(1, sizeof(*sc->sample_sizes));
> > - if (!sc->sample_sizes)
> > + c->avif_extent_offsets = av_malloc_array(item_count, sizeof(*c->avif_extent_offsets));
> > + if (!c->avif_extent_offsets)
> > return AVERROR(ENOMEM);
> > - sc->stts_count = 1;
> > - sc->stts_data = av_malloc_array(1, sizeof(*sc->stts_data));
> > - if (!sc->stts_data)
> > - return AVERROR(ENOMEM);
> > - sc->stts_data[0].count = 1;
> > - // Not used for still images. But needed by mov_build_index.
> > - sc->stts_data[0].duration = 0;
> >
> > for (int i = 0; i < item_count; i++) {
> > int item_id = (version < 2) ? avio_rb16(pb) : avio_rb32(pb);
> > if (avio_feof(pb))
> > return AVERROR_INVALIDDATA;
> > + c->avif_item_ids[i] = item_id;
> > if (version > 0)
> > avio_rb16(pb); // construction_method.
> > avio_rb16(pb); // data_reference_index.
> > @@ -7572,20 +7614,11 @@ static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> > if (rb_size(pb, &extent_offset, offset_size) < 0 ||
> > rb_size(pb, &extent_length, length_size) < 0)
> > return AVERROR_INVALIDDATA;
> > - if (item_id == c->primary_item_id) {
> > - sc->sample_sizes[0] = extent_length;
> > - sc->chunk_offsets[0] = base_offset + extent_offset;
> > - }
> > + c->avif_extent_lengths[i] = extent_length;
> > + c->avif_extent_offsets[i] = base_offset + extent_offset;
> > }
> > }
> >
> > - mov_build_index(c, st);
> > -
> > - // For still AVIF images, the iloc box contains all the necessary
> > - // information that would generally be provided by the moov box. So simply
> > - // mark that we have found the moov box so that parsing can continue.
> > - c->found_moov = 1;
> > -
> > return atom.size;
> > }
> >
> > @@ -8189,6 +8222,9 @@ static int mov_read_close(AVFormatContext *s)
> >
> > av_freep(&mov->aes_decrypt);
> > av_freep(&mov->chapter_tracks);
> > + av_freep(&mov->avif_item_ids);
> > + av_freep(&mov->avif_extent_lengths);
> > + av_freep(&mov->avif_extent_offsets);
> >
> > return 0;
> > }
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
--
Vignesh
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
next prev parent reply other threads:[~2022-07-28 18:25 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-30 21:04 Vignesh Venkatasubramanian
2022-06-30 21:04 ` [FFmpeg-devel] [PATCH 2/2] avformat/mov: Support parsing of still AVIF Alpha Channel Vignesh Venkatasubramanian
2022-07-02 9:34 ` Anton Khirnov
2022-07-02 16:32 ` Vignesh Venkatasubramanian
2022-07-02 19:34 ` Jan Ekström
2022-07-02 21:15 ` Vignesh Venkatasubramanian
2022-07-03 12:17 ` Jan Ekström
2022-07-03 16:52 ` Vignesh Venkatasubramanian
2022-07-05 16:53 ` Anton Khirnov
2022-07-12 15:17 ` Vignesh Venkatasubramanian
2022-07-11 22:25 ` [FFmpeg-devel] [PATCH 1/2] avformat/mov: Rework the AVIF parser to handle multiple items James Zern
2022-07-13 16:11 ` Vignesh Venkatasubramanian
2022-07-13 16:12 ` Vignesh Venkatasubramanian
2022-07-22 18:20 ` Vignesh Venkatasubramanian
2022-07-26 19:02 ` James Zern
2022-07-27 16:12 ` Vignesh Venkatasubramanian
2022-07-27 19:37 ` James Zern
2022-07-27 19:40 ` Andreas Rheinhardt
2022-07-28 18:25 ` Vignesh Venkatasubramanian [this message]
2022-07-28 18:25 ` Vignesh Venkatasubramanian
2022-08-02 16:54 ` James Zern
2022-08-09 20:20 ` James Zern
2022-07-27 16:12 ` Vignesh Venkatasubramanian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOJaEP+QwOTeioQk=4z3fgKgZD8SZzncqphQuLB5TK=-zmpLTQ@mail.gmail.com' \
--to=vigneshv-at-google.com@ffmpeg.org \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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