On Thu, Feb 2, 2017 at 9:34 PM, James Almer <jamr...@gmail.com> wrote: > On 1/31/2017 12:40 PM, Aaron Colwell wrote: >> >> >> On Tue, Jan 31, 2017 at 2:12 AM Vittorio Giovara <vittorio.giov...@gmail.com >> <mailto:vittorio.giov...@gmail.com>> wrote: >> >> On Sat, Jan 28, 2017 at 4:13 AM, James Almer <jamr...@gmail.com >> <mailto:jamr...@gmail.com>> wrote: >>> On 1/27/2017 11:21 PM, Aaron Colwell wrote: >>>> On Fri, Jan 27, 2017 at 5:46 PM James Almer <jamr...@gmail.com >>>> <mailto:jamr...@gmail.com>> wrote: >>>> >>>> yeah. I'm a little confused why the demuxing code didn't implement this to >>>> begin with. >>> >>> Vittorio (The one that implemented AVSphericalMapping) tried to add this at >>> first, but then removed it because he wasn't sure if he was doing it right. >> >> Hi, >> yes this was included initially but then we found out what those >> fields were really for, and I didn't want to make the users get as >> confused as we were. As a matter of fact Aaron I mentioned this to you >> when I proposed that we probably should have separated the classic >> equi projection from the tiled one in the specification, in order to >> simplify the implementation. >> >> >> Like I said before, it is not a different projection. It is still >> equirectangular and those parameters just crops the projection. It is very >> simple to just verify that the fields are zero if you don't want to support >> the cropped version.
Hello, I'm sorry but I heavily disagree. The tiled equirectangular projection is something that cannot be used standalone, you have to do additional mathematics and take into account different files or streams to generate a "normal" or full-frame equirectangular projection. Having a separate type allows to include extensions such as the bounds fields, which can be safely ignored by the every user that do not need a tiled projection. It is too late to change the spec, but I do believe that the usecase is different enough to add a new type, in order to not overcomplicate the implementation. >>>>> I know you're the one behind the spec in question, but wouldn't it be a >>>>> better idea to wait until AVSphericalMapping gets a way to propagate this >>>>> kind of information before adding support for muxing video projection >>>>> elements? Or maybe try to implement it right now... >>>>> >>>> >>>> I'm happy to implement support for the projection specific info. What would >>>> be the best way to proceed. It seems like I could just place a union with >>>> projection specific structs in AVSphericalMapping. I'd also like some >>> >>> I'm CCing Vittorio so he can chim in. I personally have no real preference. >> >> The best way in my opinion is to add a third type, such as >> AV_SPHERICAL_TILED_EQUI, and add the relevant fields in >> AVSphericalMapping, with a clear description about the actual use case >> for them, mentioning that they are used only in format. By the way, >> why do you mention adding a union? I think four plain fields should >> do. >> >> >> I don't think it is worth having the extra enum value for this. All the >> cropped fields do is control how you generate the spherical mesh or control >> the shader used to render the projection. If players don't want to support >> it they can just check to see that all the fields are zero and error out if >> not. Why would you have them check these fields every time, when this can be implicitly determined by the type semantics. I'm pretty sure API users prefer this scenario * check projection type -> if normal_equi -> project it -> if tiled_equi -> read additional data -> project it rather than * check projection type -> if equi -> read additional data -> check if data needs additional processing -> project it, or perform more operations before projecting >> I was suggesting using a union because the projection bounds fields are for >> equirect, and there are different fields for the cubemap & mesh projections. >> I figured that the enum + union of structs might be a reasonable way to >> organize the projection specific fields. This is a structure whose size does not depend on ABI and can be extended as we like, there is no need to separate new fields in such a way as long as they are properly documented, in my opinion. Please keep me in CC. -- Vittorio _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
