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.
>>
>>> + 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
--
Vittorio
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
