On 3/8/2017 7:09 PM, Vittorio Giovara wrote:
> On Wed, Mar 8, 2017 at 4:05 PM, James Almer <jamr...@gmail.com> wrote:
>> On 3/8/2017 5:08 PM, Vittorio Giovara wrote:
>>> On Wed, Mar 8, 2017 at 2:46 PM, James Almer <jamr...@gmail.com> wrote:
>>>> Signed-off-by: James Almer <jamr...@gmail.com>
>>>> ---
>>>> libavformat/matroskaenc.c | 69
>>>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 69 insertions(+)
>>>>
>>>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>>>> index 1605f0cafe..0ee927d63e 100644
>>>> --- a/libavformat/matroskaenc.c
>>>> +++ b/libavformat/matroskaenc.c
>>>> @@ -918,6 +918,72 @@ static int mkv_write_video_color(AVIOContext *pb,
>>>> AVCodecParameters *par, AVStre
>>>> return 0;
>>>> }
>>>>
>>>> +static int mkv_write_video_projection(AVIOContext *pb, AVStream *st) {
>>>> + int side_data_size = 0;
>>>> + const AVSphericalMapping *spherical =
>>>> + (const AVSphericalMapping*) av_stream_get_side_data(st,
>>>> AV_PKT_DATA_SPHERICAL,
>>>> +
>>>> &side_data_size);
>>>> +
>>>> + if (side_data_size == sizeof(AVSphericalMapping)) {
>>>
>>> I don't think you have to check for this (and checking sizeof() of
>>> this struct outside lavu breaks ABI), it's enough to check that size
>>> != 0
>>
>> Ok.
>>
>> You'll probably also have to do the same on libavformat/dump.c, for
>> that matter.
>
> This is valid for most side data there, somebody should do a thorough cleanup
>
>>>> + put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
>>>> private, sizeof(private));
>>>> + break;
>>>> + }
>>>> + case AV_SPHERICAL_EQUIRECTANGULAR:
>>>> + put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>>>> + MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR);
>>>> + break;
>>>> + case AV_SPHERICAL_CUBEMAP:
>>>> + {
>>>> + uint8_t private[12];
>>>> + put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONTYPE,
>>>> + MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP);
>>>> + AV_WB32(private + 0, 0); // version + flags
>>>> + AV_WB32(private + 4, 0); // layout
>>>> + AV_WB32(private + 8, spherical->padding);
>>>> + put_ebml_binary(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPRIVATE,
>>>> private, sizeof(private));
>>>> + break;
>>>> + }
>>>> + default:
>>>> + // TODO: Mesh projection once implemented in
>>>> AVSphericalMapping
>>>
>>> a little av_log message to warn about this?
>>
>> This is a muxer, spherical->projection will be one of the supported enums.
>> This default case is just to prevent potential compiler warnings and can
>> be removed.
>>
>> What needs a log message is the demuxer.
>
> This is a muxer, that can be created by an API user that does not know
> how to read documentation and who initialises spherical->projection
> wrong. I didn't say to error out, but I think that adding just a
> warning is easier to deal with than having to debug it manually.
Alright, will add one before pushing.
>
>>>
>>>> + goto end;
>>>> + }
>>>> +
>>>> + put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEYAW,
>>>> (double)spherical->yaw / (1 << 16));
>>>> + put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEPITCH,
>>>> (double)spherical->pitch / (1 << 16));
>>>> + put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOPROJECTIONPOSEROLL,
>>>> (double)spherical->roll / (1 << 16));
>>>
>>> does the matroska spec require plain integers?
>>> spherical->{yaw,pitch,roll} are in 16.16 so there should be no need of
>>> extra conversions
>>
>> Matroska stores these three as double precision fp values.
>
> nice
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
