[FFmpeg-devel] [PATCH] matroskaenc: Add support for writing video projection.

[James Almer]

On 1/27/2017 5:12 PM, Aaron Colwell wrote:
> Adding support for writing spherical metadata in Matroska.
> 
> 
> 0001-matroskaenc-Add-support-for-writing-video-projection.patch
> 
> 
> From 5a9cf56bf3114186412bb5572b153f886edb6ddb Mon Sep 17 00:00:00 2001
> From: Aaron Colwell <acolwell at google.com>
> Date: Fri, 27 Jan 2017 12:07:25 -0800
> Subject: [PATCH] matroskaenc: Add support for writing video projection
>  element.
> 
> Adding support for writing spherical metadata in Matroska.
> ---
>  libavformat/matroskaenc.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f731b678b9..1b186db223 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -1038,6 +1038,67 @@ static int mkv_write_stereo_mode(AVFormatContext *s, AVIOContext *pb,
>      return ret;
>  }
>  
> +static int mkv_write_video_projection(AVIOContext *pb, AVStream *st)
> +{
> +    AVSphericalMapping *spherical_mapping = (AVSphericalMapping*)av_stream_get_side_data(st, AV_PKT_DATA_SPHERICAL, NULL);
> +    ebml_master projection;
> +    int projection_type = 0;
> +
> +    AVIOContext *dyn_cp;
> +    uint8_t *projection_private_ptr = NULL;
> +    int ret, projection_private_size;
> +
> +    if (!spherical_mapping)
> +        return 0;
> +
> +    if (spherical_mapping->projection != AV_SPHERICAL_EQUIRECTANGULAR &&
> +        spherical_mapping->projection != AV_SPHERICAL_CUBEMAP) {
> +        av_log(pb, AV_LOG_WARNING, "Unsupported projection %d. projection not written.\n", spherical_mapping->projection);
> +        return 0;
> +    }
> +
> +    ret = avio_open_dyn_buf(&dyn_cp);
> +    if (ret < 0)
> +        return ret;
> +
> +    switch (spherical_mapping->projection) {
> +    case AV_SPHERICAL_EQUIRECTANGULAR:
> +        projection_type = 1;
> +
> +        /* Create ProjectionPrivate contents */
> +        avio_wb32(dyn_cp, 0); /* version = 0 & flags = 0 */
> +        avio_wb32(dyn_cp, 0); /* projection_bounds_top */
> +        avio_wb32(dyn_cp, 0); /* projection_bounds_bottom */
> +        avio_wb32(dyn_cp, 0); /* projection_bounds_left */
> +        avio_wb32(dyn_cp, 0); /* projection_bounds_right */

You could instead use a local 20 byte array, fill it using AV_WB32() and
then write it with put_ebml_binary().

Also, the latest change to the spec draft mentions ProjectionPrivate is
optional for Equirectangular projections if the contents are going to be
all zero.

> +        break;
> +    case AV_SPHERICAL_CUBEMAP:
> +        projection_type = 2;
> +
> +        /* Create ProjectionPrivate contents */
> +        avio_wb32(dyn_cp, 0); /* version = 0 & flags = 0 */
> +        avio_wb32(dyn_cp, 0); /* layout */
> +        avio_wb32(dyn_cp, 0); /* padding */
> +        break;
> +    }

Same, 12 byte array.

What if the user is trying to remux a matroska file that has a
ProjectionPrivate element or an mp4 with an equi box filled with non zero
values, for that matter?
I know you're the one behind the spec in question, but wouldn't it be a
better idea to wait until AVSphericalMapping gets a way to propagate this
kind of information before adding support for muxing video projection
elements? Or maybe try to implement it right now...

This also applies to the mp4 patch.

> +
> +    projection_private_size = avio_close_dyn_buf(dyn_cp, &projection_private_ptr);

The dynbuf should ideally contain the whole Projection master, so you can
then pass its size to start_ebml_master() instead of zero.
See mkv_write_video_color().

> +
> +    projection = start_ebml_master(pb, MATROSKA_ID_VIDEOPROJECTION, 0);
> +    put_ebml_uint(pb, MATROSKA_ID_VIDEOPROJECTIONTYPE, projection_type);
> +    if (projection_private_size)
> +        put_ebml_binary(pb, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, projection_private_ptr, projection_private_size);
> +
> +    put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, (float)(spherical_mapping->yaw) / (1 << 16));
> +    put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, (float)(spherical_mapping->pitch) / (1 << 16));
> +    put_ebml_float(pb, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL, (float)(spherical_mapping->roll) / (1 << 16));
> +    end_ebml_master(pb, projection);
> +
> +    av_free(projection_private_ptr);
> +
> +    return 0;
> +}
> +
>  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>                             int i, AVIOContext *pb, int default_stream_exists)
>  {
> @@ -1251,6 +1312,9 @@ static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
>          ret = mkv_write_video_color(pb, par, st);
>          if (ret < 0)
>              return ret;
> +        ret = mkv_write_video_projection(pb, st);

This needs to be inside a check for strict experimental compliance
nonetheless.

> +        if (ret < 0)
> +            return ret;
>          end_ebml_master(pb, subinfo);
>          break;
>  
> -- 2.11.0.483.g087da7b7c-goog
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>