[FFmpeg-devel] [PATCH] Add spherical_mapping command-line argument to ffmpeg.
Aaron Colwell
acolwell at google.com
Mon Jun 5 22:32:42 EEST 2017
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: 8125 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170605/39b8f14a/attachment.bin>
More information about the ffmpeg-devel
mailing list