[FFmpeg-devel] [PATCH][RFC] avutil/spherical: add a flag to signal tiled/padded projections

Aaron Colwell acolwell at google.com
Thu Mar 30 17:41:44 EEST 2017


On Thu, Mar 30, 2017 at 7:17 AM James Almer <jamrial at gmail.com> wrote:

> On 3/30/2017 5:54 AM, Vittorio Giovara wrote:
> > On Wed, Mar 29, 2017 at 8:01 PM, James Almer <jamrial at 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 <jamrial at 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


More information about the ffmpeg-devel mailing list