diff mbox series

[FFmpeg-devel,2/2] avformat/mov: improve HEIF parsing

Message ID 20240109195545.59231-2-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] avformat/mov: rename avif fields to heif | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 fail Make fate failed
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

James Almer Jan. 9, 2024, 7:55 p.m. UTC
Parse iinf boxes and its child infe boxes to get the actual codec used
(AV1 for avif, HEVC for heic), and properly export extradata in a generic
way.

The avif tests reference files are updated as the extradata is now exported.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/isom.h                            |   3 +-
 libavformat/mov.c                             | 157 ++++++++++--------
 .../fate/mov-avif-demux-still-image-1-item    |   2 +-
 .../mov-avif-demux-still-image-multiple-items |   2 +-
 4 files changed, 95 insertions(+), 69 deletions(-)

Comments

Wu Jianhua April 13, 2024, 11:04 a.m. UTC | #1
> 发件人: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> 代表 James Almer <jamrial@gmail.com>
> 发送时间: 2024年1月9日 11:55
> 收件人: ffmpeg-devel@ffmpeg.org
> 主题: [FFmpeg-devel] [PATCH 2/2] avformat/mov: improve HEIF parsing
>
> Parse iinf boxes and its child infe boxes to get the actual codec used
> (AV1 for avif, HEVC for heic), and properly export extradata in a generic
> way.
>
> The avif tests reference files are updated as the extradata is now exported.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavformat/isom.h                            |   3 +-
> libavformat/mov.c                             | 157 ++++++++++--------
> .../fate/mov-avif-demux-still-image-1-item    |   2 +-
> .../mov-avif-demux-still-image-multiple-items |   2 +-
> 4 files changed, 95 insertions(+), 69 deletions(-)
>

> +    if (version != 2) {
> +        av_log(c->fc, AV_LOG_ERROR, "infe: version != 2 not supported.\n");
> +        return AVERROR_PATCHWELCOME;
> +    }
> +

Hi James,

With the change, some errors occurred and the current FFmpeg failed to decode a lot of videos that can be decoded by older FFmpeg.
[mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] st: 0 edit list: 1 Missing key frame while searching for timestamp: 1000
[mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] st: 0 edit list 1 Cannot find an index entry before timestamp: 1000.
[mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] infe: version < 2 not supported
[mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] error reading header

I'm not familiar with the mov. Is it possible to treat this error as a warning and skip it when the version < 2?

Thanks,
Jianhua

James Almer April 13, 2024, 1:12 p.m. UTC | #2
On 4/13/2024 8:04 AM, Wu Jianhua wrote:
>> 发件人: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> 代表 James Almer <jamrial@gmail.com>
>> 发送时间: 2024年1月9日 11:55
>> 收件人: ffmpeg-devel@ffmpeg.org
>> 主题: [FFmpeg-devel] [PATCH 2/2] avformat/mov: improve HEIF parsing
>>
>> Parse iinf boxes and its child infe boxes to get the actual codec used
>> (AV1 for avif, HEVC for heic), and properly export extradata in a generic
>> way.
>>
>> The avif tests reference files are updated as the extradata is now exported.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavformat/isom.h                            |   3 +-
>> libavformat/mov.c                             | 157 ++++++++++--------
>> .../fate/mov-avif-demux-still-image-1-item    |   2 +-
>> .../mov-avif-demux-still-image-multiple-items |   2 +-
>> 4 files changed, 95 insertions(+), 69 deletions(-)
>>
> 
>> +    if (version != 2) {
>> +        av_log(c->fc, AV_LOG_ERROR, "infe: version != 2 not supported.\n");
>> +        return AVERROR_PATCHWELCOME;
>> +    }
>> +
> 
> Hi James,
> 
> With the change, some errors occurred and the current FFmpeg failed to decode a lot of videos that can be decoded by older FFmpeg.
> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] st: 0 edit list: 1 Missing key frame while searching for timestamp: 1000
> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] st: 0 edit list 1 Cannot find an index entry before timestamp: 1000.
> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] infe: version < 2 not supported
> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] error reading header
> 
> I'm not familiar with the mov. Is it possible to treat this error as a warning and skip it when the version < 2?
> 
> Thanks,
> Jianhua

Can you test 
https://ffmpeg.org//pipermail/ffmpeg-devel/2024-April/325644.html ?
Wu Jianhua April 13, 2024, 2:20 p.m. UTC | #3
> 发件人: James Almer <jamrial@gmail.com>
> 发送时间: 2024年4月13日 6:12
> 收件人: Wu Jianhua; ffmpeg-devel@ffmpeg.org
> 主题: Re: 回复: [FFmpeg-devel] [PATCH 2/2] avformat/mov: improve HEIF parsing
> 
> On 4/13/2024 8:04 AM, Wu Jianhua wrote:
>>> 发件人: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> 代表 James Almer <jamrial@gmail.com>
>>> 发送时间: 2024年1月9日 11:55
>>> 收件人: ffmpeg-devel@ffmpeg.org
>>> 主题: [FFmpeg-devel] [PATCH 2/2] avformat/mov: improve HEIF parsing
>>>
>>> Parse iinf boxes and its child infe boxes to get the actual codec used
>>> (AV1 for avif, HEVC for heic), and properly export extradata in a generic
>>> way.
>>>
>>> The avif tests reference files are updated as the extradata is now exported.
>>>
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> libavformat/isom.h                            |   3 +-
>>> libavformat/mov.c                             | 157 ++++++++++--------
>>> .../fate/mov-avif-demux-still-image-1-item    |   2 +-
>>> .../mov-avif-demux-still-image-multiple-items |   2 +-
>>> 4 files changed, 95 insertions(+), 69 deletions(-)
>>>
>>
>>> +    if (version != 2) {
>>> +        av_log(c->fc, AV_LOG_ERROR, "infe: version != 2 not supported.\n");
>>> +        return AVERROR_PATCHWELCOME;
>>> +    }
>>> +
>>
>> Hi James,
>> 
>> With the change, some errors occurred and the current FFmpeg failed to decode a lot of videos that can be decoded by older FFmpeg.
>> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] st: 0 edit list: 1 Missing key frame while searching for timestamp: 1000
>> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] st: 0 edit list 1 Cannot find an index entry before timestamp: 1000.
>> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] infe: version < 2 not supported
>> [mov,mp4,m4a,3gp,3g2,mj2 @ 000001e6e79aac80] error reading header
>> 
>> I'm not familiar with the mov. Is it possible to treat this error as a warning and skip it when the version < 2?
>> 
>> Thanks,
>> Jianhua
>
> Can you test
> https://ffmpeg.org//pipermail/ffmpeg-devel/2024-April/325644.html ?

It works. Thanks for you quick fix!
diff mbox series

Patch

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 90c4fb5530..107e2ce9bb 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -268,6 +268,7 @@  typedef struct MOVContext {
     int time_scale;
     int64_t duration;     ///< duration of the longest track
     int found_moov;       ///< 'moov' atom has been found
+    int found_iloc;       ///< 'iloc' atom has been found
     int found_mdat;       ///< 'mdat' atom has been found
     int found_hdlr_mdta;  ///< 'hdlr' atom with type 'mdta' has been found
     int trak_index;       ///< Index of the current 'trak'
@@ -327,8 +328,6 @@  typedef struct MOVContext {
         int64_t extent_offset;
     } *heif_info;
     int heif_info_size;
-    int64_t hvcC_offset;
-    int hvcC_size;
     int interleaved_read;
 } MOVContext;
 
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 12e82c66a9..47d0d41214 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1233,8 +1233,6 @@  static int mov_read_ftyp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         c->isom = 1;
     av_log(c->fc, AV_LOG_DEBUG, "ISO: File Type Major Brand: %.4s\n",(char *)&type);
     av_dict_set(&c->fc->metadata, "major_brand", type, 0);
-    c->is_still_picture_avif = !strncmp(type, "avif", 4) ||
-                               !strncmp(type, "mif1", 4);
     minor_ver = avio_rb32(pb); /* minor version */
     av_dict_set_int(&c->fc->metadata, "minor_version", minor_ver, 0);
 
@@ -4641,10 +4639,6 @@  static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     MOVStreamContext *sc;
     int ret;
 
-    if (c->is_still_picture_avif) {
-        return AVERROR_INVALIDDATA;
-    }
-
     st = avformat_new_stream(c->fc, NULL);
     if (!st) return AVERROR(ENOMEM);
     st->id = -1;
@@ -4916,7 +4910,7 @@  static int mov_read_custom(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     return ret;
 }
 
-static int heif_add_stream(MOVContext *c, int item_id)
+static int heif_add_stream(MOVContext *c, int item_id, uint32_t item_type)
 {
     MOVStreamContext *sc;
     AVStream *st;
@@ -4940,20 +4934,7 @@  static int heif_add_stream(MOVContext *c, int item_id)
 
     st->priv_data = sc;
     st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
-    st->codecpar->codec_id = AV_CODEC_ID_AV1;
-    if (c->hvcC_offset >= 0) {
-        int ret;
-        int64_t pos = avio_tell(c->fc->pb);
-        st->codecpar->codec_id = AV_CODEC_ID_HEVC;
-        if (avio_seek(c->fc->pb, c->hvcC_offset, SEEK_SET) != c->hvcC_offset) {
-            av_log(c->fc, AV_LOG_ERROR, "Failed to seek to hvcC data.\n");
-            return AVERROR_UNKNOWN;
-        }
-        ret = ff_get_extradata(c->fc, st->codecpar, c->fc->pb, c->hvcC_size);
-        if (ret < 0)
-            return ret;
-        avio_seek(c->fc->pb, pos, SEEK_SET);
-    }
+    st->codecpar->codec_id = mov_codec_id(st, item_type);
     sc->ffindex = st->index;
     c->trak_index = st->index;
     st->avg_frame_rate.num = st->avg_frame_rate.den = 1;
@@ -4996,8 +4977,6 @@  static int heif_add_stream(MOVContext *c, int item_id)
 
 static int mov_read_meta(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-    c->hvcC_offset = -1;
-    c->hvcC_size = 0;
     while (atom.size > 8) {
         uint32_t tag;
         if (avio_feof(pb))
@@ -5005,23 +4984,9 @@  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;
-            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 = heif_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 mov_read_default(c, pb, atom);
         }
     }
     return 0;
@@ -7813,18 +7778,10 @@  static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     uint64_t base_offset, extent_offset, extent_length;
     uint8_t value;
 
-    if (!c->is_still_picture_avif) {
-        // * For non-avif, we simply ignore the iloc box.
-        // * For animated avif, we don't care about the iloc box as all the
-        //   necessary information can be found in the moov box.
-        return 0;
-    }
-
-    if (c->heif_info) {
+    if (c->found_iloc) {
         av_log(c->fc, AV_LOG_INFO, "Duplicate iloc box found\n");
         return 0;
     }
-    av_assert0(!c->fc->nb_streams);
 
     version = avio_r8(pb);
     avio_rb24(pb);  // flags.
@@ -7872,29 +7829,98 @@  static int mov_read_iloc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         }
     }
 
+    c->found_iloc = 1;
     return atom.size;
 }
 
-static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+static int mov_read_infe(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-    int size = avio_rb32(pb);
-    if (avio_rl32(pb) != MKTAG('i','p','c','o'))
-        return AVERROR_INVALIDDATA;
+    char item_name[128];
+    int64_t size = atom.size;
+    uint32_t item_type;
+    int item_id;
+    int version, ret;
+
+    version = avio_r8(pb);
+    avio_rb24(pb);  // flags.
+    size -= 4;
+
+    if (version != 2) {
+        av_log(c->fc, AV_LOG_ERROR, "infe: version != 2 not supported.\n");
+        return AVERROR_PATCHWELCOME;
+    }
+
+    item_id = avio_rb16(pb);
+    avio_rb16(pb); // item_protection_index
+    item_type = avio_rl32(pb);
     size -= 8;
-    while (size > 0) {
-        int sub_size, sub_type;
-        sub_size = avio_rb32(pb);
-        sub_type = avio_rl32(pb);
-        sub_size -= 8;
-        size -= sub_size + 8;
-        if (sub_type == MKTAG('h','v','c','C')) {
-            c->hvcC_offset = avio_tell(pb);
-            c->hvcC_size = sub_size;
-            break;
+    size -= avio_get_str(pb, INT_MAX, item_name, sizeof(item_name));
+
+    // Skip all but the primary item until support is added
+    if (item_id != c->primary_item_id)
+        return 0;
+
+    if (size > 0)
+        avio_skip(pb, size);
+
+    switch (item_type) {
+    case MKTAG('a','v','0','1'):
+    case MKTAG('h','v','c','1'):
+        if (!c->heif_info) {
+            av_log(c->fc, AV_LOG_ERROR, "Missing iloc before iinf\n");
+            return AVERROR_INVALIDDATA;
         }
-        avio_skip(pb, sub_size);
+        ret = heif_add_stream(c, item_id, item_type);
+        if (ret < 0)
+            return ret;
+        break;
+    default:
+        av_log(c->fc, AV_LOG_DEBUG, "infe: ignoring item_type %s.\n", av_fourcc2str(item_type));
+        break;
     }
-    return atom.size;
+
+    return 0;
+}
+
+static int mov_read_iinf(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+    int entry_count;
+    int version, ret;
+
+    version = avio_r8(pb);
+    avio_rb24(pb);  // flags.
+    entry_count = version ? avio_rb32(pb) : avio_rb16(pb);
+
+    for (int i = 0; i < entry_count; i++) {
+        MOVAtom infe;
+
+        infe.size = avio_rb32(pb) - 8;
+        infe.type = avio_rl32(pb);
+        ret = mov_read_infe(c, pb, infe);
+        if (ret < 0)
+            return ret;
+    }
+
+    return 0;
+}
+
+static int mov_read_iprp(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+    MOVAtom ipco;
+    int ret;
+
+    ipco.size = avio_rb32(pb) - 8;
+    ipco.type = avio_rl32(pb);
+    if (ipco.type != MKTAG('i','p','c','o'))
+        return AVERROR_INVALIDDATA;
+    if (avio_feof(pb))
+        return AVERROR_INVALIDDATA;
+
+    ret = mov_read_default(c, pb, ipco);
+    if (ret < 0)
+        return ret;
+
+    return 0;
 }
 
 static const MOVParseTableEntry mov_default_parse_table[] = {
@@ -8005,6 +8031,7 @@  static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('p','i','t','m'), mov_read_pitm },
 { MKTAG('e','v','c','C'), mov_read_glbl },
 { MKTAG('i','p','r','p'), mov_read_iprp },
+{ MKTAG('i','i','n','f'), mov_read_iinf },
 { 0, NULL }
 };
 
@@ -8672,8 +8699,8 @@  static int mov_read_header(AVFormatContext *s)
             av_log(s, AV_LOG_ERROR, "error reading header\n");
             return err;
         }
-    } while ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mov->found_moov && !mov->moov_retry++);
-    if (!mov->found_moov) {
+    } while ((pb->seekable & AVIO_SEEKABLE_NORMAL) && !mov->found_moov && !mov->found_iloc && !mov->moov_retry++);
+    if (!mov->found_moov && !mov->found_iloc) {
         av_log(s, AV_LOG_ERROR, "moov atom not found\n");
         return AVERROR_INVALIDDATA;
     }
diff --git a/tests/ref/fate/mov-avif-demux-still-image-1-item b/tests/ref/fate/mov-avif-demux-still-image-1-item
index 93773afd4e..770c98f868 100644
--- a/tests/ref/fate/mov-avif-demux-still-image-1-item
+++ b/tests/ref/fate/mov-avif-demux-still-image-1-item
@@ -1,7 +1,7 @@ 
 #format: frame checksums
 #version: 2
 #hash: MD5
-#extradata 0,                              13, b52ae298d37128862ef1918cf916239c
+#extradata 0,                               4, b24b71499a8480fa4469bcbcba2140aa
 #tb 0: 1/1
 #media_type 0: video
 #codec_id 0: av1
diff --git a/tests/ref/fate/mov-avif-demux-still-image-multiple-items b/tests/ref/fate/mov-avif-demux-still-image-multiple-items
index 93773afd4e..770c98f868 100644
--- a/tests/ref/fate/mov-avif-demux-still-image-multiple-items
+++ b/tests/ref/fate/mov-avif-demux-still-image-multiple-items
@@ -1,7 +1,7 @@ 
 #format: frame checksums
 #version: 2
 #hash: MD5
-#extradata 0,                              13, b52ae298d37128862ef1918cf916239c
+#extradata 0,                               4, b24b71499a8480fa4469bcbcba2140aa
 #tb 0: 1/1
 #media_type 0: video
 #codec_id 0: av1