Given this insistence on using a separate type, the fact that the "tiled equirect" is under specified, and folks don't actually care about the "tiled equirect" case right now could I just add code to mux the 2 cases that are already well specified? I could also send out a patch to fix the demuxers so they reject the "tiled equirect" cases for now. This seems like reasonable compromise to allow end-to-end preservation of non-controversial use cases. That is really all I'm trying to do right now.
Aaron On Sat, Feb 4, 2017 at 7:46 AM Vittorio Giovara <vittorio.giov...@gmail.com> wrote: > On Fri, Feb 3, 2017 at 5:57 PM, Aaron Colwell <acolw...@google.com> wrote: > > I still think you don't understand what these fields do given what you > say > > here. Yes there is a little more math. At the end of the day all these > > fields do is specify a the min & max for the latitude & longitude. This > > essentially translates to adding scale factors and offsets in your > shader or > > something similar in your 3D geometry creation logic. I get it if > > implementations don't want to do this small bit of extra work, but saying > > this is a different type seems strange because you wouldn't do this when > > talking about cropped 2D images. > > If I don't understand these fields, maybe the specification could do a > better job at explaining what they are for ;) > > I am aware that the final projection is the same, but the actual > problem is that if we don't introduce a new type we would be conveying > a different semantics to a single projection type. In other words we > require applications to behave differently according to two different > fields (the projection type and the offset fields) rather than a > single one. So yes, the projection is the same, the usecase is > different, the app behaviour is different, the final processing is > different -- I think that all this warrants a separate type. > > If I still didn't get my point across, let's try with an example: an > application that does not support the tiled equirectangular > projection, with a separate type it can immediately discern that it is > an unsupported type and inform the user, with a single type, instead, > it has to sort-of-implement the tiling and check for fields that > should not matter. Another example: look at AVStereo3DType, there is a > side-by-side and a side-by-side-quincunx : an application that does > not support quincux can just abort and notify the user, if they were > two separate fields, you could have applications that do not support > quincunx but try to render the side-by-side part (which usually > results in a garbage rendering). > > So I reiterate that in my opinion a separate enum value which > "notifies" apps that they should check additional types is the way to > go. > > >> 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. > > > > > > It feels like you are just moving the problem to the demuxers and muxers > > here. Adding a new type means all demuxers will have to contain logic to > > generate these different types and all muxers will have to contain logic > to > > collapse these types back down to a single value. > > Yes, I would a 100% add more logic to the 2 muxers and 2 demuxers that > implement this spec rather than the thousands applications that > implement the ffmpeg api. Also the "different logic" is literally an > "if" case, if not little else. > > > I don't really want to keep arguing about this. If folks really want > > different types then I'll do it just because I want to get reading and > > writing of metadata working end-to-end. I'd like to make a small request > to > > use the term "cropped equirectagular" instead of "tiled equirectangular" > but > > I don't feel to strongly about that. > > Please don't, "cropped" has entirely different meaning, and it's > already confusing that you can do bitstream level cropping and surface > cropping. If you really hate the term "tiled" you could use "bounded > equirectangular" as it is used in the spec. > > > I suppose this is just a difference in style so I don't feel too strongly > > about it. I was just trying to use unions & structs here to make it a > little > > clearer about which fields are expected to be valid and when. The fields > > seemed to be disjoint sets so I was trying to use language features to > > convey that. > > > > I'd also like to float a potential alternative solution. We could just > > convey the projection private data as a size and byte array in this > struct. > > That would allow me to pass the metadata from demuxers to muxers so I can > > achieve my goals, and allow actual parsing of the information to be > deferred > > until someone needs it. How do you feel about this compromise position? > > Again I don't see any advantage in using your proposal, it makes the > code difficult to read and hard to debug. Binary metadata are > intrinsically bad in my opinion, for this use case you could just add > four fields, and read/write four times. > > I still have the code I had around when I implemented that. > > + projection = AV_SPHERICAL_EQUIRECTANGULAR; > + > + /* 0.32 fixed point */ > + t = sc->height * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); > + b = sc->height * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); > + l = sc->width * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); > + r = sc->width * (float) avio_rb32(pb) / ((uint64_t) 1 << 32); > > [...] > > + sc->spherical->left_offset = l; > + sc->spherical->top_offset = t; > + sc->spherical->right_offset = r; > + sc->spherical->bottom_offset = b; > > (and the reverse for writing of course). > -- > Vittorio > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
