diff mbox series

[FFmpeg-devel,2/2] lavf/mov: Support dref.url box for ISO 14496-12

Message ID 1596807454-17939-2-git-send-email-mypopydev@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] lavf/mov: Remove redundant code
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Jun Zhao Aug. 7, 2020, 1:37 p.m. UTC
From: Jun Zhao <barryjzhao@tencent.com>

Enable the dref.url box

Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
---
 libavformat/isom.h |  6 ++++++
 libavformat/mov.c  | 45 ++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 44 insertions(+), 7 deletions(-)

Comments

Zhao Zhili Aug. 7, 2020, 2:29 p.m. UTC | #1
> On Aug 7, 2020, at 9:37 PM, Jun Zhao <mypopydev@gmail.com> wrote:
> 
> From: Jun Zhao <barryjzhao@tencent.com>
> 
> Enable the dref.url box
> 
> Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
> ---
> libavformat/isom.h |  6 ++++++
> libavformat/mov.c  | 45 ++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 44 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/isom.h b/libavformat/isom.h
> index 41a9c64..01e2c33 100644
> --- a/libavformat/isom.h
> +++ b/libavformat/isom.h
> @@ -77,6 +77,12 @@ typedef struct MOVDref {
>     char volume[28];
>     char filename[64];
>     int16_t nlvl_to, nlvl_from;
> +
> +    /*
> +     * DataEntryUrlBox
> +     */
> +    char *location;         /* a location to find the resource with the given name */
> +    uint32_t location_len;
> } MOVDref;
> 
> typedef struct MOVAtom {
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 808ef7c..8fe2981 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -575,6 +575,7 @@ static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>     AVStream *st;
>     MOVStreamContext *sc;
>     int entries, i, j;
> +    int flags;
> 
>     if (c->fc->nb_streams < 1)
>         return 0;
> @@ -599,18 +600,19 @@ static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>         MOVDref *dref = &sc->drefs[i];
>         uint32_t size = avio_rb32(pb);
>         int64_t next = avio_tell(pb) + size - 4;
> +        int ret;
> 
>         if (size < 12)
>             return AVERROR_INVALIDDATA;
> 
>         dref->type = avio_rl32(pb);
> -        avio_rb32(pb); // version + flags
> +        avio_r8(pb); /* version */
> +        flags = avio_rb24(pb);
> 
>         if (dref->type == MKTAG('a','l','i','s') && size > 150) {
>             /* macintosh alias record */
>             uint16_t volume_len, len;
>             int16_t type;
> -            int ret;
> 
>             avio_skip(pb, 10);
> 
> @@ -696,6 +698,24 @@ static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>                 } else
>                     avio_skip(pb, len);
>             }
> +        } else if (dref->type == MKTAG('u','r','l',' ') && size >= 12) {
> +            if (flags == 1) {
> +                /* do noting when elementray stream in the same file */
> +                av_log(c->fc, AV_LOG_TRACE, "media data location in the same file\n");
> +            } else {
> +                uint32_t len = size - 12;
> +                dref->location = av_malloc(len + 1); /* Add null terminator */
> +                if (!dref->location)
> +                    return AVERROR(ENOMEM);
> +
> +                ret = ffio_read_size(pb, dref->location, len);
> +                if (ret < 0) {
> +                    av_freep(&dref->location);
> +                    return ret;
> +                }
> +                dref->location_len = len;
> +                dref->location[len] = 0;
> +            }
>         } else {
>             av_log(c->fc, AV_LOG_DEBUG, "Unknown dref type 0x%08"PRIx32" size %"PRIu32"\n",
>                    dref->type, size);
> @@ -4169,6 +4189,11 @@ static int mov_open_dref(MOVContext *c, AVIOContext **pb, const char *src, MOVDr
>             if (!c->fc->io_open(c->fc, pb, filename, AVIO_FLAG_READ, NULL))
>                 return 0;
>         }
> +    } else if (ref->location) {
> +        av_log(c->fc, AV_LOG_WARNING, "Try to open the media file in separate file from %s.\n",
> +            ref->location);
> +        if (!c->fc->io_open(c->fc, pb, ref->location, AVIO_FLAG_READ, NULL))
> +            return 0;
>     } else if (c->use_absolute_path) {
>         av_log(c->fc, AV_LOG_WARNING, "Using absolute path on user request, "
>                "this is a possible security issue\n");
> @@ -4243,23 +4268,28 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> 
>     mov_build_index(c, st);
> 
> -    if (sc->dref_id-1 < sc->drefs_count && sc->drefs[sc->dref_id-1].path) {
> +    if (sc->dref_id-1 < sc->drefs_count &&
> +        (sc->drefs[sc->dref_id-1].path || sc->drefs[sc->dref_id-1].location)) {
>         MOVDref *dref = &sc->drefs[sc->dref_id - 1];
>         if (c->enable_drefs) {
>             if (mov_open_dref(c, &sc->pb, c->fc->url, dref) < 0)
>                 av_log(c->fc, AV_LOG_ERROR,
>                        "stream %d, error opening alias: path='%s', dir='%s', "
> -                       "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d\n",
> +                       "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d, "
> +                       "url : len=%d, location='%s'\n",

%d should be %u

Print null as string is undefined, I'm not sure about the rules of ffmpeg, do
we need something like (dref->location ? dref->location : "")?

>                        st->index, dref->path, dref->dir, dref->filename,
> -                       dref->volume, dref->nlvl_from, dref->nlvl_to);
> +                       dref->volume, dref->nlvl_from, dref->nlvl_to,
> +                       dref->location_len, dref->location);
>         } else {
>             av_log(c->fc, AV_LOG_WARNING,
>                    "Skipped opening external track: "
>                    "stream %d, alias: path='%s', dir='%s', "
> -                   "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d."
> +                   "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d. "
> +                   "url : len=%d, location='%s' "
>                    "Set enable_drefs to allow this.\n",
>                    st->index, dref->path, dref->dir, dref->filename,
> -                   dref->volume, dref->nlvl_from, dref->nlvl_to);
> +                   dref->volume, dref->nlvl_from, dref->nlvl_to,
> +                   dref->location_len, dref->location);
>         }
>     } else {
>         sc->pb = c->fc->pb;
> @@ -7318,6 +7348,7 @@ static int mov_read_close(AVFormatContext *s)
>         for (j = 0; j < sc->drefs_count; j++) {
>             av_freep(&sc->drefs[j].path);
>             av_freep(&sc->drefs[j].dir);
> +            av_freep(&sc->drefs[j].location);
>         }
>         av_freep(&sc->drefs);
> 
> -- 
> 2.7.4
> 
> _______________________________________________
> 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".
Kieran Kunhya Aug. 7, 2020, 7:01 p.m. UTC | #2
>
> > @@ -4169,6 +4189,11 @@ static int mov_open_dref(MOVContext *c,
> AVIOContext **pb, const char *src, MOVDr
> >             if (!c->fc->io_open(c->fc, pb, filename, AVIO_FLAG_READ,
> NULL))
> >                 return 0;
> >         }
> > +    } else if (ref->location) {
> > +        av_log(c->fc, AV_LOG_WARNING, "Try to open the media file in
> separate file from %s.\n",
> > +            ref->location);
> > +        if (!c->fc->io_open(c->fc, pb, ref->location, AVIO_FLAG_READ,
> NULL))
> > +            return 0;
>

Is it safe to just open files like this?

Kieran
mypopy@gmail.com Aug. 8, 2020, 6:47 a.m. UTC | #3
On Fri, Aug 7, 2020 at 10:29 PM Zhao Zhili <quinkblack@foxmail.com> wrote:
>
>
>
> > On Aug 7, 2020, at 9:37 PM, Jun Zhao <mypopydev@gmail.com> wrote:
> >
> > From: Jun Zhao <barryjzhao@tencent.com>
> >
> > Enable the dref.url box
> >
> > Signed-off-by: Jun Zhao <barryjzhao@tencent.com>
> > ---
> > libavformat/isom.h |  6 ++++++
> > libavformat/mov.c  | 45 ++++++++++++++++++++++++++++++++++++++-------
> > 2 files changed, 44 insertions(+), 7 deletions(-)
> >
> > diff --git a/libavformat/isom.h b/libavformat/isom.h
> > index 41a9c64..01e2c33 100644
> > --- a/libavformat/isom.h
> > +++ b/libavformat/isom.h
> > @@ -77,6 +77,12 @@ typedef struct MOVDref {
> >     char volume[28];
> >     char filename[64];
> >     int16_t nlvl_to, nlvl_from;
> > +
> > +    /*
> > +     * DataEntryUrlBox
> > +     */
> > +    char *location;         /* a location to find the resource with the given name */
> > +    uint32_t location_len;
> > } MOVDref;
> >
> > typedef struct MOVAtom {
> > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > index 808ef7c..8fe2981 100644
> > --- a/libavformat/mov.c
> > +++ b/libavformat/mov.c
> > @@ -575,6 +575,7 @@ static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >     AVStream *st;
> >     MOVStreamContext *sc;
> >     int entries, i, j;
> > +    int flags;
> >
> >     if (c->fc->nb_streams < 1)
> >         return 0;
> > @@ -599,18 +600,19 @@ static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >         MOVDref *dref = &sc->drefs[i];
> >         uint32_t size = avio_rb32(pb);
> >         int64_t next = avio_tell(pb) + size - 4;
> > +        int ret;
> >
> >         if (size < 12)
> >             return AVERROR_INVALIDDATA;
> >
> >         dref->type = avio_rl32(pb);
> > -        avio_rb32(pb); // version + flags
> > +        avio_r8(pb); /* version */
> > +        flags = avio_rb24(pb);
> >
> >         if (dref->type == MKTAG('a','l','i','s') && size > 150) {
> >             /* macintosh alias record */
> >             uint16_t volume_len, len;
> >             int16_t type;
> > -            int ret;
> >
> >             avio_skip(pb, 10);
> >
> > @@ -696,6 +698,24 @@ static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >                 } else
> >                     avio_skip(pb, len);
> >             }
> > +        } else if (dref->type == MKTAG('u','r','l',' ') && size >= 12) {
> > +            if (flags == 1) {
> > +                /* do noting when elementray stream in the same file */
> > +                av_log(c->fc, AV_LOG_TRACE, "media data location in the same file\n");
> > +            } else {
> > +                uint32_t len = size - 12;
> > +                dref->location = av_malloc(len + 1); /* Add null terminator */
> > +                if (!dref->location)
> > +                    return AVERROR(ENOMEM);
> > +
> > +                ret = ffio_read_size(pb, dref->location, len);
> > +                if (ret < 0) {
> > +                    av_freep(&dref->location);
> > +                    return ret;
> > +                }
> > +                dref->location_len = len;
> > +                dref->location[len] = 0;
> > +            }
> >         } else {
> >             av_log(c->fc, AV_LOG_DEBUG, "Unknown dref type 0x%08"PRIx32" size %"PRIu32"\n",
> >                    dref->type, size);
> > @@ -4169,6 +4189,11 @@ static int mov_open_dref(MOVContext *c, AVIOContext **pb, const char *src, MOVDr
> >             if (!c->fc->io_open(c->fc, pb, filename, AVIO_FLAG_READ, NULL))
> >                 return 0;
> >         }
> > +    } else if (ref->location) {
> > +        av_log(c->fc, AV_LOG_WARNING, "Try to open the media file in separate file from %s.\n",
> > +            ref->location);
> > +        if (!c->fc->io_open(c->fc, pb, ref->location, AVIO_FLAG_READ, NULL))
> > +            return 0;
> >     } else if (c->use_absolute_path) {
> >         av_log(c->fc, AV_LOG_WARNING, "Using absolute path on user request, "
> >                "this is a possible security issue\n");
> > @@ -4243,23 +4268,28 @@ static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >
> >     mov_build_index(c, st);
> >
> > -    if (sc->dref_id-1 < sc->drefs_count && sc->drefs[sc->dref_id-1].path) {
> > +    if (sc->dref_id-1 < sc->drefs_count &&
> > +        (sc->drefs[sc->dref_id-1].path || sc->drefs[sc->dref_id-1].location)) {
> >         MOVDref *dref = &sc->drefs[sc->dref_id - 1];
> >         if (c->enable_drefs) {
> >             if (mov_open_dref(c, &sc->pb, c->fc->url, dref) < 0)
> >                 av_log(c->fc, AV_LOG_ERROR,
> >                        "stream %d, error opening alias: path='%s', dir='%s', "
> > -                       "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d\n",
> > +                       "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d, "
> > +                       "url : len=%d, location='%s'\n",
>
> %d should be %u
>
> Print null as string is undefined, I'm not sure about the rules of ffmpeg, do
> we need something like (dref->location ? dref->location : "")?
Good catch, will check NULL string
>
> >                        st->index, dref->path, dref->dir, dref->filename,
> > -                       dref->volume, dref->nlvl_from, dref->nlvl_to);
> > +                       dref->volume, dref->nlvl_from, dref->nlvl_to,
> > +                       dref->location_len, dref->location);
> >         } else {
> >             av_log(c->fc, AV_LOG_WARNING,
> >                    "Skipped opening external track: "
> >                    "stream %d, alias: path='%s', dir='%s', "
> > -                   "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d."
> > +                   "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d. "
> > +                   "url : len=%d, location='%s' "
> >                    "Set enable_drefs to allow this.\n",
> >                    st->index, dref->path, dref->dir, dref->filename,
> > -                   dref->volume, dref->nlvl_from, dref->nlvl_to);
> > +                   dref->volume, dref->nlvl_from, dref->nlvl_to,
> > +                   dref->location_len, dref->location);
> >         }
> >     } else {
> >         sc->pb = c->fc->pb;
> > @@ -7318,6 +7348,7 @@ static int mov_read_close(AVFormatContext *s)
> >         for (j = 0; j < sc->drefs_count; j++) {
> >             av_freep(&sc->drefs[j].path);
> >             av_freep(&sc->drefs[j].dir);
> > +            av_freep(&sc->drefs[j].location);
> >         }
> >         av_freep(&sc->drefs);
> >
> > --
> > 2.7.4
> >
> > _______________________________________________
> > 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".
diff mbox series

Patch

diff --git a/libavformat/isom.h b/libavformat/isom.h
index 41a9c64..01e2c33 100644
--- a/libavformat/isom.h
+++ b/libavformat/isom.h
@@ -77,6 +77,12 @@  typedef struct MOVDref {
     char volume[28];
     char filename[64];
     int16_t nlvl_to, nlvl_from;
+
+    /*
+     * DataEntryUrlBox
+     */
+    char *location;         /* a location to find the resource with the given name */
+    uint32_t location_len;
 } MOVDref;
 
 typedef struct MOVAtom {
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 808ef7c..8fe2981 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -575,6 +575,7 @@  static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     AVStream *st;
     MOVStreamContext *sc;
     int entries, i, j;
+    int flags;
 
     if (c->fc->nb_streams < 1)
         return 0;
@@ -599,18 +600,19 @@  static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         MOVDref *dref = &sc->drefs[i];
         uint32_t size = avio_rb32(pb);
         int64_t next = avio_tell(pb) + size - 4;
+        int ret;
 
         if (size < 12)
             return AVERROR_INVALIDDATA;
 
         dref->type = avio_rl32(pb);
-        avio_rb32(pb); // version + flags
+        avio_r8(pb); /* version */
+        flags = avio_rb24(pb);
 
         if (dref->type == MKTAG('a','l','i','s') && size > 150) {
             /* macintosh alias record */
             uint16_t volume_len, len;
             int16_t type;
-            int ret;
 
             avio_skip(pb, 10);
 
@@ -696,6 +698,24 @@  static int mov_read_dref(MOVContext *c, AVIOContext *pb, MOVAtom atom)
                 } else
                     avio_skip(pb, len);
             }
+        } else if (dref->type == MKTAG('u','r','l',' ') && size >= 12) {
+            if (flags == 1) {
+                /* do noting when elementray stream in the same file */
+                av_log(c->fc, AV_LOG_TRACE, "media data location in the same file\n");
+            } else {
+                uint32_t len = size - 12;
+                dref->location = av_malloc(len + 1); /* Add null terminator */
+                if (!dref->location)
+                    return AVERROR(ENOMEM);
+
+                ret = ffio_read_size(pb, dref->location, len);
+                if (ret < 0) {
+                    av_freep(&dref->location);
+                    return ret;
+                }
+                dref->location_len = len;
+                dref->location[len] = 0;
+            }
         } else {
             av_log(c->fc, AV_LOG_DEBUG, "Unknown dref type 0x%08"PRIx32" size %"PRIu32"\n",
                    dref->type, size);
@@ -4169,6 +4189,11 @@  static int mov_open_dref(MOVContext *c, AVIOContext **pb, const char *src, MOVDr
             if (!c->fc->io_open(c->fc, pb, filename, AVIO_FLAG_READ, NULL))
                 return 0;
         }
+    } else if (ref->location) {
+        av_log(c->fc, AV_LOG_WARNING, "Try to open the media file in separate file from %s.\n",
+            ref->location);
+        if (!c->fc->io_open(c->fc, pb, ref->location, AVIO_FLAG_READ, NULL))
+            return 0;
     } else if (c->use_absolute_path) {
         av_log(c->fc, AV_LOG_WARNING, "Using absolute path on user request, "
                "this is a possible security issue\n");
@@ -4243,23 +4268,28 @@  static int mov_read_trak(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 
     mov_build_index(c, st);
 
-    if (sc->dref_id-1 < sc->drefs_count && sc->drefs[sc->dref_id-1].path) {
+    if (sc->dref_id-1 < sc->drefs_count &&
+        (sc->drefs[sc->dref_id-1].path || sc->drefs[sc->dref_id-1].location)) {
         MOVDref *dref = &sc->drefs[sc->dref_id - 1];
         if (c->enable_drefs) {
             if (mov_open_dref(c, &sc->pb, c->fc->url, dref) < 0)
                 av_log(c->fc, AV_LOG_ERROR,
                        "stream %d, error opening alias: path='%s', dir='%s', "
-                       "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d\n",
+                       "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d, "
+                       "url : len=%d, location='%s'\n",
                        st->index, dref->path, dref->dir, dref->filename,
-                       dref->volume, dref->nlvl_from, dref->nlvl_to);
+                       dref->volume, dref->nlvl_from, dref->nlvl_to,
+                       dref->location_len, dref->location);
         } else {
             av_log(c->fc, AV_LOG_WARNING,
                    "Skipped opening external track: "
                    "stream %d, alias: path='%s', dir='%s', "
-                   "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d."
+                   "filename='%s', volume='%s', nlvl_from=%d, nlvl_to=%d. "
+                   "url : len=%d, location='%s' "
                    "Set enable_drefs to allow this.\n",
                    st->index, dref->path, dref->dir, dref->filename,
-                   dref->volume, dref->nlvl_from, dref->nlvl_to);
+                   dref->volume, dref->nlvl_from, dref->nlvl_to,
+                   dref->location_len, dref->location);
         }
     } else {
         sc->pb = c->fc->pb;
@@ -7318,6 +7348,7 @@  static int mov_read_close(AVFormatContext *s)
         for (j = 0; j < sc->drefs_count; j++) {
             av_freep(&sc->drefs[j].path);
             av_freep(&sc->drefs[j].dir);
+            av_freep(&sc->drefs[j].location);
         }
         av_freep(&sc->drefs);