[FFmpeg-devel] [PATCH] avformat/matroskaenc: add support for Spherical Video elements

[James Almer]

On 3/8/2017 5:08 PM, Vittorio Giovara wrote:
> On Wed, Mar 8, 2017 at 2:46 PM, James Almer <jamrial at gmail.com> wrote:
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavformat/matroskaenc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 1605f0cafe..0ee927d63e 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -918,6 +918,72 @@ static int mkv_write_video_color(AVIOContext *pb, AVCodecParameters *par, AVStre
>>      return 0;
>>  }
>>
>> +static int mkv_write_video_projection(AVIOContext *pb, AVStream *st) {
>> +    int side_data_size = 0;
>> +    const AVSphericalMapping *spherical =
>> +        (const AVSphericalMapping*) av_stream_get_side_data(st, AV_PKT_DATA_SPHERICAL,
>> +                                                            &side_data_size);
>> +
>> +    if (side_data_size == sizeof(AVSphericalMapping)) {
> 
> I don't think you have to check for this (and checking sizeof() of
> this struct outside lavu breaks ABI), it's enough to check that size
> != 0

Ok.

You'll probably also have to do the same on libavformat/dump.c, for
that matter.

> 
>> +        AVIOContext *dyn_cp;
>> +        uint8_t *projection_ptr;
>> +        int ret, projection_size;
>> +
>> +        ret = avio_open_dyn_buf(&dyn_cp);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        switch (spherical->projection) {
>> +        case AV_SPHERICAL_EQUIRECTANGULAR_TILE:
>> +        {
>> +            uint8_t private[20];
>> +            put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>> +                          MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR);
>> +            AV_WB32(private +  0, 0); // version + flags
>> +            AV_WB32(private +  4, spherical->bound_top);
>> +            AV_WB32(private +  8, spherical->bound_bottom);
>> +            AV_WB32(private + 12, spherical->bound_left);
>> +            AV_WB32(private + 16, spherical->bound_right);
> 
> I'd feel better if you could use bytestream functions

Seems like pointless complexity to me, but sure.

> 
>> +            put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, private, sizeof(private));
>> +            break;
>> +        }
>> +        case AV_SPHERICAL_EQUIRECTANGULAR:
>> +            put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>> +                          MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR);
>> +            break;
>> +        case AV_SPHERICAL_CUBEMAP:
>> +        {
>> +            uint8_t private[12];
>> +            put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>> +                          MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP);
>> +            AV_WB32(private + 0, 0); // version + flags
>> +            AV_WB32(private + 4, 0); // layout
>> +            AV_WB32(private + 8, spherical->padding);
>> +            put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE, private, sizeof(private));
>> +            break;
>> +        }
>> +        default:
>> +            // TODO: Mesh projection once implemented in AVSphericalMapping
> 
> a little av_log message to warn about this?

This is a muxer, spherical->projection will be one of the supported enums.
This default case is just to prevent potential compiler warnings and can
be removed.

What needs a log message is the demuxer.

> 
>> +            goto end;
>> +        }
>> +
>> +        put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW,   (double)spherical->yaw   / (1 << 16));
>> +        put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH, (double)spherical->pitch / (1 << 16));
>> +        put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL,  (double)spherical->roll  / (1 << 16));
> 
> does the matroska spec require plain integers?
> spherical->{yaw,pitch,roll} are in 16.16 so there should be no need of
> extra conversions

Matroska stores these three as double precision fp values.

> 
>> +
>> +end:
>> +        projection_size = avio_close_dyn_buf(dyn_cp, &projection_ptr);
>> +        if (projection_size) {
>> +            ebml_master projection = start_ebml_master(pb, MATROSKA_ID_VIDEOPROJECTION, projection_size);
>> +            avio_write(pb, projection_ptr, projection_size);
>> +            end_ebml_master(pb, projection);
>> +        }
>> +        av_freep(&projection_ptr);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  static void mkv_write_field_order(AVIOContext *pb, int mode,
>>                                    enum AVFieldOrder field_order)
>>  {
>> @@ -1268,6 +1334,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);
>> +        if (ret < 0)
>> +            return ret;
>>          end_ebml_master(pb, subinfo);
>>          break;
>>
>> --
>> 2.12.0
>>
> 
> 
>