[FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata

[Hendrik Leppkes]

On Tue, Mar 7, 2017 at 7:26 PM, wm4 <nfxjfg at googlemail.com> wrote:
> On Tue, 21 Feb 2017 17:35:53 -0500
> Vittorio Giovara <vittorio.giovara at gmail.com> wrote:
>
>> ---
>>  libavformat/matroskadec.c              | 64 ++++++++++++++++++++++++++++++++--
>>  tests/ref/fate/matroska-spherical-mono |  6 +++-
>>  2 files changed, 66 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 7223e94..0a02237 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1913,16 +1913,65 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
>>      AVSphericalMapping *spherical;
>>      enum AVSphericalProjection projection;
>>      size_t spherical_size;
>> +    size_t l, t, r, b;
>> +    size_t padding = 0;
>>      int ret;
>> +    GetByteContext gb;
>> +
>> +    bytestream2_init(&gb, track->video.projection.private.data,
>> +                     track->video.projection.private.size);
>> +
>> +    if (bytestream2_get_byte(&gb) != 0) {
>> +        av_log(NULL, AV_LOG_WARNING, "Unknown spherical metadata\n");
>> +        return 0;
>> +    }
>> +
>> +    bytestream2_skip(&gb, 3); // flags
>>
>>      switch (track->video.projection.type) {
>>      case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
>> -        projection = AV_SPHERICAL_EQUIRECTANGULAR;
>> +        if (track->video.projection.private.size == 0)
>> +            projection = AV_SPHERICAL_EQUIRECTANGULAR;
>> +        else if (track->video.projection.private.size == 20) {
>> +            t = bytestream2_get_be32(&gb);
>> +            b = bytestream2_get_be32(&gb);
>> +            l = bytestream2_get_be32(&gb);
>> +            r = bytestream2_get_be32(&gb);
>> +
>> +            if (b >= UINT_MAX - t || r >= UINT_MAX - l) {
>> +                av_log(NULL, AV_LOG_ERROR,
>> +                       "Invalid bounding rectangle coordinates "
>> +                       "%zu,%zu,%zu,%zu\n", l, t, r, b);
>> +                return AVERROR_INVALIDDATA;
>> +            }
>> +
>> +            if (l || t || r || b)
>> +                projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE;
>> +            else
>> +                projection = AV_SPHERICAL_EQUIRECTANGULAR;
>> +        } else {
>> +            av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>> +            return AVERROR_INVALIDDATA;
>> +        }
>>          break;
>>      case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
>> -        if (track->video.projection.private.size < 4)
>> +        if (track->video.projection.private.size < 4) {
>> +            av_log(NULL, AV_LOG_ERROR, "Missing projection private properties\n");
>> +            return AVERROR_INVALIDDATA;
>> +        } else if (track->video.projection.private.size == 12) {
>> +            uint32_t layout = bytestream2_get_be32(&gb);
>> +            if (layout == 0) {
>> +                projection = AV_SPHERICAL_CUBEMAP;
>> +            } else {
>> +                av_log(NULL, AV_LOG_WARNING,
>> +                       "Unknown spherical cubemap layout %"PRIu32"\n", layout);
>> +                return 0;
>> +            }
>> +            padding = bytestream2_get_be32(&gb);
>> +        } else {
>> +            av_log(NULL, AV_LOG_ERROR, "Unknown spherical metadata\n");
>>              return AVERROR_INVALIDDATA;
>> -        projection = AV_SPHERICAL_CUBEMAP;
>> +        }
>>          break;
>>      default:
>>          return 0;
>> @@ -1937,6 +1986,15 @@ static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track)
>>      spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
>>      spherical->roll  = (int32_t)(track->video.projection.roll  * (1 << 16));
>>
>> +    spherical->padding = padding;
>> +
>> +    if (spherical->projection == AV_SPHERICAL_EQUIRECTANGULAR_TILE) {
>> +        spherical->bound_left   = l;
>> +        spherical->bound_top    = t;
>> +        spherical->bound_right  = r;
>> +        spherical->bound_bottom = b;
>> +    }
>> +
>>      ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t *)spherical,
>>                                    spherical_size);
>>      if (ret < 0) {
>> diff --git a/tests/ref/fate/matroska-spherical-mono b/tests/ref/fate/matroska-spherical-mono
>> index 8048aff..a70d879 100644
>> --- a/tests/ref/fate/matroska-spherical-mono
>> +++ b/tests/ref/fate/matroska-spherical-mono
>> @@ -8,7 +8,11 @@ inverted=0
>>  [SIDE_DATA]
>>  side_data_type=Spherical Mapping
>>  side_data_size=56
>> -projection=equirectangular
>> +projection=tiled equirectangular
>> +bound_left=148
>> +bound_top=73
>> +bound_right=147
>> +bound_bottom=72
>>  yaw=45
>>  pitch=30
>>  roll=15
>
> Is it just me, or did this break FATE?

It broke on some systems because it  prints the side_data_size, which
varies on some compilers/systems since the struct uses size_t
We could probably just remove side_data_size, its not very informative
and as seen here can even be compiler/platform dependent.

- Hendrik