On Thu, Mar 30, 2017 at 7:17 AM James Almer <jamr...@gmail.com> wrote:
> On 3/30/2017 5:54 AM, Vittorio Giovara wrote: > > On Wed, Mar 29, 2017 at 8:01 PM, James Almer <jamr...@gmail.com> wrote: > >> A new field is added to AVSphericalMapping for this purpose, > >> and is used by both Equirectangular and Cubemap projections. > >> This is a replacement for duplicate projection enums like > >> AV_SPHERICAL_EQUIRECTANGULAR_TILE, which are therefore > >> removed. > >> > >> Signed-off-by: James Almer <jamr...@gmail.com> > >> --- > >> This patch depends on the av_spherical_projection_name() patchset for > >> simplicity purposes. > >> > >> Ok, this is an RFC mainly because of the API/ABI break it represents. > >> The AV_SPHERICAL_EQUIRECTANGULAR_TILE projection is a month and a half > >> old and not present in any release, plus a major bump is queued as part > >> of the merges, so i personally think this change is acceptable as is for > >> such an niche and recent feature. > >> If not then i can deprecate said projection enum value instead and keep > >> the current muxer functionality for it for a while. It will need a lot > >> of preprocessor guards, though. > >> > >> The reason for this change is that eventually, a new projection enum > >> for padded cubemap may be suggested with the same arguments as the ones > >> used to introduce the one for tiled equirectangular. Then if any new > >> real projections are added, we'd could end up with duplicate enums for > >> them as well, when setting a single shared flag would be enough. > >> Stereo3D avoided a lot of duplicate types with the inverted flag, so i > >> figured the same should be done here. > >> > >> Improved doxy and/or flag name is extremely welcome (Read: needed). > > > > I'm against this idea because of explanations given in other threads. > > > > The stereo3d case is different because it's a property that can be > > applied to all types, while the cropped/padded feature applies to > > current existing projections, not the future ones. > > But it could apply. It's fairly likely that cropping/padding of unused > pixels will be present in future projections. > And this flag will not necessarily be the only one in the long term. > You could end up having a new property at some point that may have to > be signaled in some way. And it could even be present in a projection > alongside cropping/padding. > > It probably isn't a surprise, but I agree with James here. I think it is highly likely other projections will have tiling/cropping in them and I think it is better to reflect this as a flag instead of merging it with the projection enum. I'm not 100% sure padding should be rolled into the same flag as cropping, but I think that is a detail that could be ironed out separately. I think the flag mechanism itself is a good idea. > > Additionally there > > is nothing wrong with having more enum values, since it is likely that > > future cubemaps layouts will have a different enum value, and not > > another field to check. > > Having a single flags field with dozens of potential flags seems like > a much cleaner solution than several enum values to list the many > different ways a single projection can be handled to me. > I agree. In my view this addresses the "having to check multiple fields" objection raised in previous threads because now a single flag can be used to indicate whether you need to pay attention to the bound_xxx fields or not. It seems like a more reasonable and extendable compromise to me. I also don't think different cubemap layouts necessarily should have their own enum values. This feels roughly akin to merging an audio codec_id and the channel mapping into a single enum space. Aaron _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
