On 2/22/2017 1:21 PM, James Almer wrote: > On 2/21/2017 7:35 PM, Vittorio Giovara wrote:
CCing this one as well. >> --- >> 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); > > private.size can be zero and private.data NULL. bytestream2_init() > will trigger an assert in those cases. > >> + >> + 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); > > Use SIZE_SPECIFIER instead of zu. Msvc doesn't like the latter. > Same with the mov implementation. > >> + 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; > > Nit: I still think the Equi case looks better the way i suggested in > my previous email. And what i wrote in said previous email that i also didn't CC: ---- I think this'll look better as case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR: projection = AV_SPHERICAL_EQUIRECTANGULAR; if (track->video.projection.private.size == 20) { [...] if (l || t || r || b) projection = AV_SPHERICAL_EQUIRECTANGULAR_TILE; } else if (track->video.projection.private.size != 0) { // return error } 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); > > Nit: Maybe > > if (layout) { > // return error > } > projection = AV_SPHERICAL_CUBEMAP; > 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; > > These four could also be zero initialized so you don't need to check > the projection. > >> + } >> + >> 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 >> > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel