[FFmpeg-devel] [PATCH] Add spherical_mapping command-line argument to ffmpeg.

Aaron Colwell acolwell at google.com
Mon Jun 5 22:46:13 EEST 2017


Attached a new patch that fixes a bug and the indentation in the previous
patch. This should be good now. Sorry for the spam.

Aaron

On Mon, Jun 5, 2017 at 12:32 PM Aaron Colwell <acolwell at google.com> wrote:

> Comments below..
>
> On Mon, Jun 5, 2017 at 12:02 PM Vittorio Giovara <
> vittorio.giovara at gmail.com> wrote:
>
>> Hey Aaron
>> > This allows adding AVSphericalMapping information to files
>> > that don't already have it.
>> > ---
>> >  ffmpeg.h              |   3 ++
>> >  ffmpeg_opt.c          |  29 ++++++++++++-
>> >  libavutil/spherical.c | 113
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >  libavutil/spherical.h |  14 +++++++
>> >  4 files changed, 158 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ffmpeg.h b/ffmpeg.h
>> > index a806445e0d..43a28d874f 100644
>> > --- a/ffmpeg.h
>> > +++ b/ffmpeg.h
>> > @@ -44,6 +44,7 @@
>> >  #include "libavutil/fifo.h"
>> >  #include "libavutil/pixfmt.h"
>> >  #include "libavutil/rational.h"
>> > +#include "libavutil/spherical.h"
>> >  #include "libavutil/threadmessage.h"
>> >
>> >  #include "libswresample/swresample.h"
>> > @@ -228,6 +229,8 @@ typedef struct OptionsContext {
>> >      int        nb_time_bases;
>> >      SpecifierOpt *enc_time_bases;
>> >      int        nb_enc_time_bases;
>> > +    SpecifierOpt *spherical_mappings;
>> > +    int        nb_spherical_mappings;
>> >  } OptionsContext;
>> >
>> >  typedef struct InputFilter {
>> > diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
>> > index c997ea8faf..666b3791b7 100644
>> > --- a/ffmpeg_opt.c
>> > +++ b/ffmpeg_opt.c
>> > @@ -1514,12 +1514,29 @@ static void
>> check_streamcopy_filters(OptionsContext *o, AVFormatContext *oc,
>> >      }
>> >  }
>> >
>> > +static int set_spherical_mapping(const char* opt, AVStream* st) {
>> > +  size_t spherical_mapping_size = 0;
>> > +  AVSphericalMapping *spherical_mapping = NULL;
>> > +
>> > +  int ret = av_spherical_parse_option(opt, &spherical_mapping,
>> > +                                      &spherical_mapping_size);
>> > +  if (ret >= 0) {
>> > +    ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL,
>> spherical_mapping, spherical_mapping_size);
>> > +
>> > +    if (ret < 0) {
>> > +      av_freep(&spherical_mapping);
>> > +    }
>> > +  }
>> > +
>> > +  return ret;
>> > +}
>> > +
>> >  static OutputStream *new_video_stream(OptionsContext *o,
>> AVFormatContext *oc, int source_index)
>> >  {
>> >      AVStream *st;
>> >      OutputStream *ost;
>> >      AVCodecContext *video_enc;
>> > -    char *frame_rate = NULL, *frame_aspect_ratio = NULL;
>> > +    char *frame_rate = NULL, *frame_aspect_ratio = NULL,
>> *spherical_mapping = NULL;
>> >
>> >      ost = new_output_stream(o, oc, AVMEDIA_TYPE_VIDEO, source_index);
>> >      st  = ost->st;
>> > @@ -1546,6 +1563,12 @@ static OutputStream
>> *new_video_stream(OptionsContext *o, AVFormatContext *oc, in
>> >
>> >      MATCH_PER_STREAM_OPT(filter_scripts, str, ost->filters_script, oc,
>> st);
>> >      MATCH_PER_STREAM_OPT(filters,        str, ost->filters,        oc,
>> st);
>> > +    MATCH_PER_STREAM_OPT(spherical_mappings, str, spherical_mapping,
>> oc, st);
>> > +
>> > +    if (spherical_mapping && set_spherical_mapping(spherical_mapping,
>> st) < 0) {
>> > +        av_log(NULL, AV_LOG_FATAL, "Invalid spherical_mapping: %s\n",
>> spherical_mapping);
>> > +        exit_program(1);
>> > +    }
>> >
>> >      if (!ost->stream_copy) {
>> >          const char *p = NULL;
>> > @@ -3569,6 +3592,10 @@ const OptionDef options[] = {
>> >          "automatically insert correct rotate filters" },
>> >      { "hwaccel_lax_profile_check", OPT_BOOL | OPT_EXPERT,
>>           { &hwaccel_lax_profile_check},
>> >          "attempt to decode anyway if HW accelerated decoder's
>> supported profiles do not exactly match the stream" },
>> > +    { "spherical_mapping", OPT_VIDEO | HAS_ARG  | OPT_STRING |
>> OPT_SPEC |
>> > +                           OPT_OUTPUT,
>>            { .off = OFFSET(spherical_mappings) },
>> > +        "set spherical mapping for video stream", "spherical_mapping"
>> },
>> > +
>> >
>> >      /* audio options */
>> >      { "aframes",        OPT_AUDIO | HAS_ARG  | OPT_PERFILE |
>> OPT_OUTPUT,           { .func_arg = opt_audio_frames },
>>
>> this part looks ok
>>
>> > diff --git a/libavutil/spherical.c b/libavutil/spherical.c
>> > index 4be55f36cf..508584d61f 100644
>> > --- a/libavutil/spherical.c
>> > +++ b/libavutil/spherical.c
>> > @@ -19,6 +19,7 @@
>> >   */
>> >
>> >  #include "mem.h"
>> > +#include "opt.h"
>> >  #include "spherical.h"
>> >
>> >  AVSphericalMapping *av_spherical_alloc(size_t *size)
>> > @@ -77,3 +78,115 @@ int av_spherical_from_name(const char *name)
>> >
>> >      return -1;
>> >  }
>> > +
>> > +static const char *spherical_mapping_context_to_name(void *ptr)
>> > +{
>> > +    return "spherical_mapping";
>> > +}
>> > +
>> > +typedef struct {
>> > +    const AVClass* spherical_class;
>> > +    int projection;
>> > +
>> > +    double yaw;
>> > +    double pitch;
>> > +    double roll;
>> > +
>> > +    int64_t bound_left;
>> > +    int64_t bound_top;
>> > +    int64_t bound_right;
>> > +    int64_t bound_bottom;
>> > +
>> > +    int64_t padding;
>> > +} SphericalMappingContext;
>> > +
>> > +#define OFFSET(x) offsetof(SphericalMappingContext, x)
>> > +#define DEFAULT 0
>> > +#define FLAGS AV_OPT_FLAG_VIDEO_PARAM
>> > +
>> > +static const AVOption spherical_mapping_options[] = {
>> > +    { "projection", "projection", OFFSET(projection), AV_OPT_TYPE_INT,
>> > +        { .i64 = -1 }, -1, AV_SPHERICAL_EQUIRECTANGULAR_TILE, FLAGS,
>> "projection" },
>> > +    { "equirectangular", "equirectangular projection",
>> OFFSET(projection), AV_OPT_TYPE_CONST,
>> > +        { .i64 = AV_SPHERICAL_EQUIRECTANGULAR }, INT_MIN, INT_MAX,
>> FLAGS, "projection" },
>> > +    { "cubemap", "cubemap projection", 0, AV_OPT_TYPE_CONST,
>> > +        { .i64 = AV_SPHERICAL_CUBEMAP }, INT_MIN, INT_MAX, FLAGS,
>> "projection" },
>> > +    { "equirectangular_tile", "tiled equirectangular projection",
>> OFFSET(projection), AV_OPT_TYPE_CONST,
>> > +        { .i64 = AV_SPHERICAL_EQUIRECTANGULAR_TILE }, INT_MIN,
>> INT_MAX, FLAGS, "projection" },
>> > +    { "yaw", "initial yaw orientation in degrees", OFFSET(yaw),
>> AV_OPT_TYPE_DOUBLE,
>> > +        { .dbl = 0.0 }, -180.0, 180.0, FLAGS },
>> > +    { "pitch", "initial pitch orientation in degrees", OFFSET(pitch),
>> AV_OPT_TYPE_DOUBLE,
>> > +        { .dbl = 0.0 }, -90.0, 90.0, FLAGS },
>> > +    { "roll", "initial roll orientation in degrees", OFFSET(roll),
>> AV_OPT_TYPE_DOUBLE,
>> > +        { .dbl = 0.0 }, -180.0, 180.0, FLAGS },
>> > +    { "bound_left", "tiled equirectangular left bound",
>> OFFSET(bound_left), AV_OPT_TYPE_INT64,
>> > +        { .i64 = 0 }, 0, UINT_MAX, FLAGS },
>> > +    { "bound_top", "tiled equirectangular top bound",
>> OFFSET(bound_top), AV_OPT_TYPE_INT64,
>> > +        { .i64 = 0 }, 0, UINT_MAX, FLAGS },
>> > +    { "bound_right", "tiled equirectangular right bound",
>> OFFSET(bound_right), AV_OPT_TYPE_INT64,
>> > +        { .i64 = 0 }, 0, UINT_MAX, FLAGS },
>> > +    { "bound_bottom", "tiled equirectangular bottom bound",
>> OFFSET(bound_bottom), AV_OPT_TYPE_INT64,
>> > +        { .i64 = 0 }, 0, UINT_MAX, FLAGS },
>> > +    { "padding", "cubemap padding in pixels", OFFSET(padding),
>> AV_OPT_TYPE_INT64,
>> > +        { .i64 = 0 }, 0, UINT_MAX, FLAGS },
>> > +    { NULL }
>> > +};
>> > +
>> > +static const AVClass av_spherical_mapping_class = {
>> > +    .class_name = "AVSphericalMapping",
>> > +    .item_name  = spherical_mapping_context_to_name,
>> > +    .option     = spherical_mapping_options,
>> > +    .version    = LIBAVUTIL_VERSION_INT,
>> > +};
>> > +
>> > +int av_spherical_parse_option(const char* opts, AVSphericalMapping
>> **spherical_mapping,
>> > +                              size_t *spherical_mapping_size) {
>> > +    SphericalMappingContext ctx = {
>> > +        .spherical_class = &av_spherical_mapping_class
>> > +    };
>> > +    int ret;
>> > +
>> > +    if (!opts)
>> > +        return AVERROR(EINVAL);
>> > +
>> > +    av_opt_set_defaults(&ctx);
>> > +    ret = av_set_options_string(&ctx, opts, "=", ",");
>> > +    if (ret < 0)
>> > +        return ret;
>> > +
>> > +    if (ctx.projection == -1) {
>> > +        av_log(NULL, AV_LOG_ERROR, "projection must be specified\n");
>> > +        return AVERROR(EINVAL);
>> > +    }
>> > +
>> > +    if (ctx.padding > 0 && ctx.projection != AV_SPHERICAL_CUBEMAP) {
>> > +        av_log(NULL, AV_LOG_ERROR, "padding only allowed for
>> AV_SPHERICAL_CUBEMAP projection.\n");
>> > +        return AVERROR(EINVAL);
>> > +    }
>> > +
>> > +    if ((ctx.bound_left > 0 || ctx.bound_top > 0 || ctx.bound_right >
>> 0 ||
>> > +         ctx.bound_bottom > 0) && ctx.projection !=
>> AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
>> > +        av_log(NULL, AV_LOG_ERROR, "bounds only allowed for
>> AV_SPHERICAL_EQUIRECTANGULAR_TILE projection.\n");
>> > +        return AVERROR(EINVAL);
>> > +    }
>> > +
>> > +    *spherical_mapping = av_spherical_alloc(spherical_mapping_size);
>> > +    if (!*spherical_mapping)
>> > +        return AVERROR(ENOMEM);
>> > +
>> > +    (*spherical_mapping)->projection = (enum
>> AVSphericalProjection)ctx.projection;
>> > +    (*spherical_mapping)->yaw = (int32_t)(ctx.yaw * (1 << 16));
>> > +    (*spherical_mapping)->pitch = (int32_t)(ctx.pitch * (1 << 16));
>> > +    (*spherical_mapping)->roll = (int32_t)(ctx.roll * (1 << 16));
>> > +
>> > +    if (ctx.projection == AV_SPHERICAL_CUBEMAP) {
>> > +        (*spherical_mapping)->padding = (uint32_t)ctx.padding;
>> > +    } else if (ctx.projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
>> > +        (*spherical_mapping)->bound_left = (uint32_t)ctx.bound_left;
>> > +        (*spherical_mapping)->bound_top = (uint32_t)ctx.bound_top;
>> > +        (*spherical_mapping)->bound_right = (uint32_t)ctx.bound_right;
>> > +        (*spherical_mapping)->bound_bottom =
>> (uint32_t)ctx.bound_bottom;
>> > +    }
>> > +
>> > +    return ret;
>> > +}
>> > diff --git a/libavutil/spherical.h b/libavutil/spherical.h
>> > index cef759cf27..643cff7f44 100644
>> > --- a/libavutil/spherical.h
>> > +++ b/libavutil/spherical.h
>> > @@ -190,6 +190,20 @@ typedef struct AVSphericalMapping {
>> >   */
>> >  AVSphericalMapping *av_spherical_alloc(size_t *size);
>> >
>> > +/**
>> > + * Parses a command-line option into an AVSphericalMapping object.
>> > + *
>> > + * @param opts String containing comma separated list of key=value
>> pairs.
>> > + * @param spherical_mapping Output parameter for the
>> AVSphericalMapping created by this function.
>> > + * @param spherical_mapping_size Output parameter for indicating the
>> size of the struct
>> > + *     referenced by *spherical_mapping.
>> > + *
>> > + * @return 0 on success and *spherical_mapping and
>> *spherical_mapping_size contain valid information.
>> > + *        <0 on failure and *spherical_mapping and
>> *spherical_mapping_size state is undefined.
>> > + */
>> > +int av_spherical_parse_option(const char* opt, AVSphericalMapping
>> **spherical_mapping,
>> > +                              size_t *spherical_mapping_size);
>> > +
>> >  /**
>> >   * Convert the @ref bounding fields from an AVSphericalVideo
>> >   * from 0.32 fixed point to pixels.
>>
>> This is the part that I have a problem with. The code itself is
>> totally fine, but I do believe that this API should not be exposed
>> publicly.
>> It's too custom specific and binds the ffmpeg option syntax to
>> libavutil while they are two separate layers that need to stay
>> separated.
>> It should be either something more generic so that other side data can
>> attach to it, or it should have its own proper syntax and AVOption
>> type.
>> For the task at hand though I believe it would be sufficient to move
>> option parsing in ffmpeg_opt.c
>>
>
> Fair enough. I was on the fence about this but didn't feel strongly either
> way. I've attached a new patch that only contains changes in ffmpeg.h and
> ffmpeg_opt.c.
>
> Thanks for the quick review.
>
> Aaron
>
>
>>
>> Cheers
>> --
>> Vittorio
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-spherical_mapping-command-line-argument-to-ffmpe.patch
Type: text/x-patch
Size: 8204 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170605/752ce720/attachment.bin>


More information about the ffmpeg-devel mailing list