On 2/14/2017 11:20 PM, Vittorio Giovara wrote: > On Tue, Feb 14, 2017 at 6:54 PM, James Almer <jamr...@gmail.com> wrote: >> On 2/14/2017 5:52 PM, Vittorio Giovara wrote: >>> On Fri, Feb 10, 2017 at 6:25 PM, Michael Niedermayer >>> <mich...@niedermayer.cc> wrote: >>>> On Fri, Feb 10, 2017 at 04:11:43PM -0500, Vittorio Giovara wrote: >>>>> Signed-off-by: Vittorio Giovara <vittorio.giov...@gmail.com> >>>>> --- >>>>> This should help not losing details over muxing and allows >>>>> callers to get additional information in a clean manner. >>>>> >>>>> Please keep me in CC. >>>>> Vittorio >>>>> >>>>> doc/APIchanges | 5 +++++ >>>>> ffprobe.c | 11 ++++++++-- >>>>> libavformat/dump.c | 10 +++++++++ >>>>> libavutil/spherical.h | 56 >>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> libavutil/version.h | 2 +- >>>>> 5 files changed, 81 insertions(+), 3 deletions(-) >>>> >>>> breaks fate >>>> >>>> --- ./tests/ref/fate/matroska-spherical-mono 2017-02-10 >>>> 23:43:51.993432371 +0100 >>>> +++ tests/data/fate/matroska-spherical-mono 2017-02-11 >>>> 00:24:10.297483318 +0100 >>>> @@ -7,7 +7,7 @@ >>>> [/SIDE_DATA] >>>> [SIDE_DATA] >>>> side_data_type=Spherical Mapping >>>> -side_data_size=16 >>>> +side_data_size=56 >>>> projection=equirectangular >>>> yaw=45 >>>> pitch=30 >>>> Test matroska-spherical-mono failed. Look at >>>> tests/data/fate/matroska-spherical-mono.err for details. >>>> make: *** [fate-matroska-spherical-mono] Error 1 >>> >>> Ah I didn't notice, it is fixed in the next commit, but I'll amend this one >>> too. >>> >>> >>> I didn't see any comment/discussion, should I assume it is ok? >>> Please CC, thank you. >> >> These are a lot of projection specific fields. It worries me as the >> spec may change in the future with new fields added or existing >> fields changing purpose. Not to mention the Mesh projection, which >> has like fifty specific fields of its own. > > Even if the spec change (which at this point would be a terrible > terrible thing to do) there are now files in the wild and software > that have adopted this draft, so we would have to support this anyway.
If the spec changes, it will be the contents of the equi/cbmp/mesh. By exporting them raw as extradata, said changes in the spec would require no changes to our implementation. > >> Wouldn't it be a better idea to export the binary data of the >> equi/cbmp/mesh boxes into an extradata-like field in the >> AVSphericalMapping struct, and let the downstream application parse >> it instead? > > No I don't think so, lavf is an abstraction layer and one of its tasks > is to provide a (simple?) unified entry layer. and letting the user > parse binary data is IMO bad design and very fragile. Also it's not > impossible that another standard for tagging spherical metadata > emerges in the future: the current API could very easily wrap it, > while exporting the binary entry would be too specification-specific > and it would be tied too heavily on the google draft. AVSphericalMapping is already pretty tied to the google draft, but i guess you're right, it's at least generic enough for now. Wait for Aaron's opinion before addressing reviews and pushing. He sent a different patchset himself and it wouldn't be nice to push yours without at least giving him a chance to comment. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
