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

[James Almer]

On 2/28/2017 3:06 PM, Vittorio Giovara wrote:
> On Tue, Feb 28, 2017 at 12:46 PM, Vittorio Giovara
> <vittorio.giovara at gmail.com> wrote:
>>> ----
>>> 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
>>>         }
>>
>> Sorry, I don't follow, what is your suggestion?
> 
> nevermind, i get it, and ok
> 
>>>>      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);
> 
> ok sure

You pushed these two chunks without any of the cosmetic changes i
suggested. You did apply them on libav, though. Do you mind doing it
here as well, or should i?