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

[wm4]

On Tue, 7 Mar 2017 21:38:52 +0100
Hendrik Leppkes <h.leppkes at gmail.com> 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.

It broke much more here than just the size field.