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

[Vittorio Giovara]

On Tue, Feb 28, 2017 at 1:15 PM, James Almer <jamrial at gmail.com> wrote:
> On 2/28/2017 2:46 PM, Vittorio Giovara wrote:
>> On Tue, Feb 28, 2017 at 11:00 AM, James Almer <jamrial at gmail.com> wrote:
>>> 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.
>>
>> :( asserts like this are really dampening, it should be on read not on
>> creation (if at all)
>
> True, guess it should return an error value instead.

For now what should we do? Just check that private.size and
private.data are non-0 and return with an error otherwise?

>>>>> +    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.
>>
>> Umh? Isn't %zu supported on any msvc version that matters (2015 onwards)?
>> https://connect.microsoft.com/VisualStudio/feedback/details/2083820/printf-format-specifier-zu-is-supported-but-not-documented
>
> We also support 2012 and 2013. SIZE_SPECIFIER becomes zu for gcc and
> such, but Iu for MSVC.

old software..............
-- 
Vittorio