diff mbox series

[FFmpeg-devel,3/3] avutil/stereo3d: add a Stereo3D type to signal that the packing is defined by the view field

Message ID 20240622024111.6598-1-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/2] avutil/stereo3d add Monoscopic View enum value | expand

Checks

Context Check Description
yinshiyou/commit_msg_loongarch64 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/commit_msg_x86 warning Please wrap lines in the body of the commit message between 60 and 72 characters.
andriy/make_x86 success Make finished
andriy/make_fate_x86 fail Make fate failed

Commit Message

James Almer June 22, 2024, 2:41 a.m. UTC
Given that a video stream/frame may have only one view coded, or both packed in
an undefined way, and as the values of AVStereo3DView and AVStereo3DType may
clash (namely if type is AV_STEREO3D_2D, then AV_STEREO3D_VIEW_PACKED would be
invalid, and if it's anything other than it, then only AV_STEREO3D_VIEW_PACKED
would be valid), this commit adds a new type value AV_STEREO3D_VIEW that
signals the user that AVStereo3D.view contains information about the nature of
the stream, with the added constrain that AVStereo3D.view should be ignored if
AVStereo3D.type is anything other than AV_STEREO3D_VIEW.

Signed-off-by: James Almer <jamrial@gmail.com>
---
This is the only way i could think of to work around the fact AVStereo3DType
and AVStereo3DView just can't work well together if we want to keep AVStereo
backwards compatible.

 libavutil/stereo3d.c |  1 +
 libavutil/stereo3d.h | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

Comments

Anton Khirnov June 25, 2024, 8:42 a.m. UTC | #1
Quoting James Almer (2024-06-22 04:41:11)
> Given that a video stream/frame may have only one view coded, or both packed in
> an undefined way, and as the values of AVStereo3DView and AVStereo3DType may
> clash (namely if type is AV_STEREO3D_2D, then AV_STEREO3D_VIEW_PACKED would be
> invalid, and if it's anything other than it, then only AV_STEREO3D_VIEW_PACKED
> would be valid), this commit adds a new type value AV_STEREO3D_VIEW that
> signals the user that AVStereo3D.view contains information about the nature of
> the stream, with the added constrain that AVStereo3D.view should be ignored if
> AVStereo3D.type is anything other than AV_STEREO3D_VIEW.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> This is the only way i could think of to work around the fact AVStereo3DType
> and AVStereo3DView just can't work well together if we want to keep AVStereo
> backwards compatible.

I don't really see what actual problem is this supposed to address,
rather it seems to me it just muddles the situation further.

When type is AV_STEREO3D_2D, the video is monoscopic and everything else
in AVStereo3D should be ignored. In this light I also don't see the
point of patch 1.

Nontrivial values of AVStereo3DView are meaningful only for a
stereoscopic non-packed video, i.e. AV_STEREO3D_FRAMESEQUENCE.
diff mbox series

Patch

diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
index 0c0cad127c..e8e5030db9 100644
--- a/libavutil/stereo3d.c
+++ b/libavutil/stereo3d.c
@@ -65,6 +65,7 @@  static const char * const stereo3d_type_names[] = {
     [AV_STEREO3D_SIDEBYSIDE_QUINCUNX] = "side by side (quincunx subsampling)",
     [AV_STEREO3D_LINES]               = "interleaved lines",
     [AV_STEREO3D_COLUMNS]             = "interleaved columns",
+    [AV_STEREO3D_VIEW]                = "view defined",
 };
 
 static const char * const stereo3d_view_names[] = {
diff --git a/libavutil/stereo3d.h b/libavutil/stereo3d.h
index 77340f72b2..e3af519e03 100644
--- a/libavutil/stereo3d.h
+++ b/libavutil/stereo3d.h
@@ -136,6 +136,14 @@  enum AVStereo3DType {
      * @endcode
      */
     AV_STEREO3D_COLUMNS,
+
+    /**
+     * Video may be monoscopic, or stereoscopic where either the
+     * packing is unknown or only one view is present.
+     *
+     * @see AVStereo3DView
+     */
+    AV_STEREO3D_VIEW,
 };
 
 /**
@@ -207,7 +215,8 @@  typedef struct AVStereo3D {
     int flags;
 
     /**
-     * Determines which views are packed.
+     * Determines which views are packed. This field should be ignored when
+     * @ref type is set to anything other than AV_STEREO3D_VIEW.
      */
     enum AVStereo3DView view;