[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