[FFmpeg-devel] [PATCH] mov reference files search improvement

[Baptiste Coudurier]

Hi,

On Wed, Sep 02, 2009 at 04:05:01PM +0300, Maksym Veremeyenko wrote:
> Diego Biurrun ???????(??):
> >On Wed, Sep 02, 2009 at 03:54:01PM +0300, Maksym Veremeyenko wrote:
> >>Diego Biurrun ???????(??):
> >>>On Wed, Sep 02, 2009 at 02:56:38PM +0300, Maksym Veremeyenko wrote:
> >>>>Diego Biurrun ???????(??):
> >>>>>On Wed, Sep 02, 2009 at 01:26:11PM +0300, Maksym Veremeyenko wrote:
> >>>>>>Diego Biurrun ???????(??):
> >>>>>>>On Wed, Sep 02, 2009 at 09:55:32AM +0300, Maksym Veremeyenko wrote:
> [..]
> >> --- libavformat/mov.c	(revision 19698)
> >> +++ libavformat/mov.c	(working copy)
> >> @@ -273,41 +273,72 @@
> >>
> >> -                av_log(c->fc, AV_LOG_DEBUG, "type %d, len %d\n",
> type, len);
> >> +                av_log(c->fc, AV_LOG_DEBUG, "type %d, len %d\n",
> >> +                       type, len);
> >
> > cosmetics
> rolled back
> 
> >>>>--- libavformat/mov.c	(revision 19698)
> >>>>+++ libavformat/mov.c	(working copy)
> >>>>@@ -273,41 +273,72 @@
> >>>>+                } else if (type == 0) { // directory name
> >>>if (!type)
> >>I vote to leave it unchanged, because the *type* is compared
> >>against constant - it not just "zero".
> >
> >Keep it then.
> thanks
> 
> newer version attached
> 
> -- 
> ________________________________________
> Maksym Veremeyenko

> Index: libavformat/isom.h
> ===================================================================
> --- libavformat/isom.h	(revision 19698)
> +++ libavformat/isom.h	(working copy)
> @@ -55,7 +55,11 @@
>  
>  typedef struct {
>      uint32_t type;
> -    char *path;
> +    char* volume_name;
> +    char* file_name;
> +    int16_t nlvl_to, nlvl_from;
> +    char* directory_name;
> +    char* absolute_path;
>  } MOVDref;
>  
>  typedef struct {
> Index: libavformat/mov.c
> ===================================================================
> --- libavformat/mov.c	(revision 19698)
> +++ libavformat/mov.c	(working copy)
> @@ -273,20 +273,41 @@
>  
>          if (dref->type == MKTAG('a','l','i','s') && size > 150) {
>              /* macintosh alias record */
> -            uint16_t volume_len, len;
> -            char volume[28];
> +            uint16_t len;
>              int16_t type;
>  
>              url_fskip(pb, 10);
>  
> -            volume_len = get_byte(pb);
> -            volume_len = FFMIN(volume_len, 27);
> -            get_buffer(pb, volume, 27);
> -            volume[volume_len] = 0;
> -            av_log(c->fc, AV_LOG_DEBUG, "volume %s, len %d\n", volume, volume_len);
> +            /* read volume name */
> +            len = get_byte(pb);
> +            len = FFMIN(len, 27);
> +            dref->volume_name = av_mallocz(28);

use volume[28] in MOVDref structure.

> +            get_buffer(pb, dref->volume_name, 27);
> +            dref->volume_name[len] = 0;
> +            av_log(c->fc, AV_LOG_DEBUG, "volume name [%s], len %d\n",
> +                   dref->volume_name, len);
>  
> -            url_fskip(pb, 112);
> +            url_fskip(pb, 12);
>  
> +            /* read file name */
> +            len = get_byte(pb);
> +            len = FFMIN(len, 63);
> +            dref->file_name = av_mallocz(64);
> +            get_buffer(pb, dref->file_name, 63);

use filename[64] in MOVDref structure.

> [...]
>
>                  if (len&1)
>                      len += 1;
>                  if (type == 2) { // absolute path
> -                    av_free(dref->path);
> -                    dref->path = av_mallocz(len+1);
> -                    if (!dref->path)
> +                    av_free(dref->absolute_path);
> +                    dref->absolute_path = av_mallocz(len + 1);
> +                    if (!dref->absolute_path)
>                          return AVERROR(ENOMEM);

Renaming the field can be done in a separate patch.

> [...]
>
> +                } else if (type == 0) { // directory name
> +                    av_free(dref->directory_name);
> +                    dref->directory_name = av_mallocz(len + 1);
> +                    if (!dref->directory_name)
> +                        return AVERROR(ENOMEM);
> +                    get_buffer(pb, dref->directory_name, len);
> +                    dref->directory_name[len] = 0;

If you mallocz you don't need to set last byte to 0.

> [...]
>  
> +static char** mov_build_drefs_aliases(MOVDref* ref, char* filename)
> +{
>
> [...]
>
> +};
> +
> +static int mov_probe_drefs_aliases(MOVContext *c, ByteIOContext** pb,
> +                                   int idx, MOVDref* ref, char* filename)
> +{
> +    int i, n, ret;
> +    char** aliases;
> +    char* probed = av_mallocz(1);
> +
> +    /* build files list */
> +    aliases = mov_build_drefs_aliases(ref, filename);
> +
>
> [...]
>
> +};
> +
>  static int mov_read_trak(MOVContext *c, ByteIOContext *pb, MOVAtom atom)
>  {
>      AVStream *st;
> @@ -1553,10 +1697,9 @@
>  
>      mov_build_index(c, st);
>  
> -    if (sc->dref_id-1 < sc->drefs_count && sc->drefs[sc->dref_id-1].path) {
> -        if (url_fopen(&sc->pb, sc->drefs[sc->dref_id-1].path, URL_RDONLY) < 0)
> -            av_log(c->fc, AV_LOG_ERROR, "stream %d, error opening file %s: %s\n",
> -                   st->index, sc->drefs[sc->dref_id-1].path, strerror(errno));
> +    if (sc->dref_id-1 < sc->drefs_count && sc->drefs[sc->dref_id - 1].file_name) {
> +        mov_probe_drefs_aliases(c, &sc->pb, st->index,
> +                                &sc->drefs[sc->dref_id - 1], c->fc->filename);
>      } else
>          sc->pb = c->fc->pb;
>  

Probe and build can be simplified by avoiding building the list:
Just iterate through the possible until one open sucessfully.
Use a char path[1024] for one possible path.

I hope I'm clear enough :>

[...]

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
FFmpeg maintainer                                  http://www.ffmpeg.org