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

[James Almer]

On 3/7/2017 5:38 PM, Hendrik Leppkes wrote:
> 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

Or just make the fields uint32_t. They are supposed to be 32 bytes anyway.