[FFmpeg-devel] mov: Support fake moov boxes disguised as hoov

Submitted by Vittorio Giovara on Aug. 8, 2019, 8:28 p.m.

Details

Message ID 20190808202826.36504-1-vittorio.giovara@gmail.com
State New
Headers show

Commit Message

Vittorio Giovara Aug. 8, 2019, 8:28 p.m.
Some broken apps generate files that have a fake box named 'hoov'
instead of a proper 'moov' one. This is speculation but it seems like
this box contains data to be modified later (eg as file grows in size,
data gets re-written) and its name is supposed to be changed to 'moov'
once it can be used as a 'moov', but for some reason this step is skipped.

Since this is not the first time this happens ('moov' boxes can be found
in 'free' ones) extend the existing hacks to search for the moov in such
boxes and skip the moov_retry since it needs to be found right away.
---
 libavformat/mov.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

This code is ugly, tips for improving it are welcome, or a full
rejection is ok too. Unfortunately I cannot share the sample, but VLC
is able to play it.
Vittorio

Comments

Vittorio Giovara Aug. 12, 2019, 11:46 a.m.
On Thu, Aug 8, 2019 at 10:28 PM Vittorio Giovara <vittorio.giovara@gmail.com>
wrote:

> Some broken apps generate files that have a fake box named 'hoov'
> instead of a proper 'moov' one. This is speculation but it seems like
> this box contains data to be modified later (eg as file grows in size,
> data gets re-written) and its name is supposed to be changed to 'moov'
> once it can be used as a 'moov', but for some reason this step is skipped.
>
> Since this is not the first time this happens ('moov' boxes can be found
> in 'free' ones) extend the existing hacks to search for the moov in such
> boxes and skip the moov_retry since it needs to be found right away.
> ---
>  libavformat/mov.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> This code is ugly, tips for improving it are welcome, or a full
> rejection is ok too. Unfortunately I cannot share the sample, but VLC
> is able to play it.
> Vittorio
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 24de5429d1..0c2986b72e 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -6800,10 +6800,10 @@ static int mov_read_default(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>          if (atom.size >= 8) {
>              a.size = avio_rb32(pb);
>              a.type = avio_rl32(pb);
> -            if (a.type == MKTAG('f','r','e','e') &&
> +            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
> +                  a.type == MKTAG('h','o','o','v')) &&
>                  a.size >= 8 &&
> -                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT &&
> -                c->moov_retry) {
> +                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
>                  uint8_t buf[8];
>                  uint32_t *type = (uint32_t *)buf + 1;
>                  if (avio_read(pb, buf, 8) != 8)
> @@ -6811,7 +6811,7 @@ static int mov_read_default(MOVContext *c,
> AVIOContext *pb, MOVAtom atom)
>                  avio_seek(pb, -8, SEEK_CUR);
>                  if (*type == MKTAG('m','v','h','d') ||
>                      *type == MKTAG('c','m','o','v')) {
> -                    av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free
> atom.\n");
> +                    av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free
> or hoov atom.\n");
>                      a.type = MKTAG('m','o','o','v');
>                  }
>              }
> --
>

ping?
Michael Niedermayer Aug. 12, 2019, 7:37 p.m.
On Mon, Aug 12, 2019 at 01:46:55PM +0200, Vittorio Giovara wrote:
> On Thu, Aug 8, 2019 at 10:28 PM Vittorio Giovara <vittorio.giovara@gmail.com>
> wrote:
> 
> > Some broken apps generate files that have a fake box named 'hoov'
> > instead of a proper 'moov' one. This is speculation but it seems like
> > this box contains data to be modified later (eg as file grows in size,
> > data gets re-written) and its name is supposed to be changed to 'moov'
> > once it can be used as a 'moov', but for some reason this step is skipped.
> >
> > Since this is not the first time this happens ('moov' boxes can be found
> > in 'free' ones) extend the existing hacks to search for the moov in such
> > boxes and skip the moov_retry since it needs to be found right away.
> > ---
> >  libavformat/mov.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > This code is ugly, tips for improving it are welcome, or a full
> > rejection is ok too. Unfortunately I cannot share the sample, but VLC
> > is able to play it.
> > Vittorio
> >
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 24de5429d1..0c2986b72e 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -6800,10 +6800,10 @@ static int mov_read_default(MOVContext *c,
> > AVIOContext *pb, MOVAtom atom)
> >          if (atom.size >= 8) {
> >              a.size = avio_rb32(pb);
> >              a.type = avio_rl32(pb);
> > -            if (a.type == MKTAG('f','r','e','e') &&
> > +            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
> > +                  a.type == MKTAG('h','o','o','v')) &&
> >                  a.size >= 8 &&
> > -                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT &&
> > -                c->moov_retry) {
> > +                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
> >                  uint8_t buf[8];
> >                  uint32_t *type = (uint32_t *)buf + 1;
> >                  if (avio_read(pb, buf, 8) != 8)
> > @@ -6811,7 +6811,7 @@ static int mov_read_default(MOVContext *c,
> > AVIOContext *pb, MOVAtom atom)
> >                  avio_seek(pb, -8, SEEK_CUR);
> >                  if (*type == MKTAG('m','v','h','d') ||
> >                      *type == MKTAG('c','m','o','v')) {
> > -                    av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free
> > atom.\n");
> > +                    av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free
> > or hoov atom.\n");
> >                      a.type = MKTAG('m','o','o','v');
> >                  }
> >              }
> > --
> >
> 
> ping?

probably ok, but same oppinion as you, cleaner solution preferred if anyone
has a cleaner one

Thanks

[...]

Patch hide | download patch | download mbox

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 24de5429d1..0c2986b72e 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -6800,10 +6800,10 @@  static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         if (atom.size >= 8) {
             a.size = avio_rb32(pb);
             a.type = avio_rl32(pb);
-            if (a.type == MKTAG('f','r','e','e') &&
+            if (((a.type == MKTAG('f','r','e','e') && c->moov_retry) ||
+                  a.type == MKTAG('h','o','o','v')) &&
                 a.size >= 8 &&
-                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT &&
-                c->moov_retry) {
+                c->fc->strict_std_compliance < FF_COMPLIANCE_STRICT) {
                 uint8_t buf[8];
                 uint32_t *type = (uint32_t *)buf + 1;
                 if (avio_read(pb, buf, 8) != 8)
@@ -6811,7 +6811,7 @@  static int mov_read_default(MOVContext *c, AVIOContext *pb, MOVAtom atom)
                 avio_seek(pb, -8, SEEK_CUR);
                 if (*type == MKTAG('m','v','h','d') ||
                     *type == MKTAG('c','m','o','v')) {
-                    av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free atom.\n");
+                    av_log(c->fc, AV_LOG_ERROR, "Detected moov in a free or hoov atom.\n");
                     a.type = MKTAG('m','o','o','v');
                 }
             }