[FFmpeg-devel] [RFC] Browser remote content API

[Nicolas George]

L'octidi 28 pluviôse, an CCXXII, Lukasz Marek a écrit :
> I've attached implementation of my proposal API.
> 
> Second patch is just for testing purposes, but probably may be
> helpful for the student. Example may be also merged at some point
> when first protocol supports it.
> 
> -- 
> Best Regards,
> Lukasz Marek
> 
> When you look long into an abyss, the abyss looks into you. -
> Friedrich Nietzsche

> >From be932968c1ba2b9f4e34dd096f31df2ddba97249 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Sun, 16 Feb 2014 00:44:09 +0100
> Subject: [PATCH 1/2] lavf/avio: add directory list API
> 
> TODO: minor bump and APIchnage update.
> 
> This is preparation for GSoC project.
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavformat/avio.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/avio.h | 49 +++++++++++++++++++++++++++++++++
>  libavformat/url.h  |  1 +
>  3 files changed, 130 insertions(+)

I believe the implementation could go in a separate file.

> 
> diff --git a/libavformat/avio.c b/libavformat/avio.c
> index 225d982..1851654 100644
> --- a/libavformat/avio.c
> +++ b/libavformat/avio.c
> @@ -23,6 +23,7 @@
>  #include "libavutil/dict.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/time.h"
> +#include "libavutil/avassert.h"
>  #include "os_support.h"
>  #include "avformat.h"
>  #if CONFIG_NETWORK
> @@ -395,6 +396,85 @@ int avio_check(const char *url, int flags)
>      return ret;
>  }
>  
> +void avio_free_dir_list(AVIODirEntryList **entries)
> +{
> +    AVIODirEntryList *list;
> +    int i;
> +
> +    if (!entries || !(*entries))
> +        return;
> +    list = *entries;
> +    for (i = 0; i < list->nb_entries; i++) {
> +        if (list->enties[i]) {
> +            av_free(list->enties[i]->name);
> +            av_free(list->enties[i]);
> +        }
> +    }
> +    av_free(list->enties);
> +    av_freep(entries);
> +}
> +
> +static int cmp_dir_entires(AVIODirEntry *e1, AVIODirEntry *e2, int flags)
> +{
> +    int ret = flags & AVIO_SORT_DESCENDING;
> +    av_assert0(e1 && e2);
> +    if ((flags & AVIO_SORT_DIRS_FIRST) && e1->type != e2->type)
> +        return e1->type != AVIO_ENTRY_DIR;
> +    if (flags & AVIO_SORT_BY_SIZE) {
> +        if (e1->size != e2->size)
> +            return e1->size > e2->size ? ret : !ret;
> +    } else if (flags & AVIO_SORT_BY_CREATION_DATE) {
> +        if (e1->creation_timestamp != e2->creation_timestamp)
> +            return e1->creation_timestamp > e2->creation_timestamp ? ret : !ret;
> +    } else if (flags & AVIO_SORT_BY_MODIFICATION_DATE) {
> +        if (e1->modification_timestamp != e2->modification_timestamp)
> +            return e1->modification_timestamp > e2->modification_timestamp ? ret : !ret;
> +    }
> +    return strcmp(e1->name, e2->name) < 0 ? ret : !ret;
> +}
> +
> +int avio_list_dir(const char *url, AVIODirEntryList **entries, int sort_flags,
> +                  AVDictionary **options)
> +{
> +    int ret, i1, i2;
> +    URLContext *h;
> +    AVIODirEntryList *list;
> +
> +    av_assert0(entries);
> +    *entries = NULL;
> +    if ((ret = ffurl_alloc(&h, url, 0, NULL)))
> +        return ret;
> +    if (!h->prot->url_list_dir) {
> +        ret = AVERROR(ENOSYS);
> +        goto fail;
> +    }

> +    if (options && h->prot->priv_data_class &&
> +        (ret = av_opt_set_dict(h->priv_data, options)) < 0)
> +        goto fail;
> +

Why not set directly options on h?

> +    list = av_mallocz(sizeof(**entries));

sizeof(*list)?

> +    if (!list) {
> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }

> +    *entries  = list;

IMHO, should go when success is certain.

> +    if ((ret = h->prot->url_list_dir(h, url, list)) < 0)
> +        goto fail;
> +

> +    for (i1 = 0; i1 < list->nb_entries - 1; i1++)
> +        for (i2 = i1 + 1; i2 < list->nb_entries; i2++) {
> +            if (cmp_dir_entires(list->enties[i1], list->enties[i2], sort_flags))
> +                FFSWAP(AVIODirEntry *, list->enties[i1], list->enties[i2]);
> +        }

I can see why you would do it (lack of extra argument to the sort function
of qsort), but I believe a custom local implementation of sort is not a good
idea. You can probably use Michael's AV_QSORT macro: since it is a macro,
calling the cmp function with an extra argument should be possible.

> +
> +    ffurl_close(h);
> +    return 0;
> +  fail:
> +    ffurl_close(h);
> +    avio_free_dir_list(entries);
> +    return ret;
> +}
> +
>  int64_t ffurl_size(URLContext *h)
>  {
>      int64_t pos, size;
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index 4f4ac3c..ff8ec22 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h
> @@ -53,6 +53,32 @@ typedef struct AVIOInterruptCB {
>      void *opaque;
>  } AVIOInterruptCB;
>  
> +enum AVIODirEntryType {
> +    AVIO_ENTRY_UNKNOWN,
> +    AVIO_ENTRY_DIR,
> +    AVIO_ENTRY_FILE
> +};
> +
> +/**
> + * Describes single entry of the directory.
> + */
> +typedef struct AVIODirEntry {
> +    char *name;                      /**< filename */

> +    enum AVIODirEntryType type;      /**< one of AVIO_ENTRY_* value */

Very minor nit: putting the fields with smaller type after the bigger ones
is better for packing.

> +    int64_t size;                    /**< file size */

> +    int64_t creation_timestamp;      /**< creation timestamp */

ANy reason to include this one and not atime and ctime which are much more
common?

> +    int64_t modification_timestamp;  /**< modification timestamp */

Also: I suppose this is in AV_TIME_BASE (microseconds) since the Unix epoch.
Maybe the doxy should state it. As is, the doxy for the fields could be
completely removed.

> +} AVIODirEntry;
> +
> +/**
> + * List of directory entries.
> + * @see avio_list_dir()
> + */
> +typedef struct AVIODirEntryList {
> +    AVIODirEntry **enties;
> +    int nb_entries;
> +} AVIODirEntryList;
> +
>  /**
>   * Bytestream IO Context.
>   * New fields can be added to the end with minor version bumps.
> @@ -164,6 +190,29 @@ typedef struct AVIOContext {
>   */
>  int avio_check(const char *url, int flags);
>  
> +#define AVIO_SORT_DESCENDING           0x001 /**< use descending mode instead of ascending */
> +#define AVIO_SORT_DIRS_FIRST           0x002 /**< move all dir names (sorted) in front of the list */

> +#define AVIO_SORT_BY_SIZE              0x004 /**< sort by size before sorting by name */
> +#define AVIO_SORT_BY_CREATION_DATE     0x008 /**< sort by creation date before sorting by name */
> +#define AVIO_SORT_BY_MODIFICATION_DATE 0x010 /**< sort by modification date before sorting by name */

Inconsistent naming: time / date.

> +
> +/**
> + * Allow to read content of the directory.
> + *
> + * @param url          directory to be listed.
> + * @param[out] entries sorted directory entries.
> + * @param sort_flags   combination of AVIO_SORT_* flags.
> + * @param options      protocol options.
> + * @return 0 on success or <0 on error.
> + */
> +int avio_list_dir(const char *url, AVIODirEntryList **entries, int sort_flags,
> +                  AVDictionary **options);
> +
> +/**
> + * Free directory entries list.
> + */
> +void avio_free_dir_list(AVIODirEntryList **entries);
> +
>  /**
>   * Allocate and initialize an AVIOContext for buffered I/O. It must be later
>   * freed with av_free().
> diff --git a/libavformat/url.h b/libavformat/url.h
> index 712ea0f..f64f34d 100644
> --- a/libavformat/url.h
> +++ b/libavformat/url.h
> @@ -89,6 +89,7 @@ typedef struct URLProtocol {
>      const AVClass *priv_data_class;
>      int flags;
>      int (*url_check)(URLContext *h, int mask);
> +    int (*url_list_dir)(URLContext *h, const char *dir, AVIODirEntryList *entries);
>  } URLProtocol;
>  
>  /**

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140216/a6adc223/attachment.asc>