| Message ID | tencent_A68553075C4A8B8A661FD0A926C4A7D9EA06@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Sat, 25 Dec 2021, Zhao Zhili wrote: > Fix #8883. > --- > doc/demuxers.texi | 6 ++++++ > libavformat/isom.h | 1 + > libavformat/mov.c | 9 +++++++-- > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/doc/demuxers.texi b/doc/demuxers.texi > index cab8a7072c..276b536ac5 100644 > --- a/doc/demuxers.texi > +++ b/doc/demuxers.texi > @@ -713,6 +713,12 @@ specify. > > @item decryption_key > 16-byte key, in hex, to decrypt files encrypted using ISO Common Encryption (CENC/AES-128 CTR; ISO/IEC 23001-7). > + > +@item use_hoov > +Whether to use hoov atom as moov atom to support some broken files. Set it to 0 for disable, 1 for enable, > +-1 for automatic, which means take hoov as moov in the second pass if moov doesn't been found in the first > +pass. > + > @end table > > @subsection Audible AAX > diff --git a/libavformat/isom.h b/libavformat/isom.h > index ef8f19b18c..7c837811a6 100644 > --- a/libavformat/isom.h > +++ b/libavformat/isom.h > @@ -305,6 +305,7 @@ typedef struct MOVContext { > int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd > int have_read_mfra_size; > uint32_t mfra_size; > + int use_hoov; > } MOVContext; > > int ff_mp4_read_descr_len(AVIOContext *pb); > diff --git a/libavformat/mov.c b/libavformat/mov.c > index ea2f010aa0..82237f930b 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -7325,7 +7325,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom) > a.size = avio_rb32(pb); > a.type = avio_rl32(pb); > if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) || > - a.type == MKTAG('h','o','o','v')) && > + a.type == MKTAG('h','o','o','v') && c->use_hoov == 1) && > a.size >= 8 && > c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) { > uint32_t type; > @@ -7951,8 +7951,12 @@ static int mov_read_header(AVFormatContext *s) > > /* check MOV header */ > do { > - if (mov->moov_retry) > + if (mov->moov_retry) { > avio_seek(pb, 0, SEEK_SET); > + /* reset use_hoov from auto mode to force mode in second pass */ > + if (mov->use_hoov == -1) > + mov->use_hoov = 1; > + } > if ((err = mov_read_default(mov, pb, atom)) < 0) { > av_log(s, AV_LOG_ERROR, "error reading header\n"); > return err; > @@ -8593,6 +8597,7 @@ static const AVOption mov_options[] = { > { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM }, > { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL, > {.i64 = 0}, 0, 1, FLAGS }, > + { "use_hoov", "Take hoov as moov atom, 0 for disable, -1 for auto, 1 means force", OFFSET(use_hoov), AV_OPT_TYPE_INT, {.i64 = 1}, -1, 1, FLAGS }, Kind of unusual that the default is 1 and not -1 (automatic). Is it intentional? Does it make sense to find hoov atom first and discard moov? Can't that cause problems for valid files? If it can, then maybe -1 should be the default, no? Thanks, Marton
> 在 2021年12月25日，18:36，Marton Balint <cus@passwd.hu> 写道： > > > > On Sat, 25 Dec 2021, Zhao Zhili wrote: > >> Fix #8883. >> --- >> doc/demuxers.texi | 6 ++++++ >> libavformat/isom.h | 1 + >> libavformat/mov.c | 9 +++++++-- >> 3 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/doc/demuxers.texi b/doc/demuxers.texi >> index cab8a7072c..276b536ac5 100644 >> --- a/doc/demuxers.texi >> +++ b/doc/demuxers.texi >> @@ -713,6 +713,12 @@ specify. >> >> @item decryption_key >> 16-byte key, in hex, to decrypt files encrypted using ISO Common Encryption (CENC/AES-128 CTR; ISO/IEC 23001-7). >> + >> +@item use_hoov >> +Whether to use hoov atom as moov atom to support some broken files. Set it to 0 for disable, 1 for enable, >> +-1 for automatic, which means take hoov as moov in the second pass if moov doesn't been found in the first >> +pass. >> + >> @end table >> >> @subsection Audible AAX >> diff --git a/libavformat/isom.h b/libavformat/isom.h >> index ef8f19b18c..7c837811a6 100644 >> --- a/libavformat/isom.h >> +++ b/libavformat/isom.h >> @@ -305,6 +305,7 @@ typedef struct MOVContext { >> int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd >> int have_read_mfra_size; >> uint32_t mfra_size; >> + int use_hoov; >> } MOVContext; >> >> int ff_mp4_read_descr_len(AVIOContext *pb); >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index ea2f010aa0..82237f930b 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -7325,7 +7325,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom) >> a.size = avio_rb32(pb); >> a.type = avio_rl32(pb); >> if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) || >> - a.type == MKTAG('h','o','o','v')) && >> + a.type == MKTAG('h','o','o','v') && c->use_hoov == 1) && >> a.size >= 8 && >> c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) { >> uint32_t type; >> @@ -7951,8 +7951,12 @@ static int mov_read_header(AVFormatContext *s) >> >> /* check MOV header */ >> do { >> - if (mov->moov_retry) >> + if (mov->moov_retry) { >> avio_seek(pb, 0, SEEK_SET); >> + /* reset use_hoov from auto mode to force mode in second pass */ >> + if (mov->use_hoov == -1) >> + mov->use_hoov = 1; >> + } >> if ((err = mov_read_default(mov, pb, atom)) < 0) { >> av_log(s, AV_LOG_ERROR, "error reading header\n"); >> return err; >> @@ -8593,6 +8597,7 @@ static const AVOption mov_options[] = { >> { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM }, >> { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL, >> {.i64 = 0}, 0, 1, FLAGS }, >> + { "use_hoov", "Take hoov as moov atom, 0 for disable, -1 for auto, 1 means force", OFFSET(use_hoov), AV_OPT_TYPE_INT, {.i64 = 1}, -1, 1, FLAGS }, > > Kind of unusual that the default is 1 and not -1 (automatic). Is it intentional? Does it make sense to find hoov atom first and discard moov? Can't that cause problems for valid files? If it can, then maybe -1 should be the default, no? I have sent a patch for same method, and i don’t think this is a better way. I prefer the method: https://patchwork.ffmpeg.org/project/ffmpeg/patch/20211223071012.52638-1-lq@chinaffmpeg.org/ > > Thanks, > Marton > _______________________________________________ > 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". Thanks Steven
> On Dec 25, 2021, at 6:36 PM, Marton Balint <cus@passwd.hu> wrote: > > On Sat, 25 Dec 2021, Zhao Zhili wrote: > >> Fix #8883. >> --- >> doc/demuxers.texi | 6 ++++++ >> libavformat/isom.h | 1 + >> libavformat/mov.c | 9 +++++++-- >> 3 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/doc/demuxers.texi b/doc/demuxers.texi >> index cab8a7072c..276b536ac5 100644 >> --- a/doc/demuxers.texi >> +++ b/doc/demuxers.texi >> @@ -713,6 +713,12 @@ specify. >> >> @item decryption_key >> 16-byte key, in hex, to decrypt files encrypted using ISO Common Encryption (CENC/AES-128 CTR; ISO/IEC 23001-7). >> + >> +@item use_hoov >> +Whether to use hoov atom as moov atom to support some broken files. Set it to 0 for disable, 1 for enable, >> +-1 for automatic, which means take hoov as moov in the second pass if moov doesn't been found in the first >> +pass. >> + >> @end table >> >> @subsection Audible AAX >> diff --git a/libavformat/isom.h b/libavformat/isom.h >> index ef8f19b18c..7c837811a6 100644 >> --- a/libavformat/isom.h >> +++ b/libavformat/isom.h >> @@ -305,6 +305,7 @@ typedef struct MOVContext { >> int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd >> int have_read_mfra_size; >> uint32_t mfra_size; >> + int use_hoov; >> } MOVContext; >> >> int ff_mp4_read_descr_len(AVIOContext *pb); >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index ea2f010aa0..82237f930b 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -7325,7 +7325,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom) >> a.size = avio_rb32(pb); >> a.type = avio_rl32(pb); >> if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) || >> - a.type == MKTAG('h','o','o','v')) && >> + a.type == MKTAG('h','o','o','v') && c->use_hoov == 1) && >> a.size >= 8 && >> c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) { >> uint32_t type; >> @@ -7951,8 +7951,12 @@ static int mov_read_header(AVFormatContext *s) >> >> /* check MOV header */ >> do { >> - if (mov->moov_retry) >> + if (mov->moov_retry) { >> avio_seek(pb, 0, SEEK_SET); >> + /* reset use_hoov from auto mode to force mode in second pass */ >> + if (mov->use_hoov == -1) >> + mov->use_hoov = 1; >> + } >> if ((err = mov_read_default(mov, pb, atom)) < 0) { >> av_log(s, AV_LOG_ERROR, "error reading header\n"); >> return err; >> @@ -8593,6 +8597,7 @@ static const AVOption mov_options[] = { >> { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM }, >> { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL, >> {.i64 = 0}, 0, 1, FLAGS }, >> + { "use_hoov", "Take hoov as moov atom, 0 for disable, -1 for auto, 1 means force", OFFSET(use_hoov), AV_OPT_TYPE_INT, {.i64 = 1}, -1, 1, FLAGS }, > > Kind of unusual that the default is 1 and not -1 (automatic). Is it intentional? Does it make sense to find hoov atom first and discard moov? Can't that cause problems for valid files? If it can, then maybe -1 should be the default, no? I prefer to set it to -1 and handle it automatically, but it may break those files Derek mentioned: http://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290359.html So force taking hoov as moov is for backward compatibility. It’s pain to workaround another workaround. > > Thanks, > Marton > _______________________________________________ > 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".
> 在 2021年12月25日，19:42，zhilizhao(赵志立) <quinkblack@foxmail.com> 写道： > > > >> On Dec 25, 2021, at 6:36 PM, Marton Balint <cus@passwd.hu> wrote: >> >> On Sat, 25 Dec 2021, Zhao Zhili wrote: >> >>> Fix #8883. >>> --- >>> doc/demuxers.texi | 6 ++++++ >>> libavformat/isom.h | 1 + >>> libavformat/mov.c | 9 +++++++-- >>> 3 files changed, 14 insertions(+), 2 deletions(-) >>> >>> diff --git a/doc/demuxers.texi b/doc/demuxers.texi >>> index cab8a7072c..276b536ac5 100644 >>> --- a/doc/demuxers.texi >>> +++ b/doc/demuxers.texi >>> @@ -713,6 +713,12 @@ specify. >>> >>> @item decryption_key >>> 16-byte key, in hex, to decrypt files encrypted using ISO Common Encryption (CENC/AES-128 CTR; ISO/IEC 23001-7). >>> + >>> +@item use_hoov >>> +Whether to use hoov atom as moov atom to support some broken files. Set it to 0 for disable, 1 for enable, >>> +-1 for automatic, which means take hoov as moov in the second pass if moov doesn't been found in the first >>> +pass. >>> + >>> @end table >>> >>> @subsection Audible AAX >>> diff --git a/libavformat/isom.h b/libavformat/isom.h >>> index ef8f19b18c..7c837811a6 100644 >>> --- a/libavformat/isom.h >>> +++ b/libavformat/isom.h >>> @@ -305,6 +305,7 @@ typedef struct MOVContext { >>> int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd >>> int have_read_mfra_size; >>> uint32_t mfra_size; >>> + int use_hoov; >>> } MOVContext; >>> >>> int ff_mp4_read_descr_len(AVIOContext *pb); >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index ea2f010aa0..82237f930b 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -7325,7 +7325,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>> a.size = avio_rb32(pb); >>> a.type = avio_rl32(pb); >>> if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) || >>> - a.type == MKTAG('h','o','o','v')) && >>> + a.type == MKTAG('h','o','o','v') && c->use_hoov == 1) && >>> a.size >= 8 && >>> c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) { >>> uint32_t type; >>> @@ -7951,8 +7951,12 @@ static int mov_read_header(AVFormatContext *s) >>> >>> /* check MOV header */ >>> do { >>> - if (mov->moov_retry) >>> + if (mov->moov_retry) { >>> avio_seek(pb, 0, SEEK_SET); >>> + /* reset use_hoov from auto mode to force mode in second pass */ >>> + if (mov->use_hoov == -1) >>> + mov->use_hoov = 1; >>> + } >>> if ((err = mov_read_default(mov, pb, atom)) < 0) { >>> av_log(s, AV_LOG_ERROR, "error reading header\n"); >>> return err; >>> @@ -8593,6 +8597,7 @@ static const AVOption mov_options[] = { >>> { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM }, >>> { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL, >>> {.i64 = 0}, 0, 1, FLAGS }, >>> + { "use_hoov", "Take hoov as moov atom, 0 for disable, -1 for auto, 1 means force", OFFSET(use_hoov), AV_OPT_TYPE_INT, {.i64 = 1}, -1, 1, FLAGS }, >> >> Kind of unusual that the default is 1 and not -1 (automatic). Is it intentional? Does it make sense to find hoov atom first and discard moov? Can't that cause problems for valid files? If it can, then maybe -1 should be the default, no? > > I prefer to set it to -1 and handle it automatically, but it may break those files Derek mentioned: > > http://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290359.html > > So force taking hoov as moov is for backward compatibility. It’s pain to workaround another workaround. https://ffmpeg.org/pipermail/ffmpeg-devel/2021-December/290245.html This can fix both problem, and will not modify any original main workflow, and the simplest way, I think you have change the main workflow to fix one or two bug, too complex. > >> >> Thanks, >> Marton >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". Thanks Steven
diff --git a/doc/demuxers.texi b/doc/demuxers.texi index cab8a7072c..276b536ac5 100644 --- a/doc/demuxers.texi +++ b/doc/demuxers.texi @@ -713,6 +713,12 @@ specify. @item decryption_key 16-byte key, in hex, to decrypt files encrypted using ISO Common Encryption (CENC/AES-128 CTR; ISO/IEC 23001-7). + +@item use_hoov +Whether to use hoov atom as moov atom to support some broken files. Set it to 0 for disable, 1 for enable, +-1 for automatic, which means take hoov as moov in the second pass if moov doesn't been found in the first +pass. + @end table @subsection Audible AAX diff --git a/libavformat/isom.h b/libavformat/isom.h index ef8f19b18c..7c837811a6 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -305,6 +305,7 @@ typedef struct MOVContext { int32_t movie_display_matrix[3][3]; ///< display matrix from mvhd int have_read_mfra_size; uint32_t mfra_size; + int use_hoov; } MOVContext; int ff_mp4_read_descr_len(AVIOContext *pb); diff --git a/libavformat/mov.c b/libavformat/mov.c index ea2f010aa0..82237f930b 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7325,7 +7325,7 @@ static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom) a.size = avio_rb32(pb); a.type = avio_rl32(pb); if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) || - a.type == MKTAG('h','o','o','v')) && + a.type == MKTAG('h','o','o','v') && c->use_hoov == 1) && a.size >= 8 && c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) { uint32_t type; @@ -7951,8 +7951,12 @@ static int mov_read_header(AVFormatContext *s) /* check MOV header */ do { - if (mov->moov_retry) + if (mov->moov_retry) { avio_seek(pb, 0, SEEK_SET); + /* reset use_hoov from auto mode to force mode in second pass */ + if (mov->use_hoov == -1) + mov->use_hoov = 1; + } if ((err = mov_read_default(mov, pb, atom)) < 0) { av_log(s, AV_LOG_ERROR, "error reading header\n"); return err; @@ -8593,6 +8597,7 @@ static const AVOption mov_options[] = { { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM }, { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS }, + { "use_hoov", "Take hoov as moov atom, 0 for disable, -1 for auto, 1 means force", OFFSET(use_hoov), AV_OPT_TYPE_INT, {.i64 = 1}, -1, 1, FLAGS }, { NULL }, };