diff mbox

[FFmpeg-devel] mov, matroskadec : Allow matroskadec & mov to share spherical parsing logic.

Message ID CAA0c1bB8Ln9RF1gbhZnkWMshfzS5zPHVk-t2Wzr0CJaUCo03SA@mail.gmail.com
State Not Applicable
Headers show

Commit Message

Aaron Colwell Feb. 7, 2017, 2:11 a.m. UTC
I've attached an updated patch and modified test suite files so that the
code now passes FATE on my local machine.

The media files needed to be updated because they contained fields that
indicated tiled-equirect.  Based on the discussion in another thread,
tiled-equirect is supposed to get its own enum value so technically
the existing code was being more permissive than it should have been and
was signalling full sphere equirect when it shouldn't have been. Full
sphere equirect is actually what most people use so it seems reasonable to
adjust the test files to reflect that case instead.

Aaron

On Mon, Feb 6, 2017 at 4:57 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Mon, Feb 06, 2017 at 09:59:58PM +0000, Aaron Colwell wrote:
> > - Extracts common spherical metadata parsing logic.
> > - Adds checks to enforce that only non-tiled equirect & non-padded
> cubemaps
> >   are accepted.
>
> >  Makefile        |    5 ++-
> >  matroskadec.c   |   39 ++++++++++++++--------------
> >  mov.c           |   35 ++++++++++++++-----------
> >  mov_spherical.c |   78
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  mov_spherical.h |   61 +++++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 182 insertions(+), 36 deletions(-)
> > ebec975d2645e6cde0902b8a966545617fc420dc
> 0001-mov-matroskadec-Allow-matroskadec-mov-to-share-spher.patch
> > From 218dfd7ca1924bc44a168b34ec3c07051823c2dc Mon Sep 17 00:00:00 2001
> > From: Aaron Colwell <acolwell@google.com>
> > Date: Mon, 6 Feb 2017 13:48:58 -0800
> > Subject: [PATCH] mov,matroskadec : Allow matroskadec & mov to share
> spherical
> >  parsing logic.
> >
> > - Extracts common spherical metadata parsing logic.
> > - Adds checks to enforce that only non-tiled equirect & non-padded
> cubemaps
> >   are accepted.
> > ---
> >  libavformat/Makefile        |  5 +--
> >  libavformat/matroskadec.c   | 39 ++++++++++++-----------
> >  libavformat/mov.c           | 35 +++++++++++---------
> >  libavformat/mov_spherical.c | 78
> +++++++++++++++++++++++++++++++++++++++++++++
> >  libavformat/mov_spherical.h | 61 +++++++++++++++++++++++++++++++++++
> >  5 files changed, 182 insertions(+), 36 deletions(-)
> >  create mode 100644 libavformat/mov_spherical.c
> >  create mode 100644 libavformat/mov_spherical.h
>
> breaks fate-h264-direct-bff
> TEST    h264-direct-bff
> --- ./tests/ref/fate/h264-direct-bff    2017-02-06 12:12:00.827925330 +0100
> +++ tests/data/fate/h264-direct-bff     2017-02-07 01:54:38.584965163 +0100
> @@ -1,16 +0,0 @@
> -#tb 0: 1001/30000
> -#media_type 0: video
> -#codec_id 0: rawvideo
> -#dimensions 0: 720x484
> -#sar 0: 1/1
> -0,          0,          0,        1,   522720, 0x1ccad503
> -0,          1,          1,        1,   522720, 0xd266d6e8
> -0,          2,          2,        1,   522720, 0x535473b3
> -0,          3,          3,        1,   522720, 0xf8b53c53
> -0,          4,          4,        1,   522720, 0x4e4cc04b
> -0,          5,          5,        1,   522720, 0x20ea3515
> -0,          6,          6,        1,   522720, 0xb9c67e30
> -0,          7,          7,        1,   522720, 0x03d2e35a
> -0,          8,          8,        1,   522720, 0xae2e7896
> -0,          9,          9,        1,   522720, 0x6da37f41
> -0,         12,         12,        1,   522720, 0x7caf4954
> Test h264-direct-bff failed. Look at tests/data/fate/h264-direct-bff.err
> for details.
> make: *** [fate-h264-direct-bff] Error 1
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Asymptotically faster algorithms should always be preferred if you have
> asymptotical amounts of data
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff mbox

Patch

From a953c6fb909cfabb5d208b22d02e8904d4095734 Mon Sep 17 00:00:00 2001
From: Aaron Colwell <acolwell@google.com>
Date: Mon, 6 Feb 2017 13:48:58 -0800
Subject: [PATCH] mov,matroskadec : Allow matroskadec & mov to share spherical
 parsing logic.

- Extracts common spherical metadata parsing logic.
- Adds checks to enforce that only non-tiled equirect & non-padded cubemaps
  are accepted.
---
 libavformat/Makefile        |  5 +--
 libavformat/matroskadec.c   | 42 +++++++++++++-----------
 libavformat/mov.c           | 35 +++++++++++---------
 libavformat/mov_spherical.c | 78 +++++++++++++++++++++++++++++++++++++++++++++
 libavformat/mov_spherical.h | 61 +++++++++++++++++++++++++++++++++++
 5 files changed, 185 insertions(+), 36 deletions(-)
 create mode 100644 libavformat/mov_spherical.c
 create mode 100644 libavformat/mov_spherical.h

diff --git a/libavformat/Makefile b/libavformat/Makefile
index fc2d76067b..15ec4e42b6 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -265,7 +265,8 @@  OBJS-$(CONFIG_M4V_MUXER)                 += rawenc.o
 OBJS-$(CONFIG_MATROSKA_DEMUXER)          += matroskadec.o matroska.o  \
                                             rmsipr.o flac_picture.o \
                                             oggparsevorbis.o vorbiscomment.o \
-                                            flac_picture.o replaygain.o
+                                            flac_picture.o replaygain.o \
+                                            mov_spherical.o
 OBJS-$(CONFIG_MATROSKA_MUXER)            += matroskaenc.o matroska.o \
                                             avc.o hevc.o \
                                             flacenc_header.o avlanguage.o vorbiscomment.o wv.o \
@@ -283,7 +284,7 @@  OBJS-$(CONFIG_MLV_DEMUXER)               += mlvdec.o riffdec.o
 OBJS-$(CONFIG_MM_DEMUXER)                += mm.o
 OBJS-$(CONFIG_MMF_DEMUXER)               += mmf.o
 OBJS-$(CONFIG_MMF_MUXER)                 += mmf.o rawenc.o
-OBJS-$(CONFIG_MOV_DEMUXER)               += mov.o mov_chan.o replaygain.o
+OBJS-$(CONFIG_MOV_DEMUXER)               += mov.o mov_chan.o mov_spherical.o replaygain.o
 OBJS-$(CONFIG_MOV_MUXER)                 += movenc.o avc.o hevc.o vpcc.o \
                                             movenchint.o mov_chan.o rtp.o \
                                             movenccenc.o rawutils.o
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 7223e94b55..abb8676762 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -54,6 +54,7 @@ 
 #include "internal.h"
 #include "isom.h"
 #include "matroska.h"
+#include "mov_spherical.h"
 #include "oggdec.h"
 /* For ff_codec_get_id(). */
 #include "riff.h"
@@ -1909,36 +1910,39 @@  static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
     return 0;
 }
 
-static int mkv_parse_video_projection(AVStream *st, const MatroskaTrack *track) {
+static int mkv_parse_video_projection(AVFormatContext *fc, AVStream *st, const MatroskaTrack *track) {
     AVSphericalMapping *spherical;
-    enum AVSphericalProjection projection;
     size_t spherical_size;
-    int ret;
+    int ret = 0;
+    AVIOContext pb;
 
-    switch (track->video.projection.type) {
-    case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
-        projection = AV_SPHERICAL_EQUIRECTANGULAR;
-        break;
-    case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
-        if (track->video.projection.private.size < 4)
-            return AVERROR_INVALIDDATA;
-        projection = AV_SPHERICAL_CUBEMAP;
-        break;
-    default:
-        return 0;
-    }
+    if (track->video.projection.type == MATROSKA_VIDEO_PROJECTION_TYPE_RECTANGULAR)
+      return 0;
 
     spherical = av_spherical_alloc(&spherical_size);
     if (!spherical)
         return AVERROR(ENOMEM);
-    spherical->projection = projection;
 
     spherical->yaw   = (int32_t)(track->video.projection.yaw   * (1 << 16));
     spherical->pitch = (int32_t)(track->video.projection.pitch * (1 << 16));
     spherical->roll  = (int32_t)(track->video.projection.roll  * (1 << 16));
 
-    ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t *)spherical,
-                                  spherical_size);
+    ffio_init_context(&pb, track->video.projection.private.data, track->video.projection.private.size,
+                      0, NULL, NULL, NULL, NULL);
+    switch (track->video.projection.type) {
+    case MATROSKA_VIDEO_PROJECTION_TYPE_EQUIRECTANGULAR:
+        ret = ff_mov_read_equi(fc, &pb, track->video.projection.private.size, spherical);
+        break;
+    case MATROSKA_VIDEO_PROJECTION_TYPE_CUBEMAP:
+        ret = ff_mov_read_cbmp(fc, &pb, track->video.projection.private.size, spherical);
+        break;
+    default:
+        ret = AVERROR_PATCHWELCOME;
+    }
+
+    if (!ret)
+      ret = av_stream_add_side_data(st, AV_PKT_DATA_SPHERICAL, (uint8_t *)spherical, spherical_size);
+
     if (ret < 0) {
         av_freep(&spherical);
         return ret;
@@ -2427,7 +2431,7 @@  static int matroska_parse_tracks(AVFormatContext *s)
             ret = mkv_parse_video_color(st, track);
             if (ret < 0)
                 return ret;
-            ret = mkv_parse_video_projection(st, track);
+            ret = mkv_parse_video_projection(matroska->ctx, st, track);
             if (ret < 0)
                 return ret;
         } else if (track->type == MATROSKA_TRACK_TYPE_AUDIO) {
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 6fd43a0a4e..9fdaca0a28 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -56,6 +56,7 @@ 
 #include "libavcodec/get_bits.h"
 #include "id3v1.h"
 #include "mov_chan.h"
+#include "mov_spherical.h"
 #include "replaygain.h"
 
 #if CONFIG_ZLIB
@@ -4624,7 +4625,7 @@  static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     int size;
     int32_t yaw, pitch, roll;
     uint32_t tag;
-    enum AVSphericalProjection projection;
+    int ret = 0;
 
     if (c->fc->nb_streams < 1)
         return 0;
@@ -4676,32 +4677,36 @@  static int mov_read_sv3d(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     roll  = avio_rb32(pb);
 
     size = avio_rb32(pb);
-    if (size > atom.size)
+    if (size < 8 && size > atom.size)
         return AVERROR_INVALIDDATA;
 
     tag = avio_rl32(pb);
-    avio_skip(pb, 4); /*  version + flags */
+
+    sc->spherical = av_spherical_alloc(&sc->spherical_size);
+    if (!sc->spherical)
+        return AVERROR(ENOMEM);
+
+    sc->spherical->yaw   = yaw;
+    sc->spherical->pitch = pitch;
+    sc->spherical->roll  = roll;
+
     switch (tag) {
     case MKTAG('c','b','m','p'):
-        projection = AV_SPHERICAL_CUBEMAP;
+        ret = ff_mov_read_cbmp(c->fc, pb, size - 8, sc->spherical);
         break;
     case MKTAG('e','q','u','i'):
-        projection = AV_SPHERICAL_EQUIRECTANGULAR;
+        ret = ff_mov_read_equi(c->fc, pb, size - 8, sc->spherical);
         break;
     default:
         av_log(c->fc, AV_LOG_ERROR, "Unknown projection type\n");
-        return 0;
+        ret = AVERROR_PATCHWELCOME;
     }
 
-    sc->spherical = av_spherical_alloc(&sc->spherical_size);
-    if (!sc->spherical)
-        return AVERROR(ENOMEM);
-
-    sc->spherical->projection = projection;
-
-    sc->spherical->yaw   = yaw;
-    sc->spherical->pitch = pitch;
-    sc->spherical->roll  = roll;
+    if (ret < 0) {
+      av_freep(&sc->spherical);
+      sc->spherical_size = 0;
+      return ret;
+    }
 
     return 0;
 }
diff --git a/libavformat/mov_spherical.c b/libavformat/mov_spherical.c
new file mode 100644
index 0000000000..a9971142bd
--- /dev/null
+++ b/libavformat/mov_spherical.c
@@ -0,0 +1,78 @@ 
+/*
+ * Copyright (c) 2017 Aaron Colwell
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "mov_spherical.h"
+
+int ff_mov_read_equi(AVFormatContext *fc, AVIOContext *pb, int64_t size,
+                     AVSphericalMapping* spherical_mapping)
+{
+    uint32_t projection_bounds_top;
+    uint32_t projection_bounds_bottom;
+    uint32_t projection_bounds_left;
+    uint32_t projection_bounds_right;
+
+    if (size == 0) {
+        spherical_mapping->projection = AV_SPHERICAL_EQUIRECTANGULAR;
+        return 0;
+    }
+
+    if (size < 20)
+        return AVERROR_INVALIDDATA;
+
+    avio_skip(pb, 4); /*  version + flags */
+    projection_bounds_top = avio_rb32(pb);
+    projection_bounds_bottom = avio_rb32(pb);
+    projection_bounds_left = avio_rb32(pb);
+    projection_bounds_right = avio_rb32(pb);
+
+    if (projection_bounds_top != 0 || projection_bounds_bottom != 0 ||
+        projection_bounds_left != 0 || projection_bounds_right != 0) {
+        av_log(fc, AV_LOG_ERROR, "Unsupported equirect bounds. top %u bottom %u left %u right %u\n",
+               projection_bounds_top, projection_bounds_bottom,
+               projection_bounds_left, projection_bounds_right);
+        return AVERROR_PATCHWELCOME;
+    }
+
+    spherical_mapping->projection = AV_SPHERICAL_EQUIRECTANGULAR;
+    return 0;
+}
+
+int ff_mov_read_cbmp(AVFormatContext *fc, AVIOContext *pb, int64_t size,
+                     AVSphericalMapping* spherical_mapping)
+{
+    uint32_t layout;
+    uint32_t padding;
+
+    if (size < 12)
+        return AVERROR_INVALIDDATA;
+
+    avio_skip(pb, 4); /*  version + flags */
+    layout = avio_rb32(pb);
+    padding = avio_rb32(pb);
+
+    if (layout != 0 || padding != 0) {
+        av_log(fc, AV_LOG_ERROR, "Unsupported cubemap parameters. layout %u, padding %u\n",
+               layout, padding);
+        return AVERROR_PATCHWELCOME;
+    }
+
+    spherical_mapping->projection = AV_SPHERICAL_CUBEMAP;
+    return 0;
+}
diff --git a/libavformat/mov_spherical.h b/libavformat/mov_spherical.h
new file mode 100644
index 0000000000..fe514a4b6c
--- /dev/null
+++ b/libavformat/mov_spherical.h
@@ -0,0 +1,61 @@ 
+/*
+ * Copyright (c) 2017 Aaron Colwell
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * mov 'equi', 'cbmp' tag reading/writing.
+ * @author Aaron Colwell
+ */
+
+#ifndef AVFORMAT_MOV_SPHERICAL_H
+#define AVFORMAT_MOV_SPHERICAL_H
+
+#include <stdint.h>
+#include "libavcodec/avcodec.h"
+#include "libavformat/avformat.h"
+#include "libavformat/avio.h"
+#include "libavutil/spherical.h"
+
+/**
+ * Parses the contents of an 'equi' box and sets the appropriate fields in spherical_mapping.
+ *
+ * @param fc    AVFormatContext
+ * @param pb    AVIOContext containing 'equi' box data.
+ * @param size  Remaining size in the 'equi' box
+ * @param spherical_mapping Structure updated with parsed data.
+ * @return      0 if ok, or negative AVERROR code on failure
+ *
+ */
+int ff_mov_read_equi(AVFormatContext *fc, AVIOContext *pb, int64_t size,
+                     AVSphericalMapping* spherical_mapping);
+
+/**
+ * Parses the contents of an 'cbmp' box and sets the appropriate fields in spherical_mapping.
+ *
+ * @param fc    AVFormatContext
+ * @param pb    AVIOContext containing 'cbmp' box data.
+ * @param size  Remaining size in the 'cbmp' box
+ * @param spherical_mapping Structure updated with parsed data.
+ * @return      0 if ok, or negative AVERROR code on failure
+ *
+ */
+int ff_mov_read_cbmp(AVFormatContext *fc, AVIOContext *pb, int64_t size,
+                     AVSphericalMapping* spherical_mapping);
+
+#endif /* AVFORMAT_MOV_SPHERICAL_H */
-- 
2.11.0.483.g087da7b7c-goog