[FFmpeg-devel] [PATCHv3 3/3] mkv: Export bounds and padding from spherical metadata
James Almer
jamrial at gmail.com
Tue Feb 28 18:00:04 EET 2017
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
>>
>
More information about the ffmpeg-devel
mailing list